| Message ID | 1411375536-20067-3-git-send-email-richard@nod.at |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 2014-09-22 at 10:45 +0200, Richard Weinberger wrote: > If sync_erase() fails with EINTR, ENOMEM, EAGAIN or > EBUSY erase_worker() re-schedules the failed work. > This will lead to a deadlock because erase_worker() is called > with work_sem held in read mode. And schedule_erase() will take > this lock again. > > Signed-off-by: Richard Weinberger <richard@nod.at> Did you manage to test it? Why no -stable this time? Not that important, or just something theoretical and you never actually hit this bug? Thanks!
Am 26.09.2014 12:40, schrieb Artem Bityutskiy: > On Mon, 2014-09-22 at 10:45 +0200, Richard Weinberger wrote: >> If sync_erase() fails with EINTR, ENOMEM, EAGAIN or >> EBUSY erase_worker() re-schedules the failed work. >> This will lead to a deadlock because erase_worker() is called >> with work_sem held in read mode. And schedule_erase() will take >> this lock again. >> >> Signed-off-by: Richard Weinberger <richard@nod.at> > > Did you manage to test it? > > Why no -stable this time? Not that important, or just something > theoretical and you never actually hit this bug? It is something theoretical, I was only able to trigger it by injecting ENOMEM by hand. Thanks, //richard
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index 253ec9b..637ffff 100644 --- a/drivers/mtd/ubi/wl.c +++ b/drivers/mtd/ubi/wl.c @@ -1421,8 +1421,6 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk, { struct ubi_wl_entry *e = wl_wrk->e; int pnum = e->pnum; - int vol_id = wl_wrk->vol_id; - int lnum = wl_wrk->lnum; int err, available_consumed = 0; if (shutdown) { @@ -1459,21 +1457,15 @@ static int erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk, } ubi_err("failed to erase PEB %d, error %d", pnum, err); - kfree(wl_wrk); if (err == -EINTR || err == -ENOMEM || err == -EAGAIN || err == -EBUSY) { - int err1; - /* Re-schedule the LEB for erasure */ - err1 = schedule_erase(ubi, e, vol_id, lnum, 0); - if (err1) { - err = err1; - goto out_ro; - } + __schedule_ubi_work(ubi, wl_wrk); return err; } + kfree(wl_wrk); kmem_cache_free(ubi_wl_entry_slab, e); if (err != -EIO) /*
If sync_erase() fails with EINTR, ENOMEM, EAGAIN or EBUSY erase_worker() re-schedules the failed work. This will lead to a deadlock because erase_worker() is called with work_sem held in read mode. And schedule_erase() will take this lock again. Signed-off-by: Richard Weinberger <richard@nod.at> --- drivers/mtd/ubi/wl.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)