netdev - Re: [net-next 0/6] move CAN skb headroom content to skb extensions

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: <aXoMqaA7b2CqJZNA@strlen.de>
Date: Wed, 28 Jan 2026 14:18:33 +0100
From: Florian Westphal <fw@...len.de>
To: Oliver Hartkopp <socketcan@...tkopp.net>
Cc: Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
	linux-can@...r.kernel.org
Subject: Re: [net-next 0/6] move CAN skb headroom content to skb extensions

Oliver Hartkopp <socketcan@...tkopp.net> wrote:
> > When the first extensions were added all of them could be enabled
> > at same time, but I think that has changed.
> 
> IMO we do not need to 'union' extensions as long as automatic enum 
> calculation does it job with the enabled Kconfig options.
>
> My only concern would be distribution kernels that have an all-yes 
> config policy ;-)

Well, thats the norm, no?  Allmodconfig.
So we have two issues:
1. Turn active_extensions in sk_buff into u16
2. Growing memory usage of the skb_ext blob.

No need to add this 'union' now of course, but I think its something
that should be kept in mind: originally, all extensions could be
turned on for the same skb, but it looks like we now have mutually
exclusive ones.

> With my patch set the enum would now look like this:
> 
> #ifdef CONFIG_SKB_EXTENSIONS
> enum skb_ext_id {
> #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
>          SKB_EXT_BRIDGE_NF,
> #endif
> #ifdef CONFIG_XFRM
>          SKB_EXT_SEC_PATH,
> #endif
> #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
>          TC_SKB_EXT,
> #endif
> #if IS_ENABLED(CONFIG_MPTCP)
>          SKB_EXT_MPTCP,
> #endif
> #if IS_ENABLED(CONFIG_MCTP_FLOWS)
>          SKB_EXT_MCTP,
> #endif
> #if IS_ENABLED(CONFIG_INET_PSP)
>          SKB_EXT_PSP,
> #endif
> #if IS_ENABLED(CONFIG_CAN)
>          SKB_EXT_CAN,
> #endif
>          SKB_EXT_NUM, /* must be last */
> };
> 
> => SKB_EXT_NUM is then 7
> 
> When we (correctly) add another extension, SKB_EXT_NUM would become 8 
> which is still fine IMO. But then the BUILD_BUG_ON check in 
> skb_extensions_init() would need the below fix, right?
> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 648c20e19038..609851d70173 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -5156,11 +5156,11 @@ static __always_inline unsigned int 
> skb_ext_total_length(void)
>          return l;
>   }
> 
>   static void skb_extensions_init(void)
>   {
> -       BUILD_BUG_ON(SKB_EXT_NUM >= 8);
> +       BUILD_BUG_ON(SKB_EXT_NUM > 8);

True, the last valid extension id can't be 8, but
SKB_EXT_NUM could be.

> Should I send a proper patch?

You can, otoh I think we should have consensus on what
to do when the 8th extension is added, do we just s/u8/u16' and
eat the growing memory cost for the skb_ext growth, or do we go
to a drawing board and figure out how to best merge mutually exclusive
extensions to save space?