Re: [RFC][PATCH] rcu: Use typeof(p) instead of typeof(*p) *

[Mathieu Desnoyers]

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

> From: Mathieu Desnoyers
>> Sent: 05 October 2021 16:15
>> 
>> ----- On Oct 5, 2021, at 9:47 AM, rostedt rostedt@xxxxxxxxxxx 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