linux-kernel - Re: [PATCH 1/2] compiler.h: Introduce ptr

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: <bd93a57c-662f-470e-8ba4-509f27eada6d@efficios.com>
Date: Thu, 3 Oct 2024 09:23:31 -0400
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: 'Alan Stern' <stern@...land.harvard.edu>,
 David Laight <David.Laight@...lab.com>
Cc: Jonas Oberhauser <jonas.oberhauser@...weicloud.com>,
 Linus Torvalds <torvalds@...ux-foundation.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
 "Paul E. McKenney" <paulmck@...nel.org>, Will Deacon <will@...nel.org>,
 Peter Zijlstra <peterz@...radead.org>, Boqun Feng <boqun.feng@...il.com>,
 John Stultz <jstultz@...gle.com>, Neeraj Upadhyay <Neeraj.Upadhyay@....com>,
 Frederic Weisbecker <frederic@...nel.org>,
 Joel Fernandes <joel@...lfernandes.org>,
 Josh Triplett <josh@...htriplett.org>, Uladzislau Rezki <urezki@...il.com>,
 Steven Rostedt <rostedt@...dmis.org>, Lai Jiangshan
 <jiangshanlai@...il.com>, Zqiang <qiang.zhang1211@...il.com>,
 Ingo Molnar <mingo@...hat.com>, Waiman Long <longman@...hat.com>,
 Mark Rutland <mark.rutland@....com>, Thomas Gleixner <tglx@...utronix.de>,
 Vlastimil Babka <vbabka@...e.cz>,
 "maged.michael@...il.com" <maged.michael@...il.com>,
 Mateusz Guzik <mjguzik@...il.com>, Gary Guo <gary@...yguo.net>,
 "rcu@...r.kernel.org" <rcu@...r.kernel.org>,
 "linux-mm@...ck.org" <linux-mm@...ck.org>,
 "lkmm@...ts.linux.dev" <lkmm@...ts.linux.dev>
Subject: Re: [PATCH 1/2] compiler.h: Introduce ptr_eq() to preserve address
 dependency

On 2024-10-03 03:50, 'Alan Stern' wrote:
> On Wed, Oct 02, 2024 at 03:24:45PM +0000, David Laight wrote:
>> I think I know what you are trying to do, and you just fail.
>> Whether something can work is another matter, but that code
>> can't ever work.
>>
>> Inside if (a == b) the compiler will always use the same register
>> for references to a and b - because it knows they have the same value.
> 
> According to the other people in this discussion who have actually tried
> using this code, it _does_ work (at least some of the time).
> 
> However, I'm not one of those people and so I leave it up to them to
> decide how to respond to this critique.

I suspect that David's comment is about this specific example that
was given in this leg of the email thread:

https://lore.kernel.org/lkml/5d7d8a59-57f5-4125-95bb-fda9c193b9cf@huaweicloud.com/

> > > > > int fct_hide(void)
> > > > > > {
> > > > > >     int *a, *b;
> > > > > >
> > > > > >     do {
> > > > > >         a = READ_ONCE(p);
> > > > > >         asm volatile ("" : : : "memory");
> > > > > >         b = READ_ONCE(p);
> > > > > >     } while (a != b);
> > > > > >     OPTIMIZER_HIDE_VAR(b);
> > > > > >     return *b;
> > > > > > } 

This indeed cannot work because the hide var is done
on @b after it was compared with @a, so after the compiler
was free to use any of the registers due to the equality.

Another example that does *not* work is if we try to hide
vars on the inputs of the equality, and then proceed to do the
comparison on the resulting temporaries, e.g.:

int fct_hide(void)
{
      int *a, *b;

      do {
          a = READ_ONCE(p);
          asm volatile ("" : : : "memory");
          b = READ_ONCE(p);
      } while (OPTIMIZER_HIDE_VAR(a) != OPTIMIZER_HIDE_VAR(b));
      return *b;
}

The reason why this does *not* work is because the compiler is
free to use either temporaries for *b at the end, because they
were deemed identical.

What _does_ work however are the following two approaches:

1) Perform the equality check on the original variables, creating
new versions (with OPTIMIZER_HIDE_VAR) of both variables for the
rest of their use, therefore making sure the pointer dereference
are not derived from versions of the variables which were compared
with another pointer. (as suggested by Boqun)

2) Perform the equality check on the versions resulting of hiding
both variables, making sure those versions of the variables are
not dereferenced afterwards. (as suggested by Linus)

Thanks,

Mathieu

> 
> Alan
> 
>> Possibly something like:
>> 	c = b;
>> 	OPTIMISER_HIDE_VAR(c);
>> 	if (a == c) {
>> 		*b
>> will ensure that there isn't a speculative load from *a.
>> You'll get at least one register-register move - but they are safe.
>> Otherwise you'll need to put the condition inside an asm block.
>>
>> 	David

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com