netdev - Re: [bpf, xdp] headroom - was: Re: Question about to KMSAN: uninit-value in can

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: <fac5da75-2fc0-464c-be90-34220313af64@hartkopp.net>
Date: Mon, 5 Jan 2026 14:47:08 +0100
From: Oliver Hartkopp <socketcan@...tkopp.net>
To: Jakub Kicinski <kuba@...nel.org>, mkl@...gutronix.de,
 Prithvi <activprithvi@...il.com>
Cc: andrii@...nel.org, linux-can@...r.kernel.org,
 linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com,
 netdev@...r.kernel.org
Subject: Re: [bpf, xdp] headroom - was: Re: Question about to KMSAN:
 uninit-value in can_receive

Hello Jakub, all,

On 04.01.26 16:42, Jakub Kicinski wrote:
> On Sat, 3 Jan 2026 13:20:34 +0100 Oliver Hartkopp wrote:

>>
>> When the skb headroom is not safe to be used we need to be able to
>> identify and solve it.
> 
> Ugh, I should have looked at the report. The struct can_skb_priv
> business is highly unconventional for the networking stack.
> Would it be possible to kmalloc() this info and pass it to the socket
> via shinfo->destructor_arg?

I did some more code investigation about struct skb_shared_info which 
aims to be "invariant across clones".

Our struct can_skb_priv does the same and looks like this:

/**
  * struct can_skb_priv - private additional data inside CAN sk_buffs
  * @ifindex:	ifindex of the first interface the CAN frame appeared on
  * @skbcnt:	atomic counter to have an unique id together with skb pointer
  * @frame_len:	length of CAN frame in data link layer
  * @cf:		align to the following CAN frame at skb->data
  */
struct can_skb_priv {
	int ifindex;
	int skbcnt;
	unsigned int frame_len;
	struct can_frame cf[];
};

Where ifindex and skbcnt needs to be invariant across clones.

The frame_len is some intelligent length value caching which might be 
solved differently.

As the skbcnt is used as some incremented identifier to identify the CAN 
skb in the receive path with RPS, we might use the existing skb->hash 
space for it.

For the ifindex I would propose to store it in struct skb_shared_info:

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 86737076101d..f7233b8f461c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -604,10 +604,15 @@ struct skb_shared_info {
                 struct xsk_tx_metadata_compl xsk_meta;
         };
         unsigned int    gso_type;
         u32             tskey;

+#if IS_ENABLED(CONFIG_CAN)
+       /* initial CAN iif to avoid routing back to it (can-gw) */
+       int can_iif;
+#endif
+
         /*
          * Warning : all fields before dataref are cleared in __alloc_skb()
          */
         atomic_t        dataref;

Would this be a suitable approach to get rid of struct can_skb_priv in 
your opinion?

If so I would send three RFC patches:

- remove the need for can_skb_priv::frame_len
- make use of skb->hash instead of can_skb_priv::skbcnt
- move can_skb_priv:ifindex to skb_shared_info::can_iif

Which finally removes struct can_skb_priv and the highly unconventional 
skb->head construction.

Best regards,
Oliver