| 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
| ||
|
Message-ID: <IA1PR11MB62899945E3D2DCED17259B1E89F12@IA1PR11MB6289.namprd11.prod.outlook.com>
Date: Fri, 7 Feb 2025 14:27:46 +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 0/1] transport_header set incorrectly when using
veth
> -----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 0/1] transport_header set incorrectly when using veth
>
> From: Sreedevi Joshi <sreedevi.joshi@...el.com>
>
> When testing a use-case on veth by attaching XDP and tc ingress hooks, it was noticed that the transport_header is set incorrectly and
> causes the tc_ingress hook that is using bpf_skb_change_tail() call to report a failure.
>
> Here is the flow:
> veth ingress:
> veth_convert_skb_to_xdp_buff()- [Example: skb->trannsport_header=65535 skb->network_header=0]
> ..>skb_pp_cow_data()
> ....>skb_headers_offset_update() - adds offset without checking and this
> results in transport_header value roll over.
> [off: 192: results in skb->transport_header = 191, skb->network_header=192] tc_ingress hook: bpf_skb_change_tail()
> - Since transport_header < network_header, min_len is negative and it fails.
>
> Two possbible solutions:
> option 1: introducing the check in the skb_headers_offset_update() to skip adding offset
> to transport_header when it is not set. (patch attached) option 2: reset transport header in veth_xdp_rcv_skb()
>
> Option 1 seems to be better as it will apply to any other interfaces that may use skb_headers_offset_update and there seems to
> similar logic in the same function to check if mac_header was set before adding offset.
>
> Seeking your inputs on this.
>
> NOTES:
> 1. If veth is used without XDP hook attached, this issue is not observed as the logic uses __netif_rx() directly and the transport header
> is reset in __netif_receive_skb_core() as it detects it is not set.
>
> 2. Tested on i40e driver and confirmed it does not have this issue as the
> skb_headers_offset_update() is not in the processing path.
>
>
> Instructions to reproduce the issue along with the XDP and tc ingress programs is attached below.
>
> -------------------------------8<-------------------------------
> instructions:
>
> #build XDP and tc programs
> clang -O2 -g -target bpf -D__TARGET_ARCH_x86 -c xdp_prog.c -o xdp_prog.o clang -O2 -g -target bpf -D__TARGET_ARCH_x86 -c
> tc_bpf_prog.c -o tc_bpf_prog.o
>
> # create the veth pair
> ip link add veth0 numtxqueues 1 numrxqueues 1 type veth peer name veth1 \
> numtxqueues 1 numrxqueues 1
>
> ip addr add 10.0.1.0/24 dev veth0
> ip addr add 10.0.1.1/24 dev veth1
> ip link set veth0 address 02:00:00:00:00:00 ip link set veth1 address 02:00:00:00:00:01 ip link set veth0 up ip link set veth1 up
>
> if [ -f /proc/net/if_inet6 ]; then
> echo 1 > /proc/sys/net/ipv6/conf/veth0/disable_ipv6
> echo 1 > /proc/sys/net/ipv6/conf/veth1/disable_ipv6
> fi
>
> #attach xdp hook and tc ingress hooks to veth1 xdp-loader load veth1 xdp_prog.o
>
> tc qdisc add dev veth1 clsact
> tc filter add dev veth1 ingress bpf da obj tc_bpf_prog.o sec prog
>
> # generate traffic from veth0 egress -> veth1 ingress ping -c e 10.0.1.3 -I veth0
>
> # observe the trace pipe (make sure tracing is on) # The following prints will appear
> # ping-5330 [072] ..s2. 18266.403464: bpf_trace_printk: Failure.. new len=52 ret=-22
> cat /sys/kernel/debug/tracing/trace_pipe
>
> -------------------------------8<-------------------------------
> xdp_prog.c:
>
> #include <linux/bpf.h>
> #include <bpf/bpf_helpers.h>
>
> SEC("xdp") int netd_xdp_prog(struct xdp_md *xdp) {
> /* Squash compiler warning. */
> (void)xdp;
>
> return XDP_PASS;
> }
>
> char _license[] SEC("license") = "GPL";
>
> -------------------------------8<-------------------------------
> test_bpf_prog.c:
>
> #include <linux/bpf.h>
> #include <bpf/bpf_helpers.h>
> #include <linux/pkt_cls.h>
>
> SEC("prog") int netd_tc_test_ingress(struct __sk_buff *skb)
> {
> long ret;
>
> /* extend skb length by 10 */
> ret = bpf_skb_change_tail(skb, skb->len + 10, 0);
> if (ret < 0) {
> bpf_printk("Failure.. new len=%d ret=%d\n", skb->len+10, ret);
> return TC_ACT_SHOT;
> }
>
> bpf_printk("Success new len:%d \n", skb->len+10);
>
> return TC_ACT_UNSPEC;
> }
>
> char _license[] SEC("license") = "GPL";
>
> -------------------------------8<-------------------------------
>
> Sreedevi Joshi (1):
> net: check transport_header before adding offset
>
> net/core/skbuff.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> --
> 2.25.1
[]
Apologies for resending. Mail server had some issues earlier and didn't reach some recipients.
Powered by blists - more mailing lists