[PATCH] netlink: use 48 byte ctx instead of 6 signed longs for callback

[PATCH] netlink: use 48 byte ctx instead of 6 signed longs for callback

[Jason A. Donenfeld]

People are inclined to stuff random things into cb->args[n] because it
looks like an array of integers. Sometimes people even put u64s in there
with comments noting that a certain member takes up two slots. The
horror! Really this should mirror the usage of skb->cb, which are just
48 opaque bytes suitable for casting a struct. Then people can create
their usual casting macros for accessing strongly typed members of a
struct.

As a plus, this also gives us the same amount of space on 32bit and 64bit.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
---
 include/linux/netlink.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 593d1b9c33a8..205fa7b1f07a 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -192,7 +192,14 @@ struct netlink_callback {
 	bool			strict_check;
 	u16			answer_flags;
 	unsigned int		prev_seq, seq;
-	long			args[6];
+	union {
+		u8		ctx[48];
+
+		/* args is deprecated. Cast a struct over ctx instead
+		 * for proper type safety.
+		 */
+		long		args[6];
+	};
 };
 
 struct netlink_notify {
-- 
2.21.0

Re: [PATCH] netlink: use 48 byte ctx instead of 6 signed longs for callback

[Johannes Berg]

On Fri, 2019-06-28 at 16:40 +0200, Jason A. Donenfeld wrote:
> People are inclined to stuff random things into cb->args[n] because it
> looks like an array of integers. Sometimes people even put u64s in there
> with comments noting that a certain member takes up two slots. The
> horror! Really this should mirror the usage of skb->cb, which are just
> 48 opaque bytes suitable for casting a struct. Then people can create
> their usual casting macros for accessing strongly typed members of a
> struct.
> 
> As a plus, this also gives us the same amount of space on 32bit and 64bit.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Cc: Johannes Berg <johannes@sipsolutions.net>

Reviewed-by: Johannes Berg <johannes@sipsolutions.net>

I think this makes a lot of sense - we've got a mess here in many
places, e.g. look at struct nl80211_dump_wiphy_state in nl82011.c, I
think that could fit into the ctx[] since those don't all need to be
'long' (int or even shorter would be OK), we just want many more fields
and somehow it didn't occur to me to cast that "long args[]" array to
another struct ...

Thanks for doing this!

johannes

Re: [PATCH] netlink: use 48 byte ctx instead of 6 signed longs for callback

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 28 Jun 2019 16:42:26 +0200

> On Fri, 2019-06-28 at 16:40 +0200, Jason A. Donenfeld wrote:
>> People are inclined to stuff random things into cb->args[n] because it
>> looks like an array of integers. Sometimes people even put u64s in there
>> with comments noting that a certain member takes up two slots. The
>> horror! Really this should mirror the usage of skb->cb, which are just
>> 48 opaque bytes suitable for casting a struct. Then people can create
>> their usual casting macros for accessing strongly typed members of a
>> struct.
>> 
>> As a plus, this also gives us the same amount of space on 32bit and 64bit.
>> 
>> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
>> Cc: Johannes Berg <johannes@sipsolutions.net>
> 
> Reviewed-by: Johannes Berg <johannes@sipsolutions.net>

Applied to net-next.