Commit d59f21be authored by Richard Weinberger's avatar Richard Weinberger

UBI: Fastmap: Fix races in ubi_wl_get_peb()

ubi_wl_get_peb() has two problems, it reads the pool
size and usage counters without any protection.
While reading one value would be perfectly fine it reads multiple
values and compares them. This is racy and can lead to incorrect
pool handling.
Furthermore ubi_update_fastmap() is called without wl_lock held,
before incrementing the used counter it needs to be checked again.
It could happen that another thread consumed all PEBs from the
pool and the counter goes beyond ->size.
Signed-off-by: default avatarRichard Weinberger <richard@nod.at>
parent 399a9fee
...@@ -443,7 +443,8 @@ struct ubi_debug_info { ...@@ -443,7 +443,8 @@ struct ubi_debug_info {
* @pq_head: protection queue head * @pq_head: protection queue head
* @wl_lock: protects the @used, @free, @pq, @pq_head, @lookuptbl, @move_from, * @wl_lock: protects the @used, @free, @pq, @pq_head, @lookuptbl, @move_from,
* @move_to, @move_to_put @erase_pending, @wl_scheduled, @works, * @move_to, @move_to_put @erase_pending, @wl_scheduled, @works,
* @erroneous, @erroneous_peb_count, and @fm_work_scheduled fields * @erroneous, @erroneous_peb_count, @fm_work_scheduled, @fm_pool,
* and @fm_wl_pool fields
* @move_mutex: serializes eraseblock moves * @move_mutex: serializes eraseblock moves
* @work_sem: used to wait for all the scheduled works to finish and prevent * @work_sem: used to wait for all the scheduled works to finish and prevent
* new works from being submitted * new works from being submitted
......
...@@ -626,24 +626,36 @@ void ubi_refill_pools(struct ubi_device *ubi) ...@@ -626,24 +626,36 @@ void ubi_refill_pools(struct ubi_device *ubi)
*/ */
int ubi_wl_get_peb(struct ubi_device *ubi) int ubi_wl_get_peb(struct ubi_device *ubi)
{ {
int ret; int ret, retried = 0;
struct ubi_fm_pool *pool = &ubi->fm_pool; struct ubi_fm_pool *pool = &ubi->fm_pool;
struct ubi_fm_pool *wl_pool = &ubi->fm_wl_pool; struct ubi_fm_pool *wl_pool = &ubi->fm_wl_pool;
if (!pool->size || !wl_pool->size || pool->used == pool->size || again:
wl_pool->used == wl_pool->size) spin_lock(&ubi->wl_lock);
/* We check here also for the WL pool because at this point we can
* refill the WL pool synchronous. */
if (pool->used == pool->size || wl_pool->used == wl_pool->size) {
spin_unlock(&ubi->wl_lock);
ubi_update_fastmap(ubi); ubi_update_fastmap(ubi);
spin_lock(&ubi->wl_lock);
}
/* we got not a single free PEB */ if (pool->used == pool->size) {
if (!pool->size) spin_unlock(&ubi->wl_lock);
if (retried) {
ubi_err(ubi, "Unable to get a free PEB from user WL pool");
ret = -ENOSPC; ret = -ENOSPC;
else { goto out;
spin_lock(&ubi->wl_lock); }
retried = 1;
goto again;
}
ubi_assert(pool->used < pool->size);
ret = pool->pebs[pool->used++]; ret = pool->pebs[pool->used++];
prot_queue_add(ubi, ubi->lookuptbl[ret]); prot_queue_add(ubi, ubi->lookuptbl[ret]);
spin_unlock(&ubi->wl_lock); spin_unlock(&ubi->wl_lock);
} out:
return ret; return ret;
} }
...@@ -656,7 +668,7 @@ static struct ubi_wl_entry *get_peb_for_wl(struct ubi_device *ubi) ...@@ -656,7 +668,7 @@ static struct ubi_wl_entry *get_peb_for_wl(struct ubi_device *ubi)
struct ubi_fm_pool *pool = &ubi->fm_wl_pool; struct ubi_fm_pool *pool = &ubi->fm_wl_pool;
int pnum; int pnum;
if (pool->used == pool->size || !pool->size) { if (pool->used == pool->size) {
/* We cannot update the fastmap here because this /* We cannot update the fastmap here because this
* function is called in atomic context. * function is called in atomic context.
* Let's fail here and refill/update it as soon as possible. */ * Let's fail here and refill/update it as soon as possible. */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment