netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: David Laight <David.Laight@ACULAB.COM>
Cc: rostedt <rostedt@goodmis.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Paul <paulmck@linux.vnet.ibm.com>,
	Josh Triplett <josh@joshtriplett.org>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	"Joel Fernandes, Google" <joel@joelfernandes.org>,
	Pablo Neira Ayuso <pablo@netfilter.org>,
	Jozsef Kadlecsik <kadlec@netfilter.org>,
	Florian Westphal <fw@strlen.de>,
	"David S. Miller" <davem@davemloft.net>,
	Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
	David Ahern <dsahern@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>, rcu <rcu@vger.kernel.org>,
	netfilter-devel <netfilter-devel@vger.kernel.org>,
	coreteam <coreteam@netfilter.org>,
	netdev <netdev@vger.kernel.org>
Subject: Re: [RFC][PATCH] rcu: Use typeof(p) instead of typeof(*p) *
Date: Tue, 12 Oct 2021 10:18:03 -0400 (EDT)	[thread overview]
Message-ID: <364516086.11428.1634048283470.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <4dbff8032f874a6f921ba0555c94eeaf@AcuMS.aculab.com>

----- On Oct 11, 2021, at 4:39 AM, David Laight David.Laight@ACULAB.COM wrote:

> From: Mathieu Desnoyers
>> Sent: 05 October 2021 16:15
>> 
>> ----- On Oct 5, 2021, at 9:47 AM, rostedt rostedt@goodmis.org wrote:
>> [...]
>> > #define rcu_dereference_raw(p) \
>> > ({ \
>> > 	/* Dependency order vs. p above. */ \
>> > 	typeof(p) ________p1 = READ_ONCE(p); \
>> > -	((typeof(*p) __force __kernel *)(________p1)); \
>> > +	((typeof(p) __force __kernel)(________p1)); \
>> > })
>> 
>> AFAIU doing so removes validation that @p is indeed a pointer, so a user might
>> mistakenly
>> try to use rcu_dereference() on an integer, and get away with it. I'm not sure
>> we want to
>> loosen this check. I wonder if there might be another way to achieve the same
>> check without
>> requiring the structure to be declared, e.g. with __builtin_types_compatible_p ?
> 
> Could you pass the pointer to something like:
> static __always_inline void foo(void *arg) {};
> 
> That would fail for integers.
> Not sure whether CFI bleats about function pointers though.
> 

That would indeed validate that a pointer is being passed to rcu_dereference()
and RCU_INITIALIZER().

However it would not solve this other issue: in Steven's patch, rcu_dereference_raw
is changed like so:

 #define rcu_dereference_raw(p) \
 ({ \
         /* Dependency order vs. p above. */ \
         typeof(p) ________p1 = READ_ONCE(p); \
-        ((typeof(*p) __force __kernel *)(________p1)); \
+        ((typeof(p) __force __kernel)(________p1)); \
 })

and AFAIU the __force __kernel attributes end up applying to the pointer rather than the
object pointed to, which changes the semantic.

So checking the pointer argument is not the only issue here.

As Linus pointed out, it might indeed be simpler to just keep declaring the structure in
public headers.

Thanks,

Mathieu

>	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
> UK
> Registration No: 1397386 (Wales)

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

  reply	other threads:[~2021-10-12 14:18 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-05 13:47 [RFC][PATCH] rcu: Use typeof(p) instead of typeof(*p) * Steven Rostedt
2021-10-05 15:15 ` Mathieu Desnoyers
2021-10-05 15:58   ` Steven Rostedt
2021-10-05 16:15     ` Mathieu Desnoyers
2021-10-05 16:29       ` Paul E. McKenney
2021-10-05 16:40       ` Steven Rostedt
2021-10-11  8:39   ` David Laight
2021-10-12 14:18     ` Mathieu Desnoyers [this message]
2021-10-12 15:36       ` Steven Rostedt
2021-10-05 16:18 ` Linus Torvalds
2021-10-05 16:37   ` Steven Rostedt
2021-10-05 16:47     ` Linus Torvalds
2021-10-05 16:42   ` Steven Rostedt
2021-10-05 18:01 ` Rasmus Villemoes
2021-10-05 18:06   ` Mathieu Desnoyers
2021-10-05 18:28     ` Jan Engelhardt
2021-10-05 18:40       ` Steven Rostedt
2021-10-05 19:06         ` Jan Engelhardt
2021-10-05 19:40           ` Steven Rostedt
2021-10-05 19:46             ` Linus Torvalds
2021-10-05 20:02               ` Steven Rostedt
2021-10-05 19:49             ` Mathieu Desnoyers
2021-10-05 20:06               ` Steven Rostedt
2021-10-05 20:37             ` Steven Rostedt
2021-10-05 20:45               ` Linus Torvalds
2021-10-05 21:05                 ` Steven Rostedt
2021-10-05 21:09               ` Jan Engelhardt
2021-10-05 21:24                 ` Steven Rostedt
2021-10-11  8:34                   ` David Laight
2021-10-05 21:27                 ` Linus Torvalds
2021-10-05 22:26                   ` Jan Engelhardt
2021-10-05 18:48     ` Rasmus Villemoes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=364516086.11428.1634048283470.JavaMail.zimbra@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=coreteam@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=fw@strlen.de \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=kadlec@netfilter.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    --cc=yoshfuji@linux-ipv6.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).