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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 10 Aug 2021 10:26:15 +0100 From: Valentin Schneider <valentin.schneider@....com> To: Boqun Feng <boqun.feng@...il.com> Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, rcu@...r.kernel.org, linux-rt-users@...r.kernel.org, Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>, Ingo Molnar <mingo@...nel.org>, Peter Zijlstra <peterz@...radead.org>, Thomas Gleixner <tglx@...utronix.de>, Steven Rostedt <rostedt@...dmis.org>, Daniel Bristot de Oliveira <bristot@...hat.com>, Sebastian Andrzej Siewior <bigeasy@...utronix.de>, "Paul E. McKenney" <paulmck@...nel.org>, Frederic Weisbecker <frederic@...nel.org>, Josh Triplett <josh@...htriplett.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Davidlohr Bueso <dave@...olabs.net>, Lai Jiangshan <jiangshanlai@...il.com>, Joel Fernandes <joel@...lfernandes.org>, Anshuman Khandual <anshuman.khandual@....com>, Vincenzo Frascino <vincenzo.frascino@....com>, Steven Price <steven.price@....com>, Ard Biesheuvel <ardb@...nel.org> Subject: Re: [PATCH v2 2/4] sched: Introduce is_pcpu_safe() Hi, On 10/08/21 10:42, Boqun Feng wrote: > Hi, > > On Sat, Aug 07, 2021 at 01:58:05AM +0100, Valentin Schneider wrote: >> Some areas use preempt_disable() + preempt_enable() to safely access >> per-CPU data. The PREEMPT_RT folks have shown this can also be done by >> keeping preemption enabled and instead disabling migration (and acquiring a >> sleepable lock, if relevant). >> >> Introduce a helper which checks whether the current task can safely access >> per-CPU data, IOW if the task's context guarantees the accesses will target >> a single CPU. This accounts for preemption, CPU affinity, and migrate >> disable - note that the CPU affinity check also mandates the presence of >> PF_NO_SETAFFINITY, as otherwise userspace could concurrently render the >> upcoming per-CPU access(es) unsafe. >> >> Signed-off-by: Valentin Schneider <valentin.schneider@....com> >> --- >> include/linux/sched.h | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/include/linux/sched.h b/include/linux/sched.h >> index debc960f41e3..b77d65f677f6 100644 >> --- a/include/linux/sched.h >> +++ b/include/linux/sched.h >> @@ -1715,6 +1715,16 @@ static inline bool is_percpu_thread(void) >> #endif >> } >> >> +/* Is the current task guaranteed not to be migrated elsewhere? */ >> +static inline bool is_pcpu_safe(void) >> +{ >> +#ifdef CONFIG_SMP >> + return !preemptible() || is_percpu_thread() || current->migration_disabled; >> +#else >> + return true; >> +#endif >> +} > > I wonder whether the following can happen, say thread A is a worker > thread for CPU 1, so it has the flag PF_NO_SETAFFINITY set. > > { percpu variable X on CPU 2 is initially 0 } > > thread A > ======== > > <preemption enabled> > if (is_pcpu_safe()) { // nr_cpus_allowed == 1, so return true. > <preempted> > <hot unplug CPU 1> > unbinder_workers(1); // A->cpus_mask becomes cpu_possible_mask > <back to run on CPU 2> > __this_cpu_inc(X); > tmp = X; // tmp == 0 > <preempted> > <in thread B> > this_cpu_inc(X); // X becomes 1 > <back to run A on CPU 2> > X = tmp + 1; // race! > } > > if so, then is_percpu_thread() doesn't indicate is_pcpu_safe()? > You're absolutely right. migrate_disable() protects the thread against being migrated due to hotplug, but pure CPU affinity doesn't at all. kthread_is_per_cpu() doesn't work either, because parking is not the only approach to hotplug for those (e.g. per-CPU workqueue threads unbind themselves on hotplug, as in your example). One could hold cpus_read_lock(), but I don't see much point here. So that has to be return !preemptible() || current->migration_disabled; Thanks!
Powered by blists - more mailing lists