netdev - RE: [RFC PATCH net 1/1] net: check transport

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] [day] [month] [year] [list]

Message-ID: <IA1PR11MB6289CFC1875FB14C00F31FB689F12@IA1PR11MB6289.namprd11.prod.outlook.com>
Date: Fri, 7 Feb 2025 14:30:11 +0000
From: "Joshi, Sreedevi" <sreedevi.joshi@...el.com>
To: sreedevi.joshi <joshisre@...mtp.an.intel.com>, "edumazet@...il.com"
	<edumazet@...il.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "horms@...nel.org"
	<horms@...nel.org>, "ast@...nel.org" <ast@...nel.org>, "daniel@...earbox.net"
	<daniel@...earbox.net>
CC: "Karlsson, Magnus" <magnus.karlsson@...el.com>, "Fijalkowski, Maciej"
	<maciej.fijalkowski@...el.com>, "hawk@...nel.org" <hawk@...nel.org>,
	"john.fastabend@...il.com" <john.fastabend@...il.com>,
	"almasrymina@...gle.com" <almasrymina@...gle.com>, "asml.silence@...il.com"
	<asml.silence@...il.com>, "lorenzo@...nel.org" <lorenzo@...nel.org>,
	"Lobakin, Aleksander" <aleksander.lobakin@...el.com>, "chopps@...n.net"
	<chopps@...n.net>, "bigeasy@...utronix.de" <bigeasy@...utronix.de>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"bpf@...r.kernel.org" <bpf@...r.kernel.org>
Subject: RE: [RFC PATCH net 1/1] net: check transport_header before adding
 offset

> -----Original Message-----
> From: sreedevi.joshi <joshisre@...mtp.an.intel.com>
> Sent: Thursday, February 6, 2025 1:06 PM
> To: edumazet@...il.com; kuba@...nel.org; pabeni@...hat.com; horms@...nel.org; ast@...nel.org; daniel@...earbox.net
> Cc: Karlsson, Magnus <magnus.karlsson@...el.com>; Fijalkowski, Maciej <maciej.fijalkowski@...el.com>; hawk@...nel.org;
> john.fastabend@...il.com; almasrymina@...gle.com; asml.silence@...il.com; lorenzo@...nel.org; Lobakin, Aleksander
> <aleksander.lobakin@...el.com>; chopps@...n.net; bigeasy@...utronix.de; netdev@...r.kernel.org; linux-kernel@...r.kernel.org;
> bpf@...r.kernel.org; Joshi, Sreedevi <sreedevi.joshi@...el.com>
> Subject: [RFC PATCH net 1/1] net: check transport_header before adding offset
> 
> From: Sreedevi Joshi <sreedevi.joshi@...el.com>
> 
> skb_headers_offset_update() adds offset to the transport_header
> of skb without checking if it was set. When the transport header
> is not set, it's value is 65535. Adding offset to this causes it to
> roll over and makes the transport_header value to be less than
> network_header.
> When a tc ingress hook is attached and it invokes bpf_skb_change_tail()
> (to strip off extra bytes at the end of packet or to attach some
> extra bytes), the logic in __bpf_skb_change_tail() that calculates
> the min_len fails due to the transport_header being incorrectly set.
> 
> This issue was discovered when testing with veth interface with both xdp and
> tc ingress hooks are attached. veth_convert_skb_to_xdp_buff() calls
> skb_pp_cow_data() and it results in this function being called. Since
> transport_header is incremented without checking, it results in the condition
> where transport_header < network_header. __netif_receive_skb_core() when it
> receives this skb, skips reset of the transport header as it is already set.
> 
> This is specific to XDP path. When there is no XDP hook, the logic takes a
> different route (__netif_rx()) and the reset of the transport header happens in
> __netif_receive_skb_core() before it reaches tc ingress hook.
> 
> Fixes: f5b1729443fd ("net: Add skb_headers_offset_update helper function.")
> Signed-off-by: Sreedevi Joshi <sreedevi.joshi@...el.com>
> ---
>  net/core/skbuff.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index a441613a1e6c..79b10abd95f1 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2098,7 +2098,8 @@ void skb_headers_offset_update(struct sk_buff *skb, int off)
>  	if (skb->ip_summed == CHECKSUM_PARTIAL)
>  		skb->csum_start += off;
>  	/* {transport,network,mac}_header and tail are relative to skb->head */
> -	skb->transport_header += off;
> +	if (skb_transport_header_was_set(skb))
> +		skb->transport_header += off;
>  	skb->network_header   += off;
>  	if (skb_mac_header_was_set(skb))
>  		skb->mac_header += off;
> --
> 2.25.1

[] resending due to mail server issues.