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: <2af792de-77a0-4f77-a6b8-f207089b94b6@hartkopp.net>
Date: Wed, 7 Jan 2026 20:10:57 +0100
From: Oliver Hartkopp <socketcan@...tkopp.net>
To: Jakub Kicinski <kuba@...nel.org>
Cc: mkl@...gutronix.de, Prithvi <activprithvi@...il.com>, 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

Sorry for answering myself:

The below idea using skb->cb definitely does not work :-/

But as we never use encapsulation in CAN skbs we can use the 
inner_protocol and inner_xxx_header space when skb->encapsulation is false:

	union {
		/* encapsulation == true */
		struct {
			union {
				__be16		inner_protocol;
				__u8		inner_ipproto;
			};

			__u16			inner_transport_header;
			__u16			inner_network_header;
			__u16			inner_mac_header;
		};
		/* encapsulation == false */
		struct {
			int can_iif;
			__u16 can_frame_len;
		};
	};


Best regards,
Oliver

On 07.01.26 16:34, Oliver Hartkopp wrote:
> Hello Jakub,
> 
> On 07.01.26 01:23, Jakub Kicinski wrote:
>> On Tue, 6 Jan 2026 13:04:41 +0100 Oliver Hartkopp wrote:
>>> When such skb is echo'ed back after successful transmission via
>>> netif_rx() this leads to skb->skb_iif = skb->dev->ifindex;
>>>
>>> To prevent a loopback the CAN frame must not be sent back to the
>>> originating interface - even when it has been routed to different CAN
>>> interfaces in the meantime (which always overwrites skb_iif).
>>>
>>> Therefore we need to maintain the "real original" incoming interface.
>>
>> Alternatively perhaps for this particular use case you could use
>> something like metadata_dst to mark the frame as forwarded / annotate
>> with the originating ifindex?
> 
> I looked into it and the way how skb_dst is shared in the union behind 
> cb[] does not look very promising for skbs that wander up and down in 
> the network layer. And it is pretty complex to just store a single 
> interface index integer value.
> 
> While looking into _sk_redir to see how the _skb_refdst union is used, 
> I've seen that the _sk_redir function was removed from struct tcp_skb_cb 
> (commit e3526bb92a208).
> 
> Today we use skb->cb only for passing (address) information from the 
> network layer to the socket layer and user space. But the space in cb[] 
> could also hold the content we currently store in the problematic skb 
> headroom.
> 
> Would using skb->cb be a good approach for CAN skbs (that do not have 
> any of the Ethernet/TCP/IP requirements/features) or will there still be 
> networking code (besides CAN drivers and CAN network layer) that writes 
> into cb[] when passing the CAN skb up and down in the stack?
> 
> /**
>   * struct can_skb_cb - private data inside CAN skb->cb
>   * cb[] is 64 bit aligned which is also recommended for struct 
> sockaddr_can
>   * @magic:    to check if someone wrote to our CAN skb->cb space
>   * @flags:    extra flags for CAN_RAW and CAN_BCM sockets
>   * @can_addr:    socket address information to userspace
>   * @can_iif:    ifindex of the first interface the CAN frame appeared on
>   * @skbcnt:    atomic counter to have an unique id together with skb 
> pointer
>   * @frame_len:    bql length cache of CAN frame in data link layer
>   */
> struct can_skb_cb {
>      u32 magic;
>      u32 flags;
>      struct sockaddr_can can_addr;
>      int can_iif;
>      int skbcnt;
>      unsigned int frame_len;
> };
> 
> If not: We also don't have vlans nor inner[protocol|headers] in CAN 
> where we might store the 4 byte can_iif integer ...
> 
> Many thanks and best regards,
> Oliver