Messages in this thread | | | From | Richard Weinberger <> | Date | Sun, 2 Aug 2020 23:25:21 +0200 | Subject | Re: [PATCH] ubi: check kthread_should_stop() after the setting of task state |
| |
On Mon, Jun 1, 2020 at 11:13 AM Zhihao Cheng <chengzhihao1@huawei.com> wrote: > > A detach hung is possible when a race occurs between the detach process > and the ubi background thread. The following sequences outline the race: > > ubi thread: if (list_empty(&ubi->works)... > > ubi detach: set_bit(KTHREAD_SHOULD_STOP, &kthread->flags) > => by kthread_stop() > wake_up_process() > => ubi thread is still running, so 0 is returned > > ubi thread: set_current_state(TASK_INTERRUPTIBLE) > schedule() > => ubi thread will never be scheduled again > > ubi detach: wait_for_completion() > => hung task! > > To fix that, we need to check kthread_should_stop() after we set the > task state, so the ubi thread will either see the stop bit and exit or > the task state is reset to runnable such that it isn't scheduled out > indefinitely. > > Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com> > Cc: <Stable@vger.kernel.org> > Fixes: 801c135ce73d5df1ca ("UBI: Unsorted Block Images") > Reported-by: syzbot+853639d0cb16c31c7a14@syzkaller.appspotmail.com > --- > drivers/mtd/ubi/wl.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c > index 5146cce5fe32..a4d4343053d7 100644 > --- a/drivers/mtd/ubi/wl.c > +++ b/drivers/mtd/ubi/wl.c > @@ -1628,6 +1628,19 @@ int ubi_thread(void *u) > !ubi->thread_enabled || ubi_dbg_is_bgt_disabled(ubi)) { > set_current_state(TASK_INTERRUPTIBLE); > spin_unlock(&ubi->wl_lock); > + > + /* > + * Check kthread_should_stop() after we set the task > + * state to guarantee that we either see the stop bit > + * and exit or the task state is reset to runnable such > + * that it's not scheduled out indefinitely and detects > + * the stop bit at kthread_should_stop(). > + */ > + if (kthread_should_stop()) { > + set_current_state(TASK_RUNNING); > + break; > + } > +
Hmm, I see the problem but I fear this patch does not cure the race completely. It just lowers the chance to hit it. What if KTHREAD_SHOULD_STOP is set right after you checked for it?
> schedule(); > continue; > }
-- Thanks, //richard
| |