linux-kernel - Re: [PATCH v23 2/9] sched: Fix modifying donor->blocked on without proper locking

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANDhNCqHd5tHC_N2unYyhH3nuiq_b23WKEwHzpOWizWzCfT9JA@mail.gmail.com>
Date: Thu, 30 Oct 2025 16:42:35 -0700
From: John Stultz <jstultz@...gle.com>
To: K Prateek Nayak <kprateek.nayak@....com>
Cc: LKML <linux-kernel@...r.kernel.org>, Joel Fernandes <joelagnelf@...dia.com>, 
	Qais Yousef <qyousef@...alina.io>, Ingo Molnar <mingo@...hat.com>, 
	Peter Zijlstra <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>, 
	Vincent Guittot <vincent.guittot@...aro.org>, Dietmar Eggemann <dietmar.eggemann@....com>, 
	Valentin Schneider <vschneid@...hat.com>, Steven Rostedt <rostedt@...dmis.org>, 
	Ben Segall <bsegall@...gle.com>, Zimuzo Ezeozue <zezeozue@...gle.com>, Mel Gorman <mgorman@...e.de>, 
	Will Deacon <will@...nel.org>, Waiman Long <longman@...hat.com>, Boqun Feng <boqun.feng@...il.com>, 
	"Paul E. McKenney" <paulmck@...nel.org>, Metin Kaya <Metin.Kaya@....com>, 
	Xuewen Yan <xuewen.yan94@...il.com>, Thomas Gleixner <tglx@...utronix.de>, 
	Daniel Lezcano <daniel.lezcano@...aro.org>, Suleiman Souhlal <suleiman@...gle.com>, 
	kuyo chang <kuyo.chang@...iatek.com>, hupu <hupu.gm@...il.com>, kernel-team@...roid.com
Subject: Re: [PATCH v23 2/9] sched: Fix modifying donor->blocked on without
 proper locking

On Wed, Oct 29, 2025 at 9:51 PM K Prateek Nayak <kprateek.nayak@....com> wrote:
> On 10/30/2025 5:48 AM, John Stultz wrote:
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 517b26c515bc5..0533a14ce5935 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -6591,7 +6591,7 @@ static struct task_struct *proxy_deactivate(struct rq *rq, struct task_struct *d
> >                * as unblocked, as we aren't doing proxy-migrations
> >                * yet (more logic will be needed then).
> >                */
> > -             donor->blocked_on = NULL;
> > +             clear_task_blocked_on(donor, NULL);
>
> nit. You can probably switch this to use __clear_task_blocked_on() in
> the previous patch and then to the clear_task_blocked_on() variant here.
> It makes it more clear that proxy_deactivate() is now out of the
> "donor->blocked_lock" critical section.

The gotcha there is the __clear_task_blocked_on() will assert if we
don't hold the right lock. So this patch is sort of fixing it up to
allow for proper locking without cheating here.


> Either way, no strong feelings.
>
> >       }
> >       return NULL;
> >  }
> > @@ -6619,6 +6619,7 @@ find_proxy_task(struct rq *rq, struct task_struct *donor, struct rq_flags *rf)
> >       int this_cpu = cpu_of(rq);
> >       struct task_struct *p;
> >       struct mutex *mutex;
> > +     enum { FOUND, DEACTIVATE_DONOR } action = FOUND;
>
> nit. If you move that declaration to the top, you can preserve the nice
> reverse xmas arrangement ;)

Yeah, I meant to do that, but just overlooked it. Thanks for pointing it out.


> Apart from those couple of nits, feel free to include:
>
> Reviewed-by: K Prateek Nayak <kprateek.nayak@....com>

As always, greatly appreciate your time for the review and feedback here!
Thank you!
-john