Re: [PATCH 2/2] workqueue: fix a possible race condition between rescuer and pwq-release

[Lai Jiangshan]

On Fri, Apr 18, 2014 at 11:06 PM, Tejun Heo <tj@xxxxxxxxxx> wrote:
> Hello,
>
> Applied to wq/for-3.15-fixes with put_pwq() relocated and patch
> description and comments updated.
>
> Thanks.
> -------- 8< --------
> From 4b81955722abe4306096c7b0137e0491a7ba7b0e Mon Sep 17 00:00:00 2001
> From: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
> Date: Fri, 18 Apr 2014 11:04:16 -0400
>
> There is a race condition between rescuer_thread() and
> pwq_unbound_release_workfn().
>
> Even after a pwq is scheduled for rescue, the associated work items
> may be consumed by any worker.  If all of them are consumed before the
> rescuer gets to them and the pwq's base ref was put due to attribute
> change, the pwq may be released while still being linked on
> @wq->maydays list making the rescuer dereference already freed pwq
> later.
>
> Make send_mayday() pin the target pwq until the rescuer is done with
> it.
>
> tj: Updated comment and patch description.  Moved put_pwq() to after
>     the last @pwq->pool access.  This isn't strictly necessarily for
>     correctness but is cleaner as the pool is accessed through the
>     pwq.
>
> Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
> Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx # v3.10+
> ---
>  kernel/workqueue.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 6ba0c60..a6532f2 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1916,6 +1916,12 @@ static void send_mayday(struct work_struct *work)
>
>         /* mayday mayday mayday */
>         if (list_empty(&pwq->mayday_node)) {
> +               /*
> +                * If @pwq is for an unbound wq, its base ref may be put at
> +                * any time due to an attribute change.  Pin @pwq until the
> +                * rescuer is done with it.
> +                */
> +               get_pwq(pwq);
>                 list_add_tail(&pwq->mayday_node, &wq->maydays);
>                 wake_up_process(wq->rescuer->task);
>         }
> @@ -2459,6 +2465,7 @@ repeat:
>
>                 rescuer->pool = NULL;
>                 spin_unlock(&pool->lock);
> +               put_pwq(pwq);   /* put the ref from send_mayday() */

put_pwq() requires pool->lock held.

>                 spin_lock(&wq_mayday_lock);
>         }
>
> --
> 1.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/