* Re: [PATCH net] vsock/virtio: fix MSG_ZEROCOPY pinned-pages accounting
From: Bobby Eshleman @ 2026-04-20 18:34 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Eric Dumazet, Simon Horman, kvm, Arseniy Krasnov,
David S. Miller, Paolo Abeni, Jakub Kicinski, Michael S. Tsirkin,
Jason Wang, virtualization, linux-kernel, Eugenio Pérez,
Xuan Zhuo, Stefan Hajnoczi, Yiming Qian
In-Reply-To: <20260420132051.217589-1-sgarzare@redhat.com>
On Mon, Apr 20, 2026 at 03:20:51PM +0200, Stefano Garzarella wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
>
> virtio_transport_init_zcopy_skb() uses iter->count as the size argument
> for msg_zerocopy_realloc(), which in turn passes it to
> mm_account_pinned_pages() for RLIMIT_MEMLOCK accounting. However, this
> function is called after virtio_transport_fill_skb() has already consumed
> the iterator via __zerocopy_sg_from_iter(), so on the last skb, iter->count
> will be 0, skipping the RLIMIT_MEMLOCK enforcement.
>
> Pass pkt_len (the total bytes being sent) as an explicit parameter to
> virtio_transport_init_zcopy_skb() instead of reading the already-consumed
> iter->count.
>
> This matches TCP and UDP, which both call msg_zerocopy_realloc() with
> the original message size.
>
> Fixes: 581512a6dc93 ("vsock/virtio: MSG_ZEROCOPY flag support")
> Reported-by: Yiming Qian <yimingqian591@gmail.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> net/vmw_vsock/virtio_transport_common.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 0742091beae7..416d533f493d 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -73,6 +73,7 @@ static bool virtio_transport_can_zcopy(const struct virtio_transport *t_ops,
> static int virtio_transport_init_zcopy_skb(struct vsock_sock *vsk,
> struct sk_buff *skb,
> struct msghdr *msg,
> + size_t pkt_len,
> bool zerocopy)
> {
> struct ubuf_info *uarg;
> @@ -81,12 +82,10 @@ static int virtio_transport_init_zcopy_skb(struct vsock_sock *vsk,
> uarg = msg->msg_ubuf;
> net_zcopy_get(uarg);
> } else {
> - struct iov_iter *iter = &msg->msg_iter;
> struct ubuf_info_msgzc *uarg_zc;
>
> uarg = msg_zerocopy_realloc(sk_vsock(vsk),
> - iter->count,
> - NULL, false);
> + pkt_len, NULL, false);
> if (!uarg)
> return -1;
>
> @@ -398,11 +397,17 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> * each iteration. If this is last skb for this buffer
> * and MSG_ZEROCOPY mode is in use - we must allocate
> * completion for the current syscall.
> + *
> + * Pass pkt_len because msg iter is already consumed
> + * by virtio_transport_fill_skb(), so iter->count
> + * can not be used for RLIMIT_MEMLOCK pinned-pages
> + * accounting done by msg_zerocopy_realloc().
> */
> if (info->msg && info->msg->msg_flags & MSG_ZEROCOPY &&
> skb_len == rest_len && info->op == VIRTIO_VSOCK_OP_RW) {
> if (virtio_transport_init_zcopy_skb(vsk, skb,
> info->msg,
> + pkt_len,
> can_zcopy)) {
> kfree_skb(skb);
> ret = -ENOMEM;
> --
> 2.53.0
>
Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>
^ permalink raw reply
* Re: [PATCH bpf] bpf: Fix NULL pointer dereference in bpf_sk_storage_clone and diag paths
From: Martin KaFai Lau @ 2026-04-20 18:36 UTC (permalink / raw)
To: Weiming Shi
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Martin KaFai Lau, Alexei Starovoitov, Amery Hung,
Leon Hwang, Kees Cook, Fushuai Wang, Menglong Dong, netdev, bpf,
Xiang Mei
In-Reply-To: <20260420161432.3919396-2-bestswngs@gmail.com>
On Mon, Apr 20, 2026 at 09:14:33AM -0700, Weiming Shi wrote:
> diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
> index f8338acebf077..3b487280f50fa 100644
> --- a/net/core/bpf_sk_storage.c
> +++ b/net/core/bpf_sk_storage.c
> @@ -172,7 +172,7 @@ int bpf_sk_storage_clone(const struct sock *sk, struct sock *newsk)
> struct bpf_map *map;
>
> smap = rcu_dereference(SDATA(selem)->smap);
> - if (!(smap->map.map_flags & BPF_F_CLONE))
> + if (!smap || !(smap->map.map_flags & BPF_F_CLONE))
> continue;
>
> /* Note that for lockless listeners adding new element
> @@ -547,6 +547,8 @@ static int diag_get(struct bpf_local_storage_data *sdata, struct sk_buff *skb)
> return -EMSGSIZE;
>
> smap = rcu_dereference(sdata->smap);
> + if (!smap)
> + goto errout;
You need to study it more thoroughly and the code around it
instead of rushing to fix a problem discovered by AI/bot (?).
This is now treated as an -EMSGSIZE error by diag_get().
> if (nla_put_u32(skb, SK_DIAG_BPF_STORAGE_MAP_ID, smap->map.id))
> goto errout;
>
> @@ -599,6 +601,8 @@ static int bpf_sk_storage_diag_put_all(struct sock *sk, struct sk_buff *skb,
> saved_len = skb->len;
> hlist_for_each_entry_rcu(selem, &sk_storage->list, snode) {
> smap = rcu_dereference(SDATA(selem)->smap);
> + if (!smap)
> + continue;
> diag_size += nla_value_size(smap->map.value_size);
>
> if (nla_stgs && diag_get(SDATA(selem), skb))
... and here it will eventually return an -EMSGSIZE to the user space
which is incorrect. Pass the smap to diag_get instead.
pw-bot: cr
^ permalink raw reply
* Re: [PATCH] llc: Return -EINPROGRESS from llc_ui_connect()
From: Jakub Kicinski @ 2026-04-20 18:41 UTC (permalink / raw)
To: Ernestas Kulik; +Cc: netdev, linux-kernel
In-Reply-To: <20260415063457.1008868-1-ernestas.k@iconn-networks.com>
On Wed, 15 Apr 2026 09:34:57 +0300 Ernestas Kulik wrote:
> Given a zero sk_sndtimeo, llc_ui_connect() skips waiting for state
> change and returns 0, confusing userspace applications that will assume
> the socket is connected, making e.g. getpeername() calls error out.
>
> Set rc to -EINPROGRESS before considering blocking, akin to AF_INET
> sockets.
Please add a note on how you discovered this issue.
Including whether you're actively using this code or just scanning it
for bugs.
> diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
> index 59d593bb5d18..9317d092ba84 100644
> --- a/net/llc/af_llc.c
> +++ b/net/llc/af_llc.c
> @@ -515,10 +515,12 @@ static int llc_ui_connect(struct socket *sock, struct sockaddr_unsized *uaddr,
> sock->state = SS_UNCONNECTED;
> sk->sk_state = TCP_CLOSE;
> goto out;
> }
>
> + rc = -EINPROGRESS;
Isn't this a bit of an odd placement? ..
> if (sk->sk_state == TCP_SYN_SENT) {
> const long timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
>
> if (!timeo || !llc_ui_wait_for_conn(sk, timeo))
> goto out;
.. I suspect you mean to target this branch, right?
^ permalink raw reply
* Re: [PATCH net] tcp: make probe0 timer handle expired user timeout
From: Eric Dumazet @ 2026-04-20 18:45 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Neal Cardwell, Altan Hacigumus, Kuniyuki Iwashima,
David S . Miller, David Ahern, Paolo Abeni, Simon Horman, netdev,
Enke Chen
In-Reply-To: <20260420113346.34735a1e@kernel.org>
On Mon, Apr 20, 2026 at 11:33 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 13 Apr 2026 18:36:34 -0700 Altan Hacigumus wrote:
> > tcp_clamp_probe0_to_user_timeout() computes remaining time in jiffies
> > using subtraction with an unsigned lvalue. If elapsed probing time
> > already exceeds the configured TCP_USER_TIMEOUT, the subtraction
> > underflows and yields a large value.
> >
> > Handle this expiration case similarly to tcp_clamp_rto_to_user_timeout().
> >
> > Fixes: 344db93ae3ee ("tcp: make TCP_USER_TIMEOUT accurate for zero window probes")
> > Signed-off-by: Altan Hacigumus <ahacigu.linux@gmail.com>
>
> Hi Eric, Neal, does this makes sense?
>
I missed this patch. I will take a look asap.
Thanks.
^ permalink raw reply
* Re: [PATCH net v2] ipv6: Apply max_dst_opts_cnt to ip6_tnl_parse_tlv_enc_lim
From: Justin Iurman @ 2026-04-20 18:55 UTC (permalink / raw)
To: Ido Schimmel, daniel
Cc: kuba, edumazet, dsahern, tom, willemdebruijn.kernel, pabeni,
netdev
In-Reply-To: <20260419143137.GA885197@shredder>
On 4/19/26 16:31, Ido Schimmel wrote:
> On Sun, Apr 19, 2026 at 12:37:35AM +0200, Justin Iurman wrote:
>> Nope. But if it happens, users would be confused as max_dst_opts_cnt would
>> not have the same meaning in two different code paths. OTOH, I agree that
>> such situation would look suspicious. I guess it's fine to keep your patch
>> as is and to not over-complicate things unnecessarily.
>
> I agree that it's weird to reuse max_dst_opts_cnt here:
>
> 1. The meaning is different from the Rx path.
>
> 2. We only enforce max_dst_opts_cnt, but not max_dst_opts_len.
>
> 3. The default is derived from the initial netns, unlike in the Rx path.
>
> Given the above and that:
>
> 1. We believe that 8 options until the tunnel encapsulation limit option
> is liberal enough.
>
> 2. We don't want to over-complicate things.
>
> Can we go with an hard coded 8 and see if anyone complains? In the
> unlikely case that someone complains we can at least gain some insight
> into how this option is actually used with tunnels.
In general, I'm not a big fan of hard-coded values, but I also think
that in this context it would make sense to do so. This is not a strong
+1, let's say it's more a "not against it".
^ permalink raw reply
* Re: [PATCH net] netconsole: avoid out-of-bounds access on empty string in trim_newline()
From: Gustavo Luiz Duarte @ 2026-04-20 18:59 UTC (permalink / raw)
To: Breno Leitao
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Matthew Wood, netdev, linux-kernel, kernel-team,
stable
In-Reply-To: <20260420-netcons_trim_newline-v1-1-dc35889aeedf@debian.org>
On Mon, Apr 20, 2026 at 11:34 AM Breno Leitao <leitao@debian.org> wrote:
>
> trim_newline() unconditionally dereferences s[len - 1] after computing
> len = strnlen(s, maxlen). When the string is empty, len is 0 and the
> expression underflows to s[(size_t)-1], reading (and potentially
> writing) one byte before the buffer.
>
> The two callers feed trim_newline() with the result of strscpy() from
> configfs store callbacks (dev_name_store, userdatum_value_store).
> configfs guarantees count >= 1 reaches the callback, but the byte
> itself can be NUL: a userspace write(fd, "\0", 1) leaves the
> destination empty after strscpy() and triggers the underflow. The OOB
> write only fires if the adjacent byte happens to be '\n', so this is
> not a security issue, but the access is undefined behaviour either way.
>
> This pattern is commonly flagged by LLM-based code reviewers. While it
> is not a security fix, the underlying access is undefined behaviour and
> the change is small and self-contained, so it is a reasonable candidate
> for the stable trees.
>
> Guard the dereference on a non-zero length.
>
> Fixes: ae001dc67907 ("net: netconsole: move newline trimming to function")
> Cc: stable@vger.kernel.org
> Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Gustavo Luiz Duarte <gustavold@gmail.com>
> ---
> drivers/net/netconsole.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 3c9acd6e49e86..205384dab89a6 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -497,6 +497,8 @@ static void trim_newline(char *s, size_t maxlen)
> size_t len;
>
> len = strnlen(s, maxlen);
> + if (!len)
> + return;
> if (s[len - 1] == '\n')
> s[len - 1] = '\0';
> }
>
> ---
> base-commit: c7275b05bc428c7373d97aa2da02d3a7fa6b9f66
> change-id: 20260420-netcons_trim_newline-36f6ec3b9820
>
> Best regards,
> --
> Breno Leitao <leitao@debian.org>
>
>
^ permalink raw reply
* Re: [PATCH] gtp: disable BH before calling udp_tunnel_xmit_skb()
From: Justin Iurman @ 2026-04-20 19:02 UTC (permalink / raw)
To: David Carlier, Pablo Neira Ayuso, Harald Welte, Andrew Lunn,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Weiming Shi, osmocom-net-gprs, netdev, linux-kernel, stable
In-Reply-To: <20260417055408.4667-1-devnexen@gmail.com>
On 4/17/26 07:54, David Carlier wrote:
> gtp_genl_send_echo_req() runs as a generic netlink doit handler in
> process context with BH not disabled. It calls udp_tunnel_xmit_skb(),
> which eventually invokes iptunnel_xmit() — that uses __this_cpu_inc/dec
> on softnet_data.xmit.recursion to track the tunnel xmit recursion level.
>
> Without local_bh_disable(), the task may migrate between
> dev_xmit_recursion_inc() and dev_xmit_recursion_dec(), breaking the
> per-CPU counter pairing. The result is stale or negative recursion
> levels that can later produce false-positive
> SKB_DROP_REASON_RECURSION_LIMIT drops on either CPU.
>
> The other udp_tunnel_xmit_skb() call sites in gtp.c are unaffected:
> the data path runs under ndo_start_xmit and the echo response handlers
> run from the UDP encap rx softirq, both with BH already disabled.
>
> Fix it by disabling BH around the udp_tunnel_xmit_skb() call, mirroring
> commit 2cd7e6971fc2 ("sctp: disable BH before calling
> udp_tunnel_xmit_skb()").
Why not fix iptunnel_xmit() directly, rather than fixing all possible
callers? Basically, jut like we did for lwtunnel_{output|xmit}(). The
advantage would be that we no longer have to worry about BHs in the
callers, and BHs would only be disabled when necessary.
^ permalink raw reply
* AW: pre-boot plugged SFP autoneg advertisement
From: markus.stockhausen @ 2026-04-20 19:10 UTC (permalink / raw)
To: 'Andrew Lunn'
Cc: linux, hkallweit1, netdev, 'Jonas Jelonek', jan, nbd,
'Daniel Golle'
In-Reply-To: <664e6e24-4a94-43fa-8769-773a37a01c66@lunn.ch>
> Von: Andrew Lunn <andrew@lunn.ch>
> Gesendet: Montag, 20. April 2026 19:58
> An: markus.stockhausen@gmx.de
>
> > Sequence of events during boot is as follows:
> >
> > - SFP module is already inserted (in my case 1G)
> > - phylink_sfp_config_phy() runs long before any network config starts
> > - OpenWrt netifd daemon starts and wants to configure the network
interfaces
> > - It reads current settings via ethtool ioctl and gets autoneg=off
> > - It writes basic config values via ethtool ioctl including autneg=off
> > - Later on it starts the interface and phylink_start() is issued
>
> I would say netifd is not optimal. I'm not sure we every agree to
> return the full ksetting on an interface which is admin down. Many
> driver don't even connect to the PHY until open is called, and so are
> likely to return -ENODEV. See phy_ethtool_set_link_ksettings().
>
> Could you look into the behaviour of netifd, especially if it gets
> -ENODEV during the first read. Does it try again after setting the
> interface up?
Netifd has no issues with linksettings reading/writing in admin state.
Getting a rc=0 it assumes that all values are filled, changes the needed
attributes and writes them back. I retested and think there might be a
solution to avoid unneeded Ioctl access (see [1])
> If we know phylink is going to return a subset of the correct
> information when the interface is admin down, maybe it should return
> -ENODEV?
Or (stupid idea) phylink_ethtool_ksettings_set() should not accept all
settings in this state.
Thanks for your valuable input.
Markus
[1] https://github.com/openwrt/netifd/issues/76#issuecomment-4283478081
^ permalink raw reply
* Re: [PATCH net 1/1] net/rose: hold listener socket during call request handling
From: Yuan Tan @ 2026-04-20 19:11 UTC (permalink / raw)
To: Simon Horman, Ren Wei
Cc: yuantan098, linux-hams, netdev, davem, edumazet, kuba, pabeni,
kees, takamitz, kuniyu, jiayuan.chen, mingo, stanksal, jlayton,
yifanwucs, tomapufckgml, bird, tonanli66
In-Reply-To: <20260420162605.GV280379@horms.kernel.org>
On 4/20/2026 9:26 AM, Simon Horman wrote:
> On Fri, Apr 17, 2026 at 07:01:51PM +0800, Ren Wei wrote:
>> From: Nan Li <tonanli66@gmail.com>
>>
>> The call request receive path keeps using the listener socket after the
>> lookup lock has been dropped. Keep the listener alive across the
>> remaining validation and child socket setup by taking a reference in the
>> lookup path and releasing it once request handling is finished.
>>
>> This makes listener lifetime handling explicit and avoids races with
>> concurrent socket teardown.
>>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Cc: stable@kernel.org
>> Reported-by: Yifan Wu <yifanwucs@gmail.com>
>> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
>> Reported-by: Xin Liu <bird@lzu.edu.cn>
>> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
>> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
>> Signed-off-by: Nan Li <tonanli66@gmail.com>
>> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
>> ---
>> net/rose/af_rose.c | 24 +++++++++++++++++++-----
>> 1 file changed, 19 insertions(+), 5 deletions(-)
> Reviewed-by: Simon Horman <horms@kernel.org>
>
> Sachiko has provided some feedback on this patch.
> I do not believe they relate to shortcomings in this patch,
> and I do not believe they should block progress of this patch.
> You may want to look over them for areas to investigate as follow-up
> (maybe you already did :)
>
> ...
Thanks for your review! Yes this module still has other issues that
haven't been fixed. We'll finish what we're currently working on and
then take a look :)
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: ISO: Fix data-race on dst in iso_sock_connect()
From: Luiz Augusto von Dentz @ 2026-04-20 19:23 UTC (permalink / raw)
To: SeungJu Cheon
Cc: marcel, linux-bluetooth, netdev, linux-kernel, me, skhan,
linux-kernel-mentees
In-Reply-To: <20260418053401.128483-2-suunj1331@gmail.com>
Hi SeungJu,
On Sat, Apr 18, 2026 at 1:34 AM SeungJu Cheon <suunj1331@gmail.com> wrote:
>
> iso_sock_connect() copies the destination address into
> iso_pi(sk)->dst under lock_sock, then releases the lock and reads
> it back with bacmp() to decide between the CIS and BIS connect
> paths:
>
> lock_sock(sk);
> bacpy(&iso_pi(sk)->dst, &sa->iso_bdaddr);
> iso_pi(sk)->dst_type = sa->iso_bdaddr_type;
> release_sock(sk);
>
> if (bacmp(&iso_pi(sk)->dst, BDADDR_ANY)) // <- no lock held
>
> This read after release_sock() races with any concurrent write to
> iso_pi(sk)->dst on the same socket.
>
> Fix by performing the bacmp() inside the lock_sock critical section
> and caching the result in a local variable.
>
> This patch addresses only the bacmp() race in iso_sock_connect();
> other unprotected iso_pi(sk) accesses are fixed separately in the
> next patch.
>
> KCSAN report:
>
> BUG: KCSAN: data-race in memcmp+0x39/0xb0
>
> race at unknown origin, with read to 0xffff8f96ea66dde3 of 1 bytes by task 549 on cpu 1:
> memcmp+0x39/0xb0
> iso_sock_connect+0x275/0xb40
> __sys_connect_file+0xbd/0xe0
> __sys_connect+0xe0/0x110
> __x64_sys_connect+0x40/0x50
> x64_sys_call+0xcad/0x1c60
> do_syscall_64+0x133/0x590
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> value changed: 0x00 -> 0xee
>
> Reported by Kernel Concurrency Sanitizer on:
> CPU: 1 UID: 0 PID: 549 Comm: iso_race_combin Not tainted 7.0.0-08391-g1d51b370a0f8 #40 PREEMPT(lazy)
>
> Fixes: ccf74f2390d6 ("Bluetooth: Add BTPROTO_ISO socket type")
> Signed-off-by: SeungJu Cheon <suunj1331@gmail.com>
> ---
> net/bluetooth/iso.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index be145e2736b7..14963ba68597 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -1169,6 +1169,7 @@ static int iso_sock_connect(struct socket *sock, struct sockaddr_unsized *addr,
> struct sockaddr_iso *sa = (struct sockaddr_iso *)addr;
> struct sock *sk = sock->sk;
> int err;
> + bool bcast;
>
> BT_DBG("sk %p", sk);
>
> @@ -1191,9 +1192,11 @@ static int iso_sock_connect(struct socket *sock, struct sockaddr_unsized *addr,
> bacpy(&iso_pi(sk)->dst, &sa->iso_bdaddr);
> iso_pi(sk)->dst_type = sa->iso_bdaddr_type;
>
> + bcast = !bacmp(&iso_pi(sk)->dst, BDADDR_ANY);
> +
> release_sock(sk);
>
> - if (bacmp(&iso_pi(sk)->dst, BDADDR_ANY))
> + if (!bcast)
> err = iso_connect_cis(sk);
> else
> err = iso_connect_bis(sk);
> --
> 2.52.0
>
https://sashiko.dev/#/patchset/20260418053401.128483-1-suunj1331%40gmail.com
Seems valid, so we migth just use sa in the place of iso_pi(sk) to
avoid using it without sk being locked. Other problems it may reveal
need to be addressed in separate patches.
--
Luiz Augusto von Dentz
^ permalink raw reply
* [PATCH net] ipv4: clamp MCAST_MSFILTER getsockopt to optlen, not gf_numsrc
From: Greg Kroah-Hartman @ 2026-04-20 19:26 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Greg Kroah-Hartman, David S. Miller, David Ahern,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, stable
ip_get_mcast_msfilter() and its compat sibling read gf_numsrc from
the user's buffer header and pass it to ip_mc_gsfget(), which writes:
min(actual_sources, gf_numsrc) * sizeof(struct sockaddr_storage)
bytes back into the user's optval starting at the gf_slist_flex offset.
The only optlen check is len >= size0 (the header), so a user can pass
optlen = 144 (header only) with gf_numsrc = 4. If the socket has at
least 4 sources joined, the kernel writes 4*128 = 512 bytes via
copy_to_sockptr_offset() past the end of the user buffer.
This is a kernel-driven userspace heap overflow: the user told the
kernel their buffer size via optlen, the kernel ignored it and used a
field inside the buffer instead. On a real system the writes go into
adjacent userspace heap and copy_to_user does not fault on mapped heap
pages.
Clamp gf_numsrc to (len - size0) / sizeof(sockaddr_storage) before the
call so the kernel never writes past what the user provided. The
setsockopt path already has the equivalent check
(GROUP_FILTER_SIZE(gf_numsrc) > optlen at line 790).
Cc: "David S. Miller" <davem@davemloft.net>
Cc: David Ahern <dsahern@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Reported-by: Anthropic
Assisted-by: gkh_clanker_t1000
Cc: stable <stable@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv4/ip_sockglue.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index a55ef327ec93..c9bf5d223f21 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1456,6 +1456,11 @@ static int ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval,
return -EFAULT;
num = gsf.gf_numsrc;
+
+ if (num > (len - size0) / sizeof(struct sockaddr_storage))
+ num = (len - size0) / sizeof(struct sockaddr_storage);
+ gsf.gf_numsrc = num;
+
err = ip_mc_gsfget(sk, &gsf, optval,
offsetof(struct group_filter, gf_slist_flex));
if (err)
@@ -1486,8 +1491,12 @@ static int compat_ip_get_mcast_msfilter(struct sock *sk, sockptr_t optval,
gf.gf_interface = gf32.gf_interface;
gf.gf_fmode = gf32.gf_fmode;
num = gf.gf_numsrc = gf32.gf_numsrc;
- gf.gf_group = gf32.gf_group;
+ if (num > (len - size0) / sizeof(struct sockaddr_storage))
+ num = (len - size0) / sizeof(struct sockaddr_storage);
+ gf.gf_numsrc = num;
+
+ gf.gf_group = gf32.gf_group;
err = ip_mc_gsfget(sk, &gf, optval,
offsetof(struct compat_group_filter, gf_slist_flex));
if (err)
--
2.53.0
^ permalink raw reply related
* [PATCH net] ipv6: rpl: expand skb head when recompressed SRH grows, not only on last segment
From: Greg Kroah-Hartman @ 2026-04-20 19:32 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Greg Kroah-Hartman, David S. Miller, David Ahern,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, stable
ipv6_rpl_srh_rcv() processes a Routing Protocol for LLNs Source Routing
Header by decompressing it, swapping the next segment address into
ipv6_hdr->daddr, recompressing, and pushing the new header back. The
recompressed header can be larger than the original when the
address-elision opportunities are worse after the swap.
The function pulls (hdr->hdrlen + 1) << 3 bytes (the old header) and
pushes (chdr->hdrlen + 1) << 3 + sizeof(ipv6hdr) bytes (the new header
plus the IPv6 header). pskb_expand_head() is called to guarantee
headroom only when segments_left == 0.
A crafted SRH that loops back to the local host (each segment is a local
address, so ip6_route_input() delivers it back to ipv6_rpl_srh_rcv())
with chdr growing on each pass exhausts headroom over several
iterations. When skb_push() lands skb->data exactly at skb->head,
skb_reset_network_header() stores 0, and skb_mac_header_rebuild()'s
skb_set_mac_header(skb, -skb->mac_len) computes 0 + (u16)(-14) = 65522.
The subsequent memmove writes 14 bytes at skb->head + 65522.
Expand the head whenever there is insufficient room for the push, not
only on the final segment.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: David Ahern <dsahern@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Reported-by: Anthropic
Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv6/exthdrs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 95558fd6f447..d866ab011e0a 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -592,7 +592,9 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb)
skb_pull(skb, ((hdr->hdrlen + 1) << 3));
skb_postpull_rcsum(skb, oldhdr,
sizeof(struct ipv6hdr) + ((hdr->hdrlen + 1) << 3));
- if (unlikely(!hdr->segments_left)) {
+ if (unlikely(!hdr->segments_left ||
+ skb_headroom(skb) < sizeof(struct ipv6hdr) +
+ ((chdr->hdrlen + 1) << 3))) {
if (pskb_expand_head(skb, sizeof(struct ipv6hdr) + ((chdr->hdrlen + 1) << 3), 0,
GFP_ATOMIC)) {
__IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)), IPSTATS_MIB_OUTDISCARDS);
--
2.53.0
^ permalink raw reply related
* Re: [PATCH net v2 1/8] xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices
From: Stanislav Fomichev @ 2026-04-20 19:34 UTC (permalink / raw)
To: Jason Xing; +Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260420082805.14844-2-kerneljasonxing@gmail.com>
> From: Jason Xing <kernelxing@tencent.com>
>
> skb_checksum_help() is a common helper that writes the folded
> 16-bit checksum back via skb->data + csum_start + csum_offset,
> i.e. it relies on the skb's linear head and fails (with WARN_ONCE
> and -EINVAL) when skb_headlen() is 0.
>
> AF_XDP generic xmit takes two very different paths depending on the
> netdev. Drivers that advertise IFF_TX_SKB_NO_LINEAR (e.g. virtio_net)
> skip the "copy payload into a linear head" step on purpose as a
> performance optimisation: xsk_build_skb_zerocopy() only attaches UMEM
> pages as frags and never calls skb_put(), so skb_headlen() stays 0
> for the whole skb. For these skbs there is simply no linear area for
> skb_checksum_help() to write the csum into - the sw-csum fallback is
> structurally inapplicable.
>
> The patch tries to catch this and reject the combination with error at
> setup time. Rejecting at bind() converts this silent per-packet failure
> into a synchronous, actionable -EOPNOTSUPP at setup time. HW csum and
> launch_time metadata on IFF_TX_SKB_NO_LINEAR drivers are unaffected
> because they do not call skb_checksum_help().
>
> Without the patch, every descriptor carrying 'XDP_TX_METADATA |
> XDP_TXMD_FLAGS_CHECKSUM' produces:
> 1) a WARN_ONCE "offset (N) >= skb_headlen() (0)" from skb_checksum_help(),
> 2) sendmsg() returning -EINVAL without consuming the descriptor
> (invalid_descs is not incremented),
> 3) a wedged TX ring: __xsk_generic_xmit() does not advance the
> consumer on non-EOVERFLOW errors, so the next sendmsg() re-reads
> the same descriptor and re-hits the same WARN until the socket
> is closed.
>
> Closes: https://lore.kernel.org/all/20260419045822.843BFC2BCAF@smtp.kernel.org/#t
> Fixes: 30c3055f9c0d ("xsk: wrap generic metadata handling onto separate function")
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> net/xdp/xsk_buff_pool.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 37b7a68b89b3..c2521b6547e3 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -169,6 +169,9 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
> if (force_zc && force_copy)
> return -EINVAL;
>
> + if (pool->tx_sw_csum && (netdev->priv_flags & IFF_TX_SKB_NO_LINEAR))
> + return -EOPNOTSUPP;
> +
> if (xsk_get_pool_from_qid(netdev, queue_id))
> return -EBUSY;
>
> --
> 2.41.3
>
Wondering whether a better fixes tag is commit 11614723af26 ("xsk: Add option
to calculate TX checksum in SW")?
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
^ permalink raw reply
* Re: [PATCH net v2 2/8] xsk: handle NULL dereference of the skb without frags issue
From: Stanislav Fomichev @ 2026-04-20 19:34 UTC (permalink / raw)
To: Jason Xing; +Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260420082805.14844-3-kerneljasonxing@gmail.com>
> From: Jason Xing <kernelxing@tencent.com>
>
> When a first descriptor (xs->skb == NULL) triggers -EOVERFLOW in
> xsk_build_skb_zerocopy (e.g., MAX_SKB_FRAGS exceeded), the free_err
> EOVERFLOW handler unconditionally dereferences xs->skb via
> xsk_inc_num_desc(xs->skb) and xsk_drop_skb(xs->skb), causing a NULL
> pointer dereference.
>
> In this series, the skb is already freed by kfree_skb() inside
> xsk_build_skb_zerocopy for the first-descriptor case, so we only need
> to do the bookkeeping: cancel the one reserved CQ slot and account for
> the single invalid descriptor.
>
> Guard the existing xsk_inc_num_desc/xsk_drop_skb calls with an
> xs->skb check (for the continuation case), and add an else branch
> for the first-descriptor case that manually cancels the CQ slot and
> increments invalid_descs by one.
>
> Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> net/xdp/xsk.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 6149f6a79897..6521604f8d42 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -893,9 +893,14 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
> kfree_skb(skb);
>
> if (err == -EOVERFLOW) {
> - /* Drop the packet */
> - xsk_inc_num_desc(xs->skb);
> - xsk_drop_skb(xs->skb);
> + if (xs->skb) {
> + /* Drop the packet */
> + xsk_inc_num_desc(xs->skb);
> + xsk_drop_skb(xs->skb);
> + } else {
> + xsk_cq_cancel_locked(xs->pool, 1);
> + xs->tx->invalid_descs++;
> + }
> xskq_cons_release(xs->tx);
> } else {
> /* Let application retry */
> --
> 2.41.3
>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
^ permalink raw reply
* Re: [PATCH net v2 3/8] xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path
From: Stanislav Fomichev @ 2026-04-20 19:34 UTC (permalink / raw)
To: Jason Xing; +Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260420082805.14844-4-kerneljasonxing@gmail.com>
> From: Jason Xing <kernelxing@tencent.com>
>
> When xsk_build_skb() processes multi-buffer packets in copy mode, the
> first descriptor stores data into the skb linear area without adding
> any frags, so nr_frags stays at 0. The caller then sets xs->skb = skb
> to accumulate subsequent descriptors.
>
> If a continuation descriptor fails (e.g. alloc_page returns NULL with
> -EAGAIN), we jump to free_err where the condition:
>
> if (skb && !skb_shinfo(skb)->nr_frags)
> kfree_skb(skb);
>
> evaluates to true because nr_frags is still 0 (the first descriptor
> used the linear area, not frags). This frees the skb while xs->skb
> still points to it, creating a dangling pointer. On the next transmit
> attempt or socket close, xs->skb is dereferenced, causing a
> use-after-free or double-free.
>
> Fix by adding a !xs->skb check to the condition, ensuring we only free
> skbs that were freshly allocated in this call (xs->skb is NULL) and
> never free an in-progress multi-buffer skb that the caller still
> references.
>
> Closes: https://lore.kernel.org/all/20260415082654.21026-4-kerneljasonxing@gmail.com/
> Fixes: 6b9c129c2f93 ("xsk: remove @first_frag from xsk_build_skb()")
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> net/xdp/xsk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 6521604f8d42..4fdd1a45a9bd 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -889,7 +889,7 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
> return skb;
>
> free_err:
> - if (skb && !skb_shinfo(skb)->nr_frags)
> + if (skb && !xs->skb && !skb_shinfo(skb)->nr_frags)
> kfree_skb(skb);
>
> if (err == -EOVERFLOW) {
> --
> 2.41.3
Now "!skb_shinfo(skb)->nr_frags" feels redundant? It's either
"skb && !xs->skb" and we own the kfree. or "xs->skb != NULL" and we
want xsk_drop_skb? Or am I missing something?
^ permalink raw reply
* Re: [PATCH net v2 4/8] xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()
From: Stanislav Fomichev @ 2026-04-20 19:34 UTC (permalink / raw)
To: Jason Xing; +Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260420082805.14844-5-kerneljasonxing@gmail.com>
> From: Jason Xing <kernelxing@tencent.com>
>
> Once xsk_skb_init_misc() has been called on an skb, its destructor is
> set to xsk_destruct_skb(), which submits the descriptor address(es) to
> the completion queue and advances the CQ producer. If such an skb is
> subsequently freed via kfree_skb() along an error path - before the
> skb has ever been handed to the driver - the destructor still runs and
> submits a bogus, half-initialized address to the CQ.
>
> Introduce a new common helper to fix the issue. That function will be
> used by the subsequent patches soon.
>
> Closes: https://lore.kernel.org/all/20260419045822.843BFC2BCAF@smtp.kernel.org/
> Fixes: c30d084960cf ("xsk: avoid overwriting skb fields for multi-buffer traffic")
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> net/xdp/xsk.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 4fdd1a45a9bd..614e7bd1252b 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -717,6 +717,12 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
> return 0;
> }
>
> +static void xsk_drop_untrans_skb(struct sk_buff *skb)
> +{
> + skb->destructor = sock_wfree;
> + kfree_skb(skb);
> +}
> +
> static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
> struct xdp_desc *desc)
> {
> @@ -890,7 +896,7 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
>
> free_err:
> if (skb && !xs->skb && !skb_shinfo(skb)->nr_frags)
> - kfree_skb(skb);
> + xsk_drop_untrans_skb(skb);
>
> if (err == -EOVERFLOW) {
> if (xs->skb) {
> --
> 2.41.3
>
Have you considered the alternative where we postpone `skb->destructor =
xsk_destruct_skb` to a later point? Will this be less messy than
undoing that descriptor in a few curated places?
^ permalink raw reply
* Re: [PATCH net v2 11/12] idpf: fix xdp crash in soft reset error path
From: Jacob Keller @ 2026-04-20 19:41 UTC (permalink / raw)
To: Jakub Kicinski
Cc: andrew+netdev, davem, edumazet, pabeni, netdev, emil.s.tantilov,
stable, aleksandr.loktionov, patryk.holda
In-Reply-To: <20260418190019.194263-2-kuba@kernel.org>
On 4/18/2026 12:00 PM, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> Jakub: I'll drop this patch and apply the rest.
Thanks. Emil is on vacation, so I don't know if we'll get any response
for this fix for a bit. I'll forward this to others on the team and see
what they think.
Regards,
Jake
^ permalink raw reply
* Re: [PATCH v5 2/6] bpf: refactor masks for ADJ_ROOM flags and encap validation
From: Willem de Bruijn @ 2026-04-20 19:42 UTC (permalink / raw)
To: Nick Hudson, bpf, netdev, Willem de Bruijn, Martin KaFai Lau
Cc: Nick Hudson, Max Tottenham, Anna Glasgall, Daniel Borkmann,
Alexei Starovoitov, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-kernel
In-Reply-To: <20260420104051.1528843-3-nhudson@akamai.com>
Nick Hudson wrote:
> Refactor the helper masks for bpf_skb_adjust_room() flags to simplify
> validation logic and introduce:
>
> - BPF_F_ADJ_ROOM_ENCAP_MASK
> - BPF_F_ADJ_ROOM_DECAP_MASK
>
> Refactor existing validation checks in bpf_skb_net_shrink()
> and bpf_skb_adjust_room() to use the new masks (no behavior change).
>
> This is in preparation for supporting the new decap flags.
>
> Co-developed-by: Max Tottenham <mtottenh@akamai.com>
> Signed-off-by: Max Tottenham <mtottenh@akamai.com>
> Co-developed-by: Anna Glasgall <aglasgal@akamai.com>
> Signed-off-by: Anna Glasgall <aglasgal@akamai.com>
> Signed-off-by: Nick Hudson <nhudson@akamai.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH v5 3/6] bpf: add BPF_F_ADJ_ROOM_DECAP_* flags for tunnel decapsulation
From: Willem de Bruijn @ 2026-04-20 19:44 UTC (permalink / raw)
To: Nick Hudson, bpf, netdev, Willem de Bruijn, Martin KaFai Lau
Cc: Nick Hudson, Max Tottenham, Anna Glasgall, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, linux-kernel
In-Reply-To: <20260420104051.1528843-4-nhudson@akamai.com>
Nick Hudson wrote:
> Add new bpf_skb_adjust_room() decapsulation flags:
>
> - BPF_F_ADJ_ROOM_DECAP_L4_GRE
> - BPF_F_ADJ_ROOM_DECAP_L4_UDP
> - BPF_F_ADJ_ROOM_DECAP_IPXIP4
> - BPF_F_ADJ_ROOM_DECAP_IPXIP6
>
> These flags let BPF programs describe which tunnel layer is being
> removed, so later changes can update tunnel-related GSO state
> accordingly during decapsulation.
>
> This patch only introduces the UAPI flag definitions and helper
> documentation.
>
> Co-developed-by: Max Tottenham <mtottenh@akamai.com>
> Signed-off-by: Max Tottenham <mtottenh@akamai.com>
> Co-developed-by: Anna Glasgall <aglasgal@akamai.com>
> Signed-off-by: Anna Glasgall <aglasgal@akamai.com>
> Signed-off-by: Nick Hudson <nhudson@akamai.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH] gtp: disable BH before calling udp_tunnel_xmit_skb()
From: David CARLIER @ 2026-04-20 19:44 UTC (permalink / raw)
To: Justin Iurman
Cc: Pablo Neira Ayuso, Harald Welte, Andrew Lunn, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Weiming Shi, osmocom-net-gprs,
netdev, linux-kernel, stable
In-Reply-To: <b44de581-9f41-4804-afb1-72c491d9443a@gmail.com>
Hi Julian,
On Mon, 20 Apr 2026 at 20:02, Justin Iurman <justin.iurman@gmail.com> wrote:
>
> On 4/17/26 07:54, David Carlier wrote:
> > gtp_genl_send_echo_req() runs as a generic netlink doit handler in
> > process context with BH not disabled. It calls udp_tunnel_xmit_skb(),
> > which eventually invokes iptunnel_xmit() — that uses __this_cpu_inc/dec
> > on softnet_data.xmit.recursion to track the tunnel xmit recursion level.
> >
> > Without local_bh_disable(), the task may migrate between
> > dev_xmit_recursion_inc() and dev_xmit_recursion_dec(), breaking the
> > per-CPU counter pairing. The result is stale or negative recursion
> > levels that can later produce false-positive
> > SKB_DROP_REASON_RECURSION_LIMIT drops on either CPU.
> >
> > The other udp_tunnel_xmit_skb() call sites in gtp.c are unaffected:
> > the data path runs under ndo_start_xmit and the echo response handlers
> > run from the UDP encap rx softirq, both with BH already disabled.
> >
> > Fix it by disabling BH around the udp_tunnel_xmit_skb() call, mirroring
> > commit 2cd7e6971fc2 ("sctp: disable BH before calling
> > udp_tunnel_xmit_skb()").
>
> Why not fix iptunnel_xmit() directly, rather than fixing all possible
> callers? Basically, jut like we did for lwtunnel_{output|xmit}(). The
> advantage would be that we no longer have to worry about BHs in the
> callers, and BHs would only be disabled when necessary.
Good point — your lwtunnel fix (c03a49f3093a) is a close parallel, and
a central fix would avoid chasing callers one by one (sctp was patched
last week, gtp is this one, and tipc/wireguard/ovpn genl paths look
similar).
Happy to respin as v2 with local_bh_disable/enable moved into
iptunnel_xmit() (and ip6tunnel_xmit() for symmetry), and drop the
gtp-local hunk. That would also supersede Xin Long's recent sctp
commit
(2cd7e6971fc2), so I'll make sure to Cc him.
One thing I'd like your take on before I send: iptunnel_xmit() feels
like the natural home since it owns the recursion counter, but would
you rather see it in udp_tunnel_xmit_skb()? I don't want to pick the
wrong spot if you already have a preference.
Cheers !
^ permalink raw reply
* Re: [PATCH v5 4/6] bpf: allow new DECAP flags and add guard rails
From: Willem de Bruijn @ 2026-04-20 19:45 UTC (permalink / raw)
To: Nick Hudson, bpf, netdev, Willem de Bruijn, Martin KaFai Lau
Cc: Nick Hudson, Max Tottenham, Anna Glasgall, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-kernel
In-Reply-To: <20260420104051.1528843-5-nhudson@akamai.com>
Nick Hudson wrote:
> Add checks to require shrink-only decap, reject conflicting decap flag
> combinations, and verify removed length is sufficient for claimed header
> decapsulation.
>
> Co-developed-by: Max Tottenham <mtottenh@akamai.com>
> Signed-off-by: Max Tottenham <mtottenh@akamai.com>
> Co-developed-by: Anna Glasgall <aglasgal@akamai.com>
> Signed-off-by: Anna Glasgall <aglasgal@akamai.com>
> Signed-off-by: Nick Hudson <nhudson@akamai.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH v5 5/6] bpf: clear decap tunnel GSO state in skb_adjust_room
From: Willem de Bruijn @ 2026-04-20 19:46 UTC (permalink / raw)
To: Nick Hudson, bpf, netdev, Willem de Bruijn, Martin KaFai Lau
Cc: Nick Hudson, Max Tottenham, Anna Glasgall, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, linux-kernel
In-Reply-To: <20260420104051.1528843-6-nhudson@akamai.com>
Nick Hudson wrote:
> On shrink in bpf_skb_adjust_room(), clear tunnel-specific GSO flags
> according to the decapsulation flags:
>
> - BPF_F_ADJ_ROOM_DECAP_L4_UDP clears SKB_GSO_UDP_TUNNEL{,_CSUM}
> - BPF_F_ADJ_ROOM_DECAP_L4_GRE clears SKB_GSO_GRE{,_CSUM}
> - BPF_F_ADJ_ROOM_DECAP_IPXIP4 clears SKB_GSO_IPXIP4
> - BPF_F_ADJ_ROOM_DECAP_IPXIP6 clears SKB_GSO_IPXIP6
>
> When all tunnel-related GSO bits are cleared, also clear
> skb->encapsulation.
>
> Handle the ESP inside a UDP tunnel case where encapsulation should remain
> set.
>
> Co-developed-by: Max Tottenham <mtottenh@akamai.com>
> Signed-off-by: Max Tottenham <mtottenh@akamai.com>
> Co-developed-by: Anna Glasgall <aglasgal@akamai.com>
> Signed-off-by: Anna Glasgall <aglasgal@akamai.com>
> Signed-off-by: Nick Hudson <nhudson@akamai.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH net v2 11/12] idpf: fix xdp crash in soft reset error path
From: Jacob Keller @ 2026-04-20 19:48 UTC (permalink / raw)
To: Joshua Hay
Cc: andrew+netdev, davem, edumazet, pabeni, netdev, emil.s.tantilov,
stable, aleksandr.loktionov, patryk.holda
In-Reply-To: <20260418190019.194263-2-kuba@kernel.org>
Here's the full review on the website if it helps:
https://sashiko.dev/#/patchset/20260319224159.23885-1-emil.s.tantilov%40intel.com
Thanks,
Jake
On 4/18/2026 12:00 PM, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
>
> Jakub: I'll drop this patch and apply the rest.
> ---
> idpf: fix xdp crash in soft reset error path
>
> This commit fixes a NULL pointer dereference in idpf_vport_open() when
> soft reset fails by restoring vport->xdp_prog to the old value. It also
> updates the restart check to use IDPF_VPORT_UP instead of netif_running().
>
> Cc: stable@vger.kernel.org
>
>> diff --git a/drivers/net/ethernet/intel/idpf/xdp.c b/drivers/net/ethernet/intel/idpf/xdp.c
>> --- a/drivers/net/ethernet/intel/idpf/xdp.c
>> +++ b/drivers/net/ethernet/intel/idpf/xdp.c
>> @@ -488,6 +488,7 @@ static int idpf_xdp_setup_prog(struct idpf_vport *vport,
>> "Could not reopen the vport after XDP setup");
>>
>> cfg->user_config.xdp_prog = old;
>> + vport->xdp_prog = old;
>
> Does this also need to roll back vport->num_xdp_txq?
>
> Because idpf_initiate_soft_reset() overwrites the vport struct with the new
> queue configuration before failing, vport->num_xdp_txq might remain > 0
> while vport->xdp_prog is restored to NULL.
>
> When the interface is brought up, idpf_xdpsqs_get() evaluates
> idpf_xdp_enabled(vport) as false and returns early without allocating the
> NAPI timers (xdpsq->timer).
>
> Could another interface redirect XDP frames via ndo_xdp_xmit, bypassing the
> check because vport->num_xdp_txq > 0, and cause a NULL pointer dereference
> in libeth_xdpsq_queue_timer() during idpf_xdp_tx_finalize()?
>
>> old = prog;
>> }
>
> Is this old = prog assignment leading to a double-put of the BPF program?
>
> If idpf_initiate_soft_reset() fails, this error path drops a reference to
> the newly provided prog when it calls bpf_prog_put(old) right after this
> block.
>
> Since the function returns an error code to the core network stack, won't
> dev_xdp_install() in net/core/dev.c also explicitly call bpf_prog_put(prog)
> assuming ownership of the error cleanup?
>
> Could this underflow the reference count and cause a use-after-free?
>
> [ ... ]
>
>> diff --git a/drivers/net/ethernet/intel/idpf/xsk.c b/drivers/net/ethernet/intel/idpf/xsk.c
>> --- a/drivers/net/ethernet/intel/idpf/xsk.c
>> +++ b/drivers/net/ethernet/intel/idpf/xsk.c
>> @@ -568,7 +569,8 @@ int idpf_xsk_pool_setup(struct idpf_vport *vport, struct netdev_bpf *bpf)
>> return -EINVAL;
>> }
>>
>> - restart = idpf_xdp_enabled(vport) && netif_running(vport->netdev);
>> + restart = idpf_xdp_enabled(vport) &&
>> + test_bit(IDPF_VPORT_UP, np->state);
>> if (!restart)
>> goto pool;
>
> Does this function leave the queue pair permanently disabled if the pool
> setup fails?
>
> If restart is true, the queue pair is disabled via:
> idpf_qp_switch(vport, qid, false);
>
> If the subsequent call to libeth_xsk_setup_pool() fails, the function
> returns early:
>
> ret = libeth_xsk_setup_pool(vport->netdev, qid, pool);
> if (ret) {
> ...
> return ret;
> }
>
> Does this early return bypass the idpf_qp_switch(vport, qid, true) call
> expected later, causing a permanent partial loss of connectivity until the
> interface is manually restarted?
^ permalink raw reply
* Re: [PATCH net v2 8/8] xsk: fix u64 descriptor address truncation on 32-bit architectures
From: Stanislav Fomichev @ 2026-04-20 19:49 UTC (permalink / raw)
To: Jason Xing
Cc: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, bpf, netdev, Jason Xing
In-Reply-To: <20260420082805.14844-9-kerneljasonxing@gmail.com>
On 04/20, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
>
> In copy mode TX, xsk_skb_destructor_set_addr() stores the 64-bit
> descriptor address into skb_shinfo(skb)->destructor_arg (void *) via a
> uintptr_t cast:
>
> skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t)addr | 0x1UL);
>
> On 32-bit architectures uintptr_t is 32 bits, so the upper 32 bits of
> the descriptor address are silently dropped. In XDP_ZEROCOPY unaligned
> mode the chunk offset is encoded in bits 48-63 of the descriptor
> address (XSK_UNALIGNED_BUF_OFFSET_SHIFT = 48), meaning the offset is
> lost entirely. The completion queue then returns a truncated address to
> userspace, making buffer recycling impossible.
>
> Fix this by handling the 32-bit case directly in
> xsk_skb_destructor_set_addr(): when !CONFIG_64BIT, allocate an xsk_addrs
> struct (the same path already used for multi-descriptor SKBs) to store
> the full u64 address.
Is it easier to make XSK `depends on 64BIT` to avoid dealing with that? Does
anybody seriously run af_xdp on 32 bit systems?
^ permalink raw reply
* Re: [PATCH] gtp: disable BH before calling udp_tunnel_xmit_skb()
From: Jakub Kicinski @ 2026-04-20 19:58 UTC (permalink / raw)
To: Justin Iurman
Cc: David Carlier, Pablo Neira Ayuso, Harald Welte, Andrew Lunn,
Eric Dumazet, Paolo Abeni, Weiming Shi, osmocom-net-gprs, netdev,
linux-kernel, stable
In-Reply-To: <b44de581-9f41-4804-afb1-72c491d9443a@gmail.com>
On Mon, 20 Apr 2026 21:02:55 +0200 Justin Iurman wrote:
> On 4/17/26 07:54, David Carlier wrote:
> > gtp_genl_send_echo_req() runs as a generic netlink doit handler in
> > process context with BH not disabled. It calls udp_tunnel_xmit_skb(),
> > which eventually invokes iptunnel_xmit() — that uses __this_cpu_inc/dec
> > on softnet_data.xmit.recursion to track the tunnel xmit recursion level.
> >
> > Without local_bh_disable(), the task may migrate between
> > dev_xmit_recursion_inc() and dev_xmit_recursion_dec(), breaking the
> > per-CPU counter pairing. The result is stale or negative recursion
> > levels that can later produce false-positive
> > SKB_DROP_REASON_RECURSION_LIMIT drops on either CPU.
> >
> > The other udp_tunnel_xmit_skb() call sites in gtp.c are unaffected:
> > the data path runs under ndo_start_xmit and the echo response handlers
> > run from the UDP encap rx softirq, both with BH already disabled.
> >
> > Fix it by disabling BH around the udp_tunnel_xmit_skb() call, mirroring
> > commit 2cd7e6971fc2 ("sctp: disable BH before calling
> > udp_tunnel_xmit_skb()").
>
> Why not fix iptunnel_xmit() directly, rather than fixing all possible
> callers? Basically, jut like we did for lwtunnel_{output|xmit}(). The
> advantage would be that we no longer have to worry about BHs in the
> callers, and BHs would only be disabled when necessary.
Oops, I pushed this already. The bot hasn't caught up yet.
Let's revisit this if we find another caller in process context?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox