linux-kernel - Re: [PATCH smp 2/2] kernel/smp: Provide CSD lock timeout diagnostics

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: <20200724181913.GA25015@paulmck-ThinkPad-P72>
Date:   Fri, 24 Jul 2020 11:19:13 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     linux-kernel@...r.kernel.org, mingo@...nel.org, tglx@...utronix.de,
        bigeasy@...utronix.de, frederic@...nel.org
Subject: Re: [PATCH smp 2/2] kernel/smp: Provide CSD lock timeout diagnostics

On Thu, Jul 23, 2020 at 03:50:57PM -0700, Paul E. McKenney wrote:
> On Thu, Jul 23, 2020 at 03:55:56PM +0200, Peter Zijlstra wrote:

[ . . . ]

> > > +	ts2 = sched_clock();
> > > +	ts_delta = ts2 - *ts1;
> > > +	if (likely(ts_delta <= CSD_LOCK_TIMEOUT)) {
> > > +		cpu_relax();
> > > +		return false;
> > > +	}
> > > +
> > > +	firsttime = !*bug_id;
> > > +	if (firsttime)
> > > +		*bug_id = atomic_inc_return(&csd_bug_count);
> > > +	cpu = csd_lock_wait_getcpu(csd);
> > > +	smp_mb(); // No stale cur_csd values!
> > 
> > > +	if (WARN_ONCE(cpu < 0 || cpu >= nr_cpu_ids, "%s: cpu = %d\n", __func__, cpu))
> > > +		cpu_cur_csd = READ_ONCE(per_cpu(cur_csd, 0));
> > > +	else
> > > +		cpu_cur_csd = READ_ONCE(per_cpu(cur_csd, cpu));
> > 
> > This is a potential user-after-free, func() may free the csd when async.
> > Although I don't believe anybody does so.
> 
> Huh.  This will require some thought.  The one that the CPU is currently
> executing is available, so this is a problem only when they stack up.

Bah.  This is exactly why the cur_csd_func and cur_csd_info per-CPU
variables exist, and so I just need to actually use them.  :-/

							Thanx, Paul