linux-kernel - Re: [PATCH 2/6] sched: Simplify migration_cpu

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]

Date:   Thu, 25 Feb 2021 09:45:52 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Valentin Schneider <valentin.schneider@....com>
Cc:     Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Mel Gorman <mgorman@...e.de>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        linux-kernel@...r.kernel.org, Andi Kleen <andi@...stfloor.org>
Subject: Re: [PATCH 2/6] sched: Simplify migration_cpu_stop()

On Wed, Feb 24, 2021 at 03:34:36PM +0000, Valentin Schneider wrote:
> On 24/02/21 13:24, Peter Zijlstra wrote:
> > @@ -1950,31 +1931,20 @@ static int migration_cpu_stop(void *data
> >                       goto out;
> >
> >               if (pending) {
> > -			p->migration_pending = NULL;
> > +			if (p->migration_pending == pending)
> > +				p->migration_pending = NULL;
> >                       complete = true;
> >               }
> >
> > -		/* migrate_enable() --  we must not race against SCA */
> > -		if (dest_cpu < 0) {
> > -			/*
> > -			 * When this was migrate_enable() but we no longer
> > -			 * have a @pending, a concurrent SCA 'fixed' things
> > -			 * and we should be valid again. Nothing to do.
> > -			 */
> > -			if (!pending) {
> > -				WARN_ON_ONCE(!cpumask_test_cpu(task_cpu(p), &p->cpus_mask));
> > -				goto out;
> > -			}
> > -
> 
> This is fixed by 5+6, but at this patch I think you can have double
> completions - I thought this was an issue, but briefly looking at
> completion stuff it might not. In any case, consider:
> 
>   task_cpu(p) == Y
> 
>   SCA(p, X);
>                  SCA(p, Y);
> 
> 
> SCA(p, Y) will uninstall SCA(p, X)'s pending and complete.
> 
> migration/Y kicked by SCA(p, X) will grab arg->pending, which is still
> SCA(p, X)'s pending and also complete.

Right, so I didn't really think too hard about the intermediate states,
given it's all pretty buggered until at least 5. But yeah, double
complete is harmless.

Specifically, the refcount the stopper has should avoid the stack from
getting released.