lkml.org 
[lkml]   [2011]   [Aug]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 01/16] freezer: fix current->state restoration race in refrigerator()
Date
On Friday, August 19, 2011, Tejun Heo wrote:
> refrigerator() saves current->state before entering frozen state and
> restores it before returning using __set_current_state(); however,
> this is racy, for example, please consider the following sequence.
>
> set_current_state(TASK_INTERRUPTIBLE);
> try_to_sleep();

Did you mean try_to_freeze() here?

> if (kthread_should_stop())
> break;
> schedule();
>
> If kthread_stop() races with ->state restoration, the restoration can
> restore ->state to TASK_INTERRUPTIBLE after kthread_stop() sets it to
> TASK_RUNNING but kthread_should_stop() may still see zero
> ->should_stop because there's no memory barrier between restoring
> TASK_INTERRUPTIBLE and kthread_should_stop() test.
>
> This isn't restricted to kthread_should_stop(). current->state is
> often used in memory barrier based synchronization and silently
> restoring it w/o mb breaks them.
>
> Use set_current_state() instead.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---
> kernel/freezer.c | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/freezer.c b/kernel/freezer.c
> index 7b01de9..575f863 100644
> --- a/kernel/freezer.c
> +++ b/kernel/freezer.c
> @@ -58,7 +58,13 @@ void refrigerator(void)
> current->flags &= ~PF_FREEZING;
>
> pr_debug("%s left refrigerator\n", current->comm);
> - __set_current_state(save);
> +
> + /*
> + * Restore saved task state before returning. The mb'd version
> + * needs to be used; otherwise, it might silently break
> + * synchronization which depends on ordered task state change.
> + */
> + set_current_state(save);

I think the change is correct.

> }
> EXPORT_SYMBOL(refrigerator);

Thanks,
Rafael


\
 
 \ /
  Last update: 2011-08-19 23:09    [W:0.401 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site