netdev - [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: <20250206180551.1716413-2-sreedevi.joshi@intel.com>
Date: Thu,  6 Feb 2025 12:05:51 -0600
From: "sreedevi.joshi" <joshisre@...mtp.an.intel.com>
To: edumazet@...il.com,
	kuba@...nel.org,
	pabeni@...hat.com,
	horms@...nel.org,
	ast@...nel.org,
	daniel@...earbox.net
Cc: magnus.karlsson@...el.com,
	maciej.fijalkowski@...el.com,
	hawk@...nel.org,
	john.fastabend@...il.com,
	almasrymina@...gle.com,
	asml.silence@...il.com,
	lorenzo@...nel.org,
	aleksander.lobakin@...el.com,
	chopps@...n.net,
	bigeasy@...utronix.de,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	bpf@...r.kernel.org,
	Sreedevi Joshi <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