* [PATCH v2 2/2] sungem: validate BCM5411 PHY register reads
From: Joel @ 2026-04-24 3:33 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, linux-kernel, Joel
In-Reply-To: <20260424033348.1917-1-joelcrouch@gmail.com>
Check for -EIO and 0xffff during initialization to prevent silent failures. Add error logging to gem_init_phy to report failures.
---
drivers/net/ethernet/sun/sungem.c | 8 ++++++--
drivers/net/sungem_phy.c | 7 ++++++-
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/sun/sungem.c b/drivers/net/ethernet/sun/sungem.c
index 8e69d917d..525aba527 100644
--- a/drivers/net/ethernet/sun/sungem.c
+++ b/drivers/net/ethernet/sun/sungem.c
@@ -1707,8 +1707,12 @@ static void gem_init_phy(struct gem *gp)
sungem_phy_probe(&gp->phy_mii, gp->mii_phy_addr);
/* Init PHY */
- if (gp->phy_mii.def && gp->phy_mii.def->ops->init)
- gp->phy_mii.def->ops->init(&gp->phy_mii);
+ if (gp->phy_mii.def && gp->phy_mii.def->ops->init) {
+ int err = gp->phy_mii.def->ops->init(&gp->phy_mii);
+
+ if (err)
+ netdev_err(gp->dev, "PHY init failed: %d\n", err);
+ }
} else {
gem_pcs_reset(gp);
gem_pcs_reinit_adv(gp);
diff --git a/drivers/net/sungem_phy.c b/drivers/net/sungem_phy.c
index 44cbe1938..ec8800c93 100644
--- a/drivers/net/sungem_phy.c
+++ b/drivers/net/sungem_phy.c
@@ -296,7 +296,12 @@ static int bcm5411_init(struct mii_phy *phy)
sungem_phy_write(phy, MII_BMCR, BMCR_RESET);
sungem_phy_write(phy, MII_BMCR, 0x1340);
- data = sungem_phy_read(phy, MII_BCM5400_GB_CONTROL);
+ val = sungem_phy_read(phy, MII_BCM5400_GB_CONTROL);
+
+ if (val < 0 || val == 0xffff)
+ return -EIO;
+
+ data = (u16)val;
data |= MII_BCM5400_GB_CONTROL_FULLDUPLEXCAP;
sungem_phy_write(phy, MII_BCM5400_GB_CONTROL, data);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH bpf v2] bpf: Fix NULL pointer dereference in bpf_skb_fib_lookup()
From: patchwork-bot+netdevbpf @ 2026-04-24 3:40 UTC (permalink / raw)
To: Weiming Shi
Cc: martin.lau, daniel, ast, andrii, davem, edumazet, kuba, pabeni,
john.fastabend, sdf, eddyz87, song, yonghong.song, kpsingh,
haoluo, jolsa, horms, hawk, bpf, netdev, xmei5, paul.chaignon
In-Reply-To: <20260423183831.1325480-2-bestswngs@gmail.com>
Hello:
This patch was applied to bpf/bpf.git (master)
by Martin KaFai Lau <martin.lau@kernel.org>:
On Thu, 23 Apr 2026 11:38:32 -0700 you wrote:
> When tot_len is not provided by the user, bpf_skb_fib_lookup()
> resolves the FIB result's output device via dev_get_by_index_rcu()
> to check skb forwardability and fill in mtu_result. The returned
> pointer is dereferenced without a NULL check. If the device is
> concurrently unregistered, dev_get_by_index_rcu() returns NULL and
> is_skb_forwardable() crashes at dev->flags:
>
> [...]
Here is the summary with links:
- [bpf,v2] bpf: Fix NULL pointer dereference in bpf_skb_fib_lookup()
https://git.kernel.org/bpf/bpf/c/1081de1accb2
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH bpf v3] bpf: Fix NULL pointer dereference in bpf_sk_storage_clone and diag paths
From: patchwork-bot+netdevbpf @ 2026-04-24 3:40 UTC (permalink / raw)
To: Weiming Shi
Cc: davem, edumazet, kuba, pabeni, horms, martin.lau, ast, ameryhung,
leon.hwang, kees, wangfushuai, menglong8.dong, netdev, bpf, xmei5
In-Reply-To: <20260422065411.1007737-2-bestswngs@gmail.com>
Hello:
This patch was applied to bpf/bpf.git (master)
by Martin KaFai Lau <martin.lau@kernel.org>:
On Tue, 21 Apr 2026 23:54:12 -0700 you wrote:
> bpf_selem_unlink_nofail() sets SDATA(selem)->smap to NULL before
> removing the selem from the storage hlist. A concurrent RCU reader in
> bpf_sk_storage_clone() can observe the selem still on the list with
> smap already NULL, causing a NULL pointer dereference.
>
> general protection fault, probably for non-canonical address 0xdffffc000000000a:
> KASAN: null-ptr-deref in range [0x0000000000000050-0x0000000000000057]
> RIP: 0010:bpf_sk_storage_clone+0x1cd/0xaa0 net/core/bpf_sk_storage.c:174
> Call Trace:
> <IRQ>
> sk_clone+0xfed/0x1980 net/core/sock.c:2591
> inet_csk_clone_lock+0x30/0x760 net/ipv4/inet_connection_sock.c:1222
> tcp_create_openreq_child+0x35/0x2680 net/ipv4/tcp_minisocks.c:571
> tcp_v4_syn_recv_sock+0x123/0xf90 net/ipv4/tcp_ipv4.c:1729
> tcp_check_req+0x8e1/0x2580 include/net/tcp.h:855
> tcp_v4_rcv+0x1845/0x3b80 net/ipv4/tcp_ipv4.c:2347
>
> [...]
Here is the summary with links:
- [bpf,v3] bpf: Fix NULL pointer dereference in bpf_sk_storage_clone and diag paths
https://git.kernel.org/bpf/bpf/c/375e4e33c18d
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v2 2/2] sungem: validate BCM5411 PHY register reads
From: Andrew Lunn @ 2026-04-24 3:45 UTC (permalink / raw)
To: Joel; +Cc: netdev, linux-kernel
In-Reply-To: <20260424033348.1917-3-joelcrouch@gmail.com>
On Fri, Apr 24, 2026 at 03:33:48AM +0000, Joel wrote:
> Check for -EIO and 0xffff during initialization to prevent silent failures. Add error logging to gem_init_phy to report failures.
I assume you have the hardware?
Looking at the git history, there have not been any fixes to this
driver since 2012. Why has this problem suddenly appeared? Is you
hardware getting flaky?
Andrew
^ permalink raw reply
* Re: [PATCH] mctp i2c: check packet length before marking flow active
From: Jeremy Kerr @ 2026-04-24 4:16 UTC (permalink / raw)
To: William A. Kennington III, Matt Johnston, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Wolfram Sang
Cc: netdev, linux-kernel
In-Reply-To: <c94fc7c9-279b-4b4b-92f3-7f1b88bc0c64@wkennington.com>
Hi William,
> > Out of curiosity though, how did you hit the hdr_byte_count mismatch in
> > the first place?
>
> Our current theory is that we have known buggy firmware on our NVME MCTP
> devices and we are seeing some kind of corruption on the bus that we are
> going to fix in on the firmware side.
OK, sounds good for the overall fix, but I don't think that would be
causing the path that you're addressing here. The fix is definitely
valid, but can't be hit through any RX data corruption (we're in the
TX path).
The header byte count is populated during header construction, so a
mismatch here would indicate modification of the skb between that point
at the actual xmit. Do you see the "Bad TX len" warning in these cases?
> We started also seeing kernel
> crashes along with the bad firmware symptoms, walked through ~110 kdumps
> and found i2c locks that were held by 2 owners (eeprom reading and the
> MCTP TX queue).
Just to clarify my understanding of the state: "being held by two
owners" would indicate a violation of the lock itself. Or is it that
there are two threads blocked waiting to acquire the mutex?
For NVMe-MI, you're likely using manual tag allocation, where the tag
allocation (and hence flow state) is entirely controlled by userspace.
It may be that the NVMe protocol-level errors are causing that tags to
be held for long durations, perhaps?
Cheers,
Jeremy
^ permalink raw reply
* Re: [PATCH net-next] Documentation: net/smc: correct old value of smcr_max_recv_wr
From: Mahanta Jambigi @ 2026-04-24 5:03 UTC (permalink / raw)
To: Alexandra Winter, andrew+netdev, davem, edumazet, kuba, pabeni,
alibuda, dust.li, sidraya, wenjia
Cc: pasic, horms, tonylu, guwen, netdev, linux-s390
In-Reply-To: <b095898f-96b9-4211-aa99-58cfd5d4e538@linux.ibm.com>
On 22/04/26 5:38 pm, Alexandra Winter wrote:
> net-next is closed!
>
> On 22.04.26 10:51, Mahanta Jambigi wrote:
>> The smc-sysctl.rst documentation incorrectly stated that the previous
>> hardcoded maximum number of WR buffers on the receive path (smcr_max_recv_wr)
>> was 16. The correct historical value used before the introduction of the sysctl
>> control was 48. Update the documentation to reflect the accurate default value.
>
> s/default/historical/
> or remove the last sentence
I'll replace default with historical in v2.
^ permalink raw reply
* [bug report] 8021q: delete cleared egress QoS mappings
From: Dan Carpenter @ 2026-04-24 5:17 UTC (permalink / raw)
To: Longxuan Yu; +Cc: Simon Horman, netdev
Hello Longxuan Yu,
Commit 7dddc74af369 ("8021q: delete cleared egress QoS mappings")
from Apr 20, 2026 (linux-next), leads to the following Smatch static
checker warning:
net/8021q/vlan_dev.c:211 vlan_dev_set_egress_priority()
warn: duplicate zero check 'vlan_qos' (previous on line 200)
net/8021q/vlan_dev.c
171 int vlan_dev_set_egress_priority(const struct net_device *dev,
172 u32 skb_prio, u16 vlan_prio)
173 {
174 struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
175 struct vlan_priority_tci_mapping __rcu **mpp;
176 struct vlan_priority_tci_mapping *mp;
177 struct vlan_priority_tci_mapping *np;
178 u32 bucket = skb_prio & 0xF;
179 u32 vlan_qos = (vlan_prio << VLAN_PRIO_SHIFT) & VLAN_PRIO_MASK;
180
181 /* See if a priority mapping exists.. */
182 mpp = &vlan->egress_priority_map[bucket];
183 mp = rtnl_dereference(*mpp);
184 while (mp) {
185 if (mp->priority == skb_prio) {
186 if (!vlan_qos) {
187 rcu_assign_pointer(*mpp, rtnl_dereference(mp->next));
188 vlan->nr_egress_mappings--;
189 kfree_rcu(mp, rcu);
190 } else {
191 WRITE_ONCE(mp->vlan_qos, vlan_qos);
192 }
193 return 0;
194 }
195 mpp = &mp->next;
196 mp = rtnl_dereference(*mpp);
197 }
198
199 /* Create a new mapping then. */
200 if (!vlan_qos)
^^^^^^^^^^^^^^
The commit adds this check
201 return 0;
202
203 np = kmalloc_obj(struct vlan_priority_tci_mapping);
204 if (!np)
205 return -ENOBUFS;
206
207 np->priority = skb_prio;
208 np->vlan_qos = vlan_qos;
209 RCU_INIT_POINTER(np->next, rtnl_dereference(vlan->egress_priority_map[bucket]));
210 rcu_assign_pointer(vlan->egress_priority_map[bucket], np);
--> 211 if (vlan_qos)
^^^^^^^^^^^^^
So now this old one could be deleted.
212 vlan->nr_egress_mappings++;
213 return 0;
214 }
This email is a free service from the Smatch-CI project [smatch.sf.net].
regards,
dan carpenter
^ permalink raw reply
* [PATCH net-next v2] Documentation: net/smc: correct old value of smcr_max_recv_wr
From: Mahanta Jambigi @ 2026-04-24 5:23 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, alibuda, dust.li,
sidraya, wenjia, wintera
Cc: pasic, horms, tonylu, guwen, netdev, linux-s390, Mahanta Jambigi
The smc-sysctl.rst documentation incorrectly stated that the previous
hardcoded maximum number of WR buffers on the receive path (smcr_max_recv_wr)
was 16. The correct historical value used before the introduction of the sysctl
control was 48. Update the documentation to reflect the accurate historical
value. Also fix a couple of minor typos.
Fixes: aef3cdb47bbb net/smc: make wr buffer count configurable
Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
Reviewed-by: Sidraya Jayagond <sidraya@linux.ibm.com>
Signed-off-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
---
v2: Addressed few minor comments from Alexandra Winter.
---
Documentation/networking/smc-sysctl.rst | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/networking/smc-sysctl.rst b/Documentation/networking/smc-sysctl.rst
index a8b4f357174e..93cc6244f6c8 100644
--- a/Documentation/networking/smc-sysctl.rst
+++ b/Documentation/networking/smc-sysctl.rst
@@ -86,7 +86,7 @@ smcr_max_send_wr - INTEGER
Please be aware that all the buffers need to be allocated as a physically
continuous array in which each element is a single buffer and has the size
of SMC_WR_BUF_SIZE (48) bytes. If the allocation fails, we keep retrying
- with half of the buffer count until it is ether successful or (unlikely)
+ with half of the buffer count until it is either successful or (unlikely)
we dip below the old hard coded value which is 16 where we give up much
like before having this control.
@@ -100,14 +100,14 @@ smcr_max_recv_wr - INTEGER
depending on the workload it can be a bottleneck in a sense that threads
have to wait for work request buffers to become available. Before the
introduction of this control the maximal number of work request buffers
- available on the receive path used to be hard coded to 16. With this control
+ available on the receive path used to be hard coded to 48. With this control
it becomes configurable. The acceptable range is between 2 and 2048.
Please be aware that all the buffers need to be allocated as a physically
continuous array in which each element is a single buffer and has the size
of SMC_WR_BUF_SIZE (48) bytes. If the allocation fails, we keep retrying
- with half of the buffer count until it is ether successful or (unlikely)
- we dip below the old hard coded value which is 16 where we give up much
+ with half of the buffer count until it is either successful or (unlikely)
+ we dip below the old hard coded value which is 48 where we give up much
like before having this control.
Default: 48
--
2.51.0
^ permalink raw reply related
* Re: [PATCH bpf-next v3 1/9] bpf: Unify dynptr handling in the verifier
From: Amery Hung @ 2026-04-24 5:34 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, netdev, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
martin.lau, mykyta.yatsenko5, kernel-team
In-Reply-To: <CAEf4BzZGg1EKwYFG6QKrkNbiE6RwWKew_Zz8mz9rjshKjWzcKA@mail.gmail.com>
On Thu, Apr 23, 2026 at 5:04 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Tue, Apr 21, 2026 at 3:10 PM Amery Hung <ameryhung@gmail.com> wrote:
> >
> > Simplify dynptr checking for helper and kfunc by unifying it. Remember
> > the initialized dynptr (i.e.,g !(arg_type |= MEM_UNINIT)) pass to a
> > dynptr kfunc during process_dynptr_func() so that we can easily
> > retrieve the information for verification later. By saving it in
> > meta->dynptr, there is no need to call dynptr helpers such as
> > dynptr_id(), dynptr_ref_obj_id() and dynptr_type() in check_func_arg().
> >
> > Remove and open code the helpers in process_dynptr_func() when
> > saving id, ref_obj_id, and type. It is okay to drop spi < 0 check as
> > is_dynptr_reg_valid_init() has made sure the dynptr is valid.
> >
> > Besides, since dynptr ref_obj_id information is now pass around in
> > meta->bpf_dynptr_desc, drop the check in helper_multiple_ref_obj_use.
> >
> > Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> > Signed-off-by: Amery Hung <ameryhung@gmail.com>
> > ---
> > include/linux/bpf_verifier.h | 12 ++-
> > kernel/bpf/verifier.c | 178 +++++++----------------------------
> > 2 files changed, 41 insertions(+), 149 deletions(-)
> >
>
> [...]
>
> > @@ -7433,7 +7426,8 @@ static int process_kptr_func(struct bpf_verifier_env *env, int regno,
> > * and checked dynamically during runtime.
> > */
> > static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn_idx,
> > - enum bpf_arg_type arg_type, int clone_ref_obj_id)
> > + enum bpf_arg_type arg_type, int clone_ref_obj_id,
> > + struct bpf_dynptr_desc *dynptr)
> > {
> > struct bpf_reg_state *reg = reg_state(env, regno);
> > int err;
> > @@ -7499,6 +7493,20 @@ static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn
> > }
> >
> > err = mark_dynptr_read(env, reg);
>
> if mark_dynptr_read() fails, should we exit instead of proceeding to
> fill out dynptr info?.. even if mark_dynptr_read() cannot fail because
> of is_dynptr_reg_valid_init(), it still looks wrong and error-prone
I think I will change mark_dynptr_read() to receive spi from the
caller and make the function return void (It is technically nofail
after liveness tracking code is removed). Then, move dynptr_get_spi()
before it and don't error out. A comment seems to be enough.
/* Cannot fail as we already make sure reg is valid dynptr */
spi = dynptr_get_spi(env, reg);
mark_dynptr_read(env, reg, spi);
>
> > +
> > + if (dynptr) {
> > + struct bpf_func_state *state = bpf_func(env, reg);
> > + int spi;
> > +
> > + if (reg->type != CONST_PTR_TO_DYNPTR) {
> > + spi = dynptr_get_spi(env, reg);
^ move this up
> > + reg = &state->stack[spi].spilled_ptr;
> > + }
> > +
> > + dynptr->id = reg->id;
> > + dynptr->type = reg->dynptr.type;
> > + dynptr->ref_obj_id = reg->ref_obj_id;
> > + }
> > }
> > return err;
> > }
>
> [...]
^ permalink raw reply
* [PATCH net v4 0/8] xsk: fix bugs around xsk skb allocation
From: Jason Xing @ 2026-04-24 5:38 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, aleksander.lobakin
Cc: bpf, netdev, Jason Xing
From: Jason Xing <kernelxing@tencent.com>
There are rare issues around xsk_build_skb(). Some of them
were founded by Sashiko[1][2].
[1]: https://lore.kernel.org/all/20260415082654.21026-1-kerneljasonxing@gmail.com/
[2]: https://lore.kernel.org/all/20260418045644.28612-1-kerneljasonxing@gmail.com/
---
v4
Link: https://lore.kernel.org/all/20260422033650.68457-1-kerneljasonxing@gmail.com/
1. fix 32-bit arch issue in patch 8 (Alexander && Maciej)
2. add acked-by tags (Stan)
v3
Link: https://lore.kernel.org/all/20260420082805.14844-1-kerneljasonxing@gmail.com/
1. use !xs->skb as the indicator of first frag in patch 3 (Stan)
2. move init skb after it's allocated successfully, so patch 4,5,6 have
been adjusted accordingly (Stan)
3. do not support 32-bit arch (Stan)
4. add acked-by tags (Stan)
v2
Link: https://lore.kernel.org/all/20260418045644.28612-1-kerneljasonxing@gmail.com/#t
1. add four patches spotted by sashiko to fix buggy pre-existing
behavior
2. adjust the order of 8 patches.
Jason Xing (8):
xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices
xsk: handle NULL dereference of the skb without frags issue
xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path
xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()
xsk: avoid skb leak in XDP_TX_METADATA case
xsk: free the skb when hitting the upper bound MAX_SKB_FRAGS
xsk: fix xsk_addrs slab leak on multi-buffer error path
xsk: fix u64 descriptor address truncation on 32-bit architectures
net/xdp/xsk.c | 65 +++++++++++++++++++++++++++++++----------
net/xdp/xsk_buff_pool.c | 3 ++
2 files changed, 52 insertions(+), 16 deletions(-)
--
2.41.3
^ permalink raw reply
* [PATCH net v4 1/8] xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices
From: Jason Xing @ 2026-04-24 5:38 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, aleksander.lobakin
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260424053816.27965-1-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")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
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
^ permalink raw reply related
* [PATCH net v4 2/8] xsk: handle NULL dereference of the skb without frags issue
From: Jason Xing @ 2026-04-24 5:38 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, aleksander.lobakin
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260424053816.27965-1-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")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
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
^ permalink raw reply related
* [PATCH net v4 3/8] xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path
From: Jason Xing @ 2026-04-24 5:38 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, aleksander.lobakin
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260424053816.27965-1-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 using a !xs->skb check to handle first frag situation, 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()")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
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..d23d1b14b8b4 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)
kfree_skb(skb);
if (err == -EOVERFLOW) {
--
2.41.3
^ permalink raw reply related
* [PATCH net v4 4/8] xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()
From: Jason Xing @ 2026-04-24 5:38 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, aleksander.lobakin
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260424053816.27965-1-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.
Postpone the init phase when we believe the allocation of first frag is
successfully completed. Before this init, skb can be safely freed by
kfree_skb().
Closes: https://lore.kernel.org/all/20260419045822.843BFC2BCAF@smtp.kernel.org/
Fixes: c30d084960cf ("xsk: avoid overwriting skb fields for multi-buffer traffic")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index d23d1b14b8b4..88ec6d2cbbcf 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -739,8 +739,6 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
return ERR_PTR(err);
skb_reserve(skb, hr);
-
- xsk_skb_init_misc(skb, xs, desc->addr);
if (desc->options & XDP_TX_METADATA) {
err = xsk_skb_metadata(skb, buffer, desc, pool, hr);
if (unlikely(err))
@@ -834,7 +832,6 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
if (unlikely(err))
goto free_err;
- xsk_skb_init_misc(skb, xs, desc->addr);
if (desc->options & XDP_TX_METADATA) {
err = xsk_skb_metadata(skb, buffer, desc,
xs->pool, hr);
@@ -884,6 +881,8 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
}
}
+ if (!xs->skb)
+ xsk_skb_init_misc(skb, xs, desc->addr);
xsk_inc_num_desc(skb);
return skb;
--
2.41.3
^ permalink raw reply related
* [PATCH net v4 5/8] xsk: avoid skb leak in XDP_TX_METADATA case
From: Jason Xing @ 2026-04-24 5:38 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, aleksander.lobakin
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260424053816.27965-1-kerneljasonxing@gmail.com>
From: Jason Xing <kernelxing@tencent.com>
Fix it by explicitly adding kfree_skb() before returning back to its
caller.
How to reproduce it in virtio_net:
1. the current skb is the first one (which means no frag and xs->skb is
NULL) and users enable metadata feature.
2. xsk_skb_metadata() returns a error code.
3. the caller xsk_build_skb() clears skb by using 'skb = NULL;'.
4. there is no chance to free this skb anymore.
Closes: https://lore.kernel.org/all/20260415085204.3F87AC19424@smtp.kernel.org/
Fixes: 30c3055f9c0d ("xsk: wrap generic metadata handling onto separate function")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 88ec6d2cbbcf..c49b58199d2f 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -741,8 +741,10 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
skb_reserve(skb, hr);
if (desc->options & XDP_TX_METADATA) {
err = xsk_skb_metadata(skb, buffer, desc, pool, hr);
- if (unlikely(err))
+ if (unlikely(err)) {
+ kfree_skb(skb);
return ERR_PTR(err);
+ }
}
} else {
struct xsk_addrs *xsk_addr;
--
2.41.3
^ permalink raw reply related
* [PATCH net v4 6/8] xsk: free the skb when hitting the upper bound MAX_SKB_FRAGS
From: Jason Xing @ 2026-04-24 5:38 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, aleksander.lobakin
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260424053816.27965-1-kerneljasonxing@gmail.com>
From: Jason Xing <kernelxing@tencent.com>
Fix it by explicitly adding kfree_skb() before returning back to its
caller.
How to reproduce it in virtio_net:
1. the current skb is the first one (which means xs->skb is NULL) and
hit the limit MAX_SKB_FRAGS.
2. xsk_build_skb_zerocopy() returns -EOVERFLOW.
3. the caller xsk_build_skb() clears skb by using 'skb = NULL;'. This
is why bug can be triggered.
4. there is no chance to free this skb anymore.
Note that if in this case the xs->skb is not NULL, xsk_build_skb() will
call xsk_drop_skb(xs->skb) to do the right thing.
Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index c49b58199d2f..5e6326e076ab 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -776,8 +776,11 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
addr = buffer - pool->addrs;
for (copied = 0, i = skb_shinfo(skb)->nr_frags; copied < len; i++) {
- if (unlikely(i >= MAX_SKB_FRAGS))
+ if (unlikely(i >= MAX_SKB_FRAGS)) {
+ if (!xs->skb)
+ kfree_skb(skb);
return ERR_PTR(-EOVERFLOW);
+ }
page = pool->umem->pgs[addr >> PAGE_SHIFT];
get_page(page);
--
2.41.3
^ permalink raw reply related
* [PATCH net v4 7/8] xsk: fix xsk_addrs slab leak on multi-buffer error path
From: Jason Xing @ 2026-04-24 5:38 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, aleksander.lobakin
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260424053816.27965-1-kerneljasonxing@gmail.com>
From: Jason Xing <kernelxing@tencent.com>
When xsk_build_skb() / xsk_build_skb_zerocopy() sees the first
continuation descriptor, it promotes destructor_arg from an inlined
address to a freshly allocated xsk_addrs (num_descs = 1). The counter
is bumped to >= 2 only at the very end of a successful build (by calling
xsk_inc_num_desc()).
If the build fails in between (e.g. alloc_page() returns NULL with
-EAGAIN, or the MAX_SKB_FRAGS overflow hits), we jump to free_err, skip
calling xsk_inc_num_desc() to increment num_descs and leave the half-built
skb attached to xs->skb for the app to retry. The skb now has
1) destructor_arg = a real xsk_addrs pointer,
2) num_descs = 1
If the app never retries and just close()s the socket, xsk_release()
calls xsk_drop_skb() -> xsk_consume_skb(), which decides whether to
free xsk_addrs by testing num_descs > 1:
if (unlikely(num_descs > 1))
kmem_cache_free(xsk_tx_generic_cache, destructor_arg);
Because num_descs is exactly 1 the branch is skipped and the
xsk_addrs object is leaked to the xsk_tx_generic_cache slab.
Fix it by directly testing if destructor_arg is still addr. Or else it
is modified and used to store the newly allocated memory from
xsk_tx_generic_cache regardless of increment of num_desc, which we
need to handle.
Closes: https://lore.kernel.org/all/20260419045824.D9E5EC2BCAF@smtp.kernel.org/
Fixes: 0ebc27a4c67d ("xsk: avoid data corruption on cq descriptor number")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 5e6326e076ab..ed96f6ec8ff2 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -605,7 +605,7 @@ static void xsk_cq_submit_addr_locked(struct xsk_buff_pool *pool,
spin_lock_irqsave(&pool->cq_prod_lock, flags);
idx = xskq_get_prod(pool->cq);
- if (unlikely(num_descs > 1)) {
+ if (unlikely(!xsk_skb_destructor_is_addr(skb))) {
xsk_addr = (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;
for (i = 0; i < num_descs; i++) {
@@ -660,7 +660,7 @@ static void xsk_consume_skb(struct sk_buff *skb)
u32 num_descs = xsk_get_num_desc(skb);
struct xsk_addrs *xsk_addr;
- if (unlikely(num_descs > 1)) {
+ if (unlikely(!xsk_skb_destructor_is_addr(skb))) {
xsk_addr = (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;
kmem_cache_free(xsk_tx_generic_cache, xsk_addr);
}
--
2.41.3
^ permalink raw reply related
* [PATCH net v4 8/8] xsk: fix u64 descriptor address truncation on 32-bit architectures
From: Jason Xing @ 2026-04-24 5:38 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, aleksander.lobakin
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260424053816.27965-1-kerneljasonxing@gmail.com>
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 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 in the destructor_arg helpers:
- xsk_skb_destructor_set_addr(): on !CONFIG_64BIT, allocate an
xsk_addrs struct via kmem_cache_zalloc() to store the full u64
address. Leave num_descs as 0 (zalloc) so that the subsequent
xsk_inc_num_desc() brings it to the correct count of 1.
- xsk_skb_destructor_is_addr(): on !CONFIG_64BIT, return true only
when destructor_arg is NULL (not yet set), false when it points to
an xsk_addrs struct.
- xsk_skb_init_misc(): call xsk_skb_destructor_set_addr() first
before touching any other skb fields; on failure return early so
the skb destructor is never changed from sock_wfree.
The existing xsk_consume_skb() already handles 32-bit correctly after
these changes: xsk_skb_destructor_is_addr() returns false for any
allocated xsk_addrs, so the kmem_cache_free path is always taken.
The overhead is one extra kmem_cache_zalloc per first descriptor on
32-bit only; 64-bit builds are completely unchanged.
Closes: https://lore.kernel.org/all/20260419045824.D9E5EC2BCAF@smtp.kernel.org/
Fixes: 0ebc27a4c67d ("xsk: avoid data corruption on cq descriptor number")
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 38 +++++++++++++++++++++++++++++++-------
1 file changed, 31 insertions(+), 7 deletions(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index ed96f6ec8ff2..fe88f47741b5 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -558,7 +558,10 @@ static int xsk_cq_reserve_locked(struct xsk_buff_pool *pool)
static bool xsk_skb_destructor_is_addr(struct sk_buff *skb)
{
- return (uintptr_t)skb_shinfo(skb)->destructor_arg & 0x1UL;
+ if (IS_ENABLED(CONFIG_64BIT))
+ return (uintptr_t)skb_shinfo(skb)->destructor_arg & 0x1UL;
+ else
+ return !skb_shinfo(skb)->destructor_arg;
}
static u64 xsk_skb_destructor_get_addr(struct sk_buff *skb)
@@ -566,9 +569,21 @@ static u64 xsk_skb_destructor_get_addr(struct sk_buff *skb)
return (u64)((uintptr_t)skb_shinfo(skb)->destructor_arg & ~0x1UL);
}
-static void xsk_skb_destructor_set_addr(struct sk_buff *skb, u64 addr)
+static int xsk_skb_destructor_set_addr(struct sk_buff *skb, u64 addr)
{
+ if (!IS_ENABLED(CONFIG_64BIT)) {
+ struct xsk_addrs *xsk_addr;
+
+ xsk_addr = kmem_cache_zalloc(xsk_tx_generic_cache, GFP_KERNEL);
+ if (!xsk_addr)
+ return -ENOMEM;
+ xsk_addr->addrs[0] = addr;
+ skb_shinfo(skb)->destructor_arg = (void *)xsk_addr;
+ return 0;
+ }
+
skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t)addr | 0x1UL);
+ return 0;
}
static void xsk_inc_num_desc(struct sk_buff *skb)
@@ -644,14 +659,20 @@ void xsk_destruct_skb(struct sk_buff *skb)
sock_wfree(skb);
}
-static void xsk_skb_init_misc(struct sk_buff *skb, struct xdp_sock *xs,
- u64 addr)
+static int xsk_skb_init_misc(struct sk_buff *skb, struct xdp_sock *xs,
+ u64 addr)
{
+ int err;
+
+ err = xsk_skb_destructor_set_addr(skb, addr);
+ if (err)
+ return err;
+
skb->dev = xs->dev;
skb->priority = READ_ONCE(xs->sk.sk_priority);
skb->mark = READ_ONCE(xs->sk.sk_mark);
skb->destructor = xsk_destruct_skb;
- xsk_skb_destructor_set_addr(skb, addr);
+ return 0;
}
static void xsk_consume_skb(struct sk_buff *skb)
@@ -886,8 +907,11 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
}
}
- if (!xs->skb)
- xsk_skb_init_misc(skb, xs, desc->addr);
+ if (!xs->skb) {
+ err = xsk_skb_init_misc(skb, xs, desc->addr);
+ if (unlikely(err))
+ goto free_err;
+ }
xsk_inc_num_desc(skb);
return skb;
--
2.41.3
^ permalink raw reply related
* Re: [PATCH v1 1/2] vfio: add callback to get tph info for dma-buf
From: Zhiping Zhang @ 2026-04-24 5:41 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Alex Williamson, Stanislav Fomichev, Keith Busch, Leon Romanovsky,
Bjorn Helgaas, linux-rdma, linux-pci, netdev, dri-devel,
Yochai Cohen, Yishai Hadas
In-Reply-To: <20260423224626.GV3611611@ziepe.ca>
On Thu, Apr 23, 2026 at 3:46 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
>
> >
> On Thu, Apr 23, 2026 at 01:20:16PM -0600, Alex Williamson wrote:
>
> > My suggestion would be that we leave VFIO_DEVICE_FEATURE_DMA_BUF
> > unchanged and add a VFIO_DEVICE_FEATURE_DMA_BUF_TPH ioctl which takes
> > the fd from VFIO_DEVICE_FEATURE_DMA_BUF, along with a steering tag and
> > processing hint. It would fdget() the dmabuf fd, validate it's a
> > dmabuf via f_ops, validate it's a vfio exported dmabuf via dmabuf->ops,
> > find the matching vfio_pci_dma_buf via priv under memory_lock, and
> > stuff the provided TPH values into the object. It would be left to the
> > user to sequence setting the TPH values on the dmabuf before the dmabuf
> > is consumed by the importer.
> >
> > Is that a more reasonable uAPI? Thanks,
>
> Off hand I think it can work, with the proviso that if userspace uses
> the dmabuf before setting the tph the importer may ignore it. I don't
> think that is a problem in practice.
>
> Jason
Thanks Alex and Jason!
I agree that the separate feature sounds like a cleaner design and could avoid
the uAPI complications. For v2 I'll:
- Leave VFIO_DEVICE_FEATURE_DMA_BUF unchanged,
- Add VFIO_DEVICE_FEATURE_DMA_BUF_TPH as a SET-only feature that
takes the dmabuf fd, steering tag, and processing hint,
- Keep the dma_buf_ops.get_tph callback as-is.
and then the userspace sequence would become:
1. fd = VFIO_DEVICE_FEATURE_DMA_BUF (create dmabuf, unchanged)
2. VFIO_DEVICE_FEATURE_DMA_BUF_TPH (set TPH on fd)
3. pass fd to importer
Zhiping
^ permalink raw reply
* Re: [PATCH bpf-next v3 0/9] Refactor verifier object relationship tracking
From: Amery Hung @ 2026-04-24 5:44 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, netdev, andrii, daniel, eddyz87, memxor, martin.lau,
mykyta.yatsenko5, kernel-team
In-Reply-To: <DI0Z4SJ0TXOA.2N0IY94AF254F@gmail.com>
On Thu, Apr 23, 2026 at 5:55 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue Apr 21, 2026 at 3:10 PM PDT, Amery Hung wrote:
> >
> > (3) Referenced dynptr with referenced parent:
> >
> > file (1,1,0)
> > ^ ^
> > bpf_dynptr_from_file | +-------------------------------+
> > | bpf_dynptr_clone(A, C) |
> > dynptr A (2,3,1) dynptr C (4,3,1)
> > ^ ^
> > | |
> > dynptr A and C have the same lifetime
>
> If I understand the proposal correctly above should be:
>
> file (1,0,0)
> ^
> bpf_dynptr_from_file +-------------------------------+
>
> ref_obj_id will be inited bpf_dynptr_clone() as 3 into A and C.
> but original file->ref_obj_id will be zero.
>
> ?
This is the existing behavior. bpf_get_task_exe_file() is tagged with
(KF_ACQUIRE | KF_RET_NULL). Therefore, file will have id and
ref_obj_id both initialized to the same value. Here is the
corresponding verifier code:
if (is_kfunc_acquire(&meta)) {
int id = acquire_reference(env, insn_idx);
if (id < 0)
return id;
if (is_kfunc_ret_null(&meta))
regs[BPF_REG_0].id = id;
regs[BPF_REG_0].ref_obj_id = id;
^ permalink raw reply
* [PATCH net] net: mana: Optimize irq affinity for low vcpu configs
From: Shradha Gupta @ 2026-04-24 6:17 UTC (permalink / raw)
To: Dexuan Cui, Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Konstantin Taranov, Simon Horman, Erni Sri Satya Vennela,
Dipayaan Roy, Shiraz Saleem, Michael Kelley, Long Li, Yury Norov
Cc: Shradha Gupta, linux-hyperv, linux-kernel, netdev, Paul Rosswurm,
Shradha Gupta, Saurabh Singh Sengar, stable
In mana driver, the number of IRQs allocated are capped by the
min(num_cpu + 1, queue count). In cases, where the IRQ count is greater
than the vcpu count, we want to utilize all the vcpus, irrespective of
their NUMA/core bindings.
This is important, especially in the envs where number of vcpus are so
few that the softIRQ handling overhead on two IRQs on the same vcpu is
much more than their overheads if they were spread across sibling vcpus
This behaviour is more evident with dynamic IRQ allocation. Since MANA
IRQs are assigned at a later stage compared to static allocation, other
device IRQs may already be affinitized to the vCPUs. As a result, IRQ
weights become imbalanced, causing multiple MANA IRQs to land on the
same vCPU.
In such cases when many parallel TCP connections are tested, the
throughput drops significantly
Test envs:
=======================================================
Case 1: without this patch
=======================================================
4 vcpu(2 cores), 5 MANA IRQs (1 HWC + 4 Queue)
TYPE effective vCPU aff
=======================================================
IRQ0: HWC 0
IRQ1: mana_q1 0
IRQ2: mana_q2 2
IRQ3: mana_q3 0
IRQ4: mana_q4 3
%soft on each vCPU(mpstat -P ALL 1) on receiver
vCPU 0 1 2 3
=======================================================
pass 1: 38.85 0.03 24.89 24.65
pass 2: 39.15 0.03 24.57 25.28
pass 3: 40.36 0.03 23.20 23.17
=======================================================
Case 2: with this patch
=======================================================
4 vcpu(2 cores), 5 MANA IRQs (1 HWC + 4 Queue)
TYPE effective vCPU aff
=======================================================
IRQ0: HWC 0
IRQ1: mana_q1 0
IRQ2: mana_q2 1
IRQ3: mana_q3 2
IRQ4: mana_q4 3
%soft on each vCPU(mpstat -P ALL 1) on receiver
vCPU 0 1 2 3
=======================================================
pass 1: 15.42 15.85 14.99 14.51
pass 2: 15.53 15.94 15.81 15.93
pass 3: 16.41 16.35 16.40 16.36
=======================================================
Throughput Impact(in Gbps, same env)
=======================================================
TCP conn with patch w/o patch
20480 15.65 7.73
10240 15.63 8.93
8192 15.64 9.69
6144 15.64 13.16
4096 15.69 15.75
2048 15.69 15.83
1024 15.71 15.28
Fixes: 755391121038 ("net: mana: Allocate MSI-X vectors dynamically")
Cc: stable@vger.kernel.org
Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
.../net/ethernet/microsoft/mana/gdma_main.c | 35 +++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 098fbda0d128..433c044d53c6 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1672,6 +1672,23 @@ static int irq_setup(unsigned int *irqs, unsigned int len, int node,
return 0;
}
+static int irq_setup_linear(unsigned int *irqs, unsigned int len)
+{
+ int cpu;
+
+ rcu_read_lock();
+ for_each_online_cpu(cpu) {
+ if (len <= 0)
+ break;
+
+ irq_set_affinity_and_hint(*irqs++, cpumask_of(cpu));
+ len--;
+ }
+ rcu_read_unlock();
+
+ return 0;
+}
+
static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
{
struct gdma_context *gc = pci_get_drvdata(pdev);
@@ -1722,10 +1739,24 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
* first CPU sibling group since they are already affinitized to HWC IRQ
*/
cpus_read_lock();
- if (gc->num_msix_usable <= num_online_cpus())
+ if (gc->num_msix_usable <= num_online_cpus()) {
skip_first_cpu = true;
+ err = irq_setup(irqs, nvec, gc->numa_node, skip_first_cpu);
+ } else {
+ /*
+ * In case our IRQs are more than num_online_cpus, we try to
+ * make sure we are using all vcpus. In such a case NUMA or
+ * CPU core affinity does not matter.
+ * Note that in this case the total mana IRQ should always be
+ * num_online_cpu + 1. The first HWC IRQ is already handled
+ * in HWC setup calls
+ * So, the nvec value in this path should always be equal to
+ * num_online_cpu
+ */
+ WARN_ON(nvec > num_online_cpus());
+ err = irq_setup_linear(irqs, nvec);
+ }
- err = irq_setup(irqs, nvec, gc->numa_node, skip_first_cpu);
if (err) {
cpus_read_unlock();
goto free_irq;
base-commit: e728258debd553c95d2e70f9cd97c9fde27c7130
--
2.34.1
^ permalink raw reply related
* [PATCH iwl-net] ice: only free LL TS IRQ when the handler is present
From: Aleksandr Loktionov @ 2026-04-24 6:19 UTC (permalink / raw)
To: intel-wired-lan, anthony.l.nguyen, aleksandr.loktionov
Cc: netdev, Sergey Temerkhanov
From: Sergey Temerkhanov <sergey.temerkhanov@intel.com>
Free LL TS IRQ handler only when the handler was previously installed.
Unguarded calls to ice_free_irq_msix_ll_ts() may result in a double
free when the LL TS interrupt is not supported by the firmware because
ll_ts_irq.index is zero-initialised and would pass the index >= 0 check.
Track whether the LL TS IRQ was successfully requested by initialising
ll_ts_irq.index to -ENOENT in ice_init_pf() and when taking the
"not supported" skip path or on request failure, then guard the free
paths with ll_ts_irq.index >= 0.
Fixes: 82e71b226e0e ("ice: Enable SW interrupt from FW for LL TS")
Signed-off-by: Sergey Temerkhanov <sergey.temerkhanov@intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 3c36e36..cc3743a 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3389,7 +3389,7 @@ static void ice_free_irq_msix_misc(struct ice_pf *pf)
devm_free_irq(ice_pf_to_dev(pf), misc_irq_num, pf);
ice_free_irq(pf, pf->oicr_irq);
- if (pf->hw.dev_caps.ts_dev_info.ts_ll_int_read)
+ if (pf->ll_ts_irq.index >= 0)
ice_free_irq_msix_ll_ts(pf);
}
@@ -3473,8 +3473,10 @@ static int ice_req_irq_msix_misc(struct ice_pf *pf)
}
/* reserve one vector in irq_tracker for ll_ts interrupt */
- if (!pf->hw.dev_caps.ts_dev_info.ts_ll_int_read)
+ if (!pf->hw.dev_caps.ts_dev_info.ts_ll_int_read) {
+ pf->ll_ts_irq.index = -ENOENT;
goto skip_req_irq;
+ }
irq = ice_alloc_irq(pf, false);
if (irq.index < 0)
@@ -3487,6 +3489,7 @@ static int ice_req_irq_msix_misc(struct ice_pf *pf)
dev_err(dev, "devm_request_irq for %s failed: %d\n",
pf->int_name_ll_ts, err);
ice_free_irq(pf, pf->ll_ts_irq);
+ pf->ll_ts_irq.index = -ENOENT;
return err;
}
@@ -3496,7 +3499,7 @@ static int ice_req_irq_msix_misc(struct ice_pf *pf)
ice_ena_ctrlq_interrupts(hw, pf->oicr_irq.index);
/* This enables LL TS interrupt */
pf_intr_start_offset = rd32(hw, PFINT_ALLOC) & PFINT_ALLOC_FIRST;
- if (pf->hw.dev_caps.ts_dev_info.ts_ll_int_read)
+ if (pf->ll_ts_irq.index >= 0)
wr32(hw, PFINT_SB_CTL,
((pf->ll_ts_irq.index + pf_intr_start_offset) &
PFINT_SB_CTL_MSIX_INDX_M) | PFINT_SB_CTL_CAUSE_ENA_M);
@@ -4090,6 +4093,7 @@ int ice_init_pf(struct ice_pf *pf)
* the misc functionality and queue processing is combined in
* the same vector and that gets setup at open.
*/
+ pf->ll_ts_irq.index = -ENOENT;
err = ice_req_irq_msix_misc(pf);
if (err) {
dev_err(dev, "setup of misc vector failed: %d\n", err);
--
2.52.0
^ permalink raw reply related
* Re: [PATCH bpf-next v3 4/9] bpf: Refactor object relationship tracking and fix dynptr UAF bug
From: Amery Hung @ 2026-04-24 6:46 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Mykyta Yatsenko, bpf, netdev, alexei.starovoitov, andrii, daniel,
eddyz87, memxor, martin.lau, kernel-team
In-Reply-To: <CAEf4BzYuaA0xDm9WXv0vnorOgzmzsD-Rf6R3w5kus5Uo37SBFg@mail.gmail.com>
On Thu, Apr 23, 2026 at 5:04 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Apr 23, 2026 at 11:44 AM Amery Hung <ameryhung@gmail.com> wrote:
> >
> > On Thu, Apr 23, 2026 at 11:19 AM Mykyta Yatsenko
> > <mykyta.yatsenko5@gmail.com> wrote:
> > >
> > >
> > >
> > > On 4/21/26 11:10 PM, Amery Hung wrote:
> > > > Refactor object relationship tracking in the verifier and fix a dynptr
> > > > use-after-free bug where file/skb dynptrs are not invalidated when the
> > > > parent referenced object is freed.
> > > >
> > > > Add parent_id to bpf_reg_state to precisely track child-parent
> > > > relationships. A child object's parent_id points to the parent object's
> > > > id. This replaces the PTR_TO_MEM-specific dynptr_id and does not
> > > > increase the size of bpf_reg_state on 64-bit machines as there is
> > > > existing padding.
> > > >
> > > > When calling dynptr constructors (i.e., process_dynptr_func() with
> > > > MEM_UNINIT argument), track the parent's id if the parent is a
> > > > referenced object. This only applies to file dynptr and skb dynptr,
> > > > so only pass parent reg->id to kfunc constructors.
> > > >
> > > > For release_reference(), invalidating an object now also invalidates
> > > > all descendants by traversing the object tree. This is done using
> > > > stack-based DFS to avoid recursive call chains of release_reference() ->
> > > > unmark_stack_slots_dynptr() -> release_reference(). Referenced objects
> > > > encountered during tree traversal cannot be indirectly released. They
> > > > require an explicit helper/kfunc call to release the acquired resources.
> > > >
> > > > While the new design changes how object relationships are tracked in
> > > > the verifier, it does not change the verifier's behavior. Here is the
> > > > implication for dynptr, pointer casting, and owning/non-owning
> > > > references:
> > > >
> > > > Dynptr:
> > > >
> > > > When initializing a dynptr, referenced dynptrs acquire a reference for
> > > > ref_obj_id. If the dynptr has a referenced parent, parent_id tracks the
> > > > parent's id. When cloning, ref_obj_id and parent_id are copied from the
> > > > original. Releasing a referenced dynptr via release_reference(ref_obj_id)
> > > > invalidates all clones and derived slices. For non-referenced dynptrs,
> > > > only the specific dynptr and its children are invalidated.
> > > >
> > > > Pointer casting:
> > > >
> > > > Referenced socket pointers and their casted counterparts share the same
> > > > lifetime but have different nullness — they have different id but the
> > > > same ref_obj_id.
> > > >
> > > > Owning to non-owning reference conversion:
> > > >
> > > > After converting owning to non-owning by clearing ref_obj_id (e.g.,
> > > > object(id=1, ref_obj_id=1) -> object(id=1, ref_obj_id=0)), the
> > > > verifier only needs to release the reference state, so it calls
> > > > release_reference_nomark() instead of release_reference().
> > > >
> > > > Note that the error message "reference has not been acquired before" in
> > > > the helper and kfunc release paths is removed. This message was already
> > > > unreachable. The verifier only calls release_reference() after
> > > > confirming meta.ref_obj_id is valid, so the condition could never
> > > > trigger in practice (no selftest exercises it either). With the
> > > > refactor, release_reference() can now be called with non-acquired ids
> > > > and have different error conditions. Report directly in
> > > > release_reference() instead.
> > > >
> > > > Fixes: 870c28588afa ("bpf: net_sched: Add basic bpf qdisc kfuncs")
> > > > Signed-off-by: Amery Hung <ameryhung@gmail.com>
> > > > ---
> > > > include/linux/bpf_verifier.h | 22 ++-
> > > > kernel/bpf/log.c | 4 +-
> > > > kernel/bpf/states.c | 9 +-
> > > > kernel/bpf/verifier.c | 264 +++++++++++++++++------------------
> > > > 4 files changed, 152 insertions(+), 147 deletions(-)
> > > >
>
> [...]
>
> > > > @@ -673,82 +675,56 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_
> > > > mark_dynptr_stack_regs(env, &state->stack[spi].spilled_ptr,
> > > > &state->stack[spi - 1].spilled_ptr, type);
> > > >
> > > > - if (dynptr_type_refcounted(type)) {
> > > > - /* The id is used to track proper releasing */
> > > > - int id;
> > > > -
> > > > - if (clone_ref_obj_id)
> > > > - id = clone_ref_obj_id;
> > > > - else
> > > > - id = acquire_reference(env, insn_idx);
> > > > -
> > > > - if (id < 0)
> > > > - return id;
> > > > -
> > > > - state->stack[spi].spilled_ptr.ref_obj_id = id;
> > > > - state->stack[spi - 1].spilled_ptr.ref_obj_id = id;
>
> is id assigned somewhere else now? where?
>
> > > > + if (dynptr->type == BPF_DYNPTR_TYPE_INVALID) { /* dynptr constructors */
> > > > + if (dynptr_type_referenced(type)) {
> > > > + ref_obj_id = acquire_reference(env, insn_idx);
> > > > + if (ref_obj_id < 0)
> > > > + return ref_obj_id;
> > > > + }
> > > > + } else { /* bpf_dynptr_clone() */
> > > > + ref_obj_id = dynptr->ref_obj_id;
> > > > + parent_id = dynptr->parent_id;
> > > > }
> > > >
> > > > + state->stack[spi].spilled_ptr.ref_obj_id = ref_obj_id;
> > > > + state->stack[spi - 1].spilled_ptr.ref_obj_id = ref_obj_id;
> > > > + state->stack[spi].spilled_ptr.parent_id = parent_id;
> > > > + state->stack[spi - 1].spilled_ptr.parent_id = parent_id;
> > > > +
> > > > return 0;
> > > > }
> > > >
>
> [...]
>
> > > > -/* The pointer with the specified id has released its reference to kernel
> > > > - * resources. Identify all copies of the same pointer and clear the reference.
> > > > - *
> > > > - * This is the release function corresponding to acquire_reference(). Idempotent.
> > > > - */
> > > > -static int release_reference(struct bpf_verifier_env *env, int ref_obj_id)
> > > > +static int idstack_push(struct bpf_idmap *idmap, u32 id)
> > > > +{
> > > > + int i;
> > > > +
> > > > + if (!id)
> > > > + return 0;
> > > > +
> > > > + for (i = 0; i < idmap->cnt; i++)
> > > > + if (idmap->map[i].old == id)
> > > > + return 0;
> > > > +
> > > > + if (WARN_ON_ONCE(idmap->cnt >= BPF_ID_MAP_SIZE))
> > > > + return -EFAULT;
> > >
> > > It feels like this check belongs above, maybe the first thing to do.
> >
> > Shouldn't it allow the verification to continue when pushing an id
> > that already exists in the stack?
>
> yep
>
> >
> > >
> > > > +
> > > > + idmap->map[idmap->cnt++].old = id;
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int idstack_pop(struct bpf_idmap *idmap)
> > > > {
> > > > + if (!idmap->cnt)
> > > > + return 0;
> > > > +
> > > > + return idmap->map[--idmap->cnt].old;
> > > > +}
> > > > +
> > > > +/* Release id and objects referencing the id iteratively in a DFS manner */
> > > > +static int release_reference(struct bpf_verifier_env *env, int id)
> > > > +{
> > > > + u32 mask = (1 << STACK_SPILL) | (1 << STACK_DYNPTR);
> > > > struct bpf_verifier_state *vstate = env->cur_state;
> > > > + struct bpf_idmap *idstack = &env->idmap_scratch;
> > > > + struct bpf_stack_state *stack;
> > > > struct bpf_func_state *state;
> > > > struct bpf_reg_state *reg;
> > > > - int err;
> > > > + int root_id = id, err;
> > > >
> > > > - err = release_reference_nomark(vstate, ref_obj_id);
> > > > - if (err)
> > > > - return err;
> > > > + idstack->cnt = 0;
> > > > + idstack_push(idstack, id);
> > > >
> > > > - bpf_for_each_reg_in_vstate(vstate, state, reg, ({
> > > > - if (reg->ref_obj_id == ref_obj_id)
> > > > - mark_reg_invalid(env, reg);
> > > > - }));
> > > > + if (find_reference_state(vstate, id))
> > > > + WARN_ON_ONCE(release_reference_nomark(vstate, id));
> > > > +
> > > > + while ((id = idstack_pop(idstack))) {
> > > > + bpf_for_each_reg_in_vstate_mask(vstate, state, reg, stack, mask, ({
> > > > + if (reg->id != id && reg->parent_id != id && reg->ref_obj_id != id)
> > > > + continue;
> > > > +
> > > > + if (reg->ref_obj_id && id != root_id) {
> > >
> > > Does this line check that the only ref_obj_id we should see is either
> > > 0 or root_id? can we rewrite it as
> > > if (reg->ref_obj_id && reg->ref_obj_id != root_id)
> > >
> > > this is simpler, because id can also be reg->id/reg->parent_id, which is
> > > hard to reason what that means.
> > >
> >
> > Looking at it again, I think my release_reference() is wrong. It first
> > calls release_reference_nomark() if the id is a ref_obj_id. This will
> > invalidate all registers with the same ref_obj_id without traversing
> > their children.
> >
>
> hm, will it? from what I see, release_reference_nomark() doesn't do
> anything with registers/stack slots, just removes the reference from
> state->refs
You are right... I was conflating it with something else.
I think Mykyta's suggestion seems to be more straightforward so I will
change it in the next version.
>
>
> > I should remember the root_ref_obj_id instead of calling
> > release_reference_nomark(). Then, the check you mentioned should be:
> > if (reg->ref_obj_id && reg->ref_obj_id != root_ref_obj_id)
> >
> > I will write a selftest to confirm the issue and include it in the
> > next respin if this turns out to be a bug.
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH iwl-next] ice: fix FDB deletion
From: Rinitha, SX @ 2026-04-24 6:50 UTC (permalink / raw)
To: Loktionov, Aleksandr, intel-wired-lan@lists.osuosl.org,
Nguyen, Anthony L, Loktionov, Aleksandr
Cc: netdev@vger.kernel.org, Staniszewski, Jakub
In-Reply-To: <20260320050533.422475-1-aleksandr.loktionov@intel.com>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Aleksandr Loktionov
> Sent: 20 March 2026 10:36
> To: intel-wired-lan@lists.osuosl.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Cc: netdev@vger.kernel.org; Staniszewski, Jakub <jakub.staniszewski@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-next] ice: fix FDB deletion
>
> From: Jakub Staniszewski <jakub.staniszewski@intel.com>
>
> Correct the logic in ndo_fdb_del() to align with other drivers in upstream. The condition was inverted — it was rejecting permanent
> (NUD_PERMANENT) MAC addresses while allowing non-permanent ones to be deleted, which is the opposite of the intended behavior.
>
> The correct logic is to reject deletion of non-permanent entries, mirroring the fix applied to ndo_dflt_fdb_del() in commit 645359930231
> ("rtnetlink: Fix inverted check in ndo_dflt_fdb_del()").
>
> Fixes: e94d4478669357cd ("ice: Implement filter sync, NDO operations and bump version")
> Signed-off-by: Jakub Staniszewski <jakub.staniszewski@intel.com>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Tested-by: Rinitha S <sx.rinitha@intel.com> (A Contingent worker at Intel)
^ permalink raw reply
* Re: Re: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty
From: Bezdeka, Florian @ 2026-04-24 6:56 UTC (permalink / raw)
To: willemdebruijn.kernel@gmail.com, 25181214217@stu.xidian.edu.cn
Cc: syzbot+e5d6936b9f4545fd88ab@syzkaller.appspotmail.com,
davem@davemloft.net, dsahern@kernel.org, sd@queasysnail.net,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
horms@kernel.org, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com
In-Reply-To: <5eabb1a1.7642.19dbd869da3.Coremail.25181214217@stu.xidian.edu.cn>
On Fri, 2026-04-24 at 11:26 +0800, 王明煜 wrote:
> Hi Sabrina and Jakub,
>
> Before sending out the v3 patch, I synced my tree to the latest mainline and checked the current state of `ip6_make_skb()`.
>
> It turns out that the missing `ip6_cork_release(cork)` in the error path was already naturally resolved by Eric Dumazet's recent refactoring commit:
> b409a7f7176b ("ipv6: colocate inet6_cork in inet_cork_full")
>
> With Eric's changes, the error handling now correctly calls `ip6_cork_release(cork)` if `ip6_setup_cork()` fails, meaning the memory leak is no longer present in the latest tree.
Fine, so mainline is correct now. What about the stable trees? Did you
check them already?
Florian
>
> Please disregard my v1 and v2 patches. I am also telling syzbot to close this report based on Eric's commit.
>
> Thank you all again for your time, the deep code review, and for guiding me to find the true root cause. I learned a huge amount from this discussion!
>
> #syz fix: ipv6: colocate inet6_cork in inet_cork_full
>
> Best regards,
> Mingyu Wang
>
> 2026-04-24 11:16:30 "王明煜" <25181214217@stu.xidian.edu.cn> 写道：
> > Hi,
> >
> > Thank you so much for the review and for pointing me to the correct Fixes tag!
> >
> > You hit the nail on the head regarding `__ip6_append_data()`. After re-evaluating the code path based on your question, I realize my assumption in v2 was incorrect. `__ip6_append_data()` does indeed guarantee that an skb is queued upon success, making the `skb == NULL` path dead code in this context.
> >
> > I traced the `failslab` memory leak back to its true origin: the lockless fast path wrapper `ip6_make_skb()`.
> >
> > Sabrina previously noted that `ip6_setup_cork()` failures correctly release the dst. That is absolutely true for the slow path, where `udp_v6_flush_pending_frames()` eventually handles the cleanup.
> >
> > However, in the fast path, `ip6_make_skb()` calls `ip6_setup_cork()`. Inside `ip6_setup_cork()`, `cork->base.dst` is assigned early. If a subsequent memory allocation fails (e.g., `v6_cork->opt = kzalloc(...)` failing due to failslab), it returns an error. `ip6_make_skb()` then directly returns `ERR_PTR(err)` WITHOUT calling `ip6_cork_release(cork)`.
> >
> > Since `udpv6_sendmsg()` assumes the `dst` reference is stolen by `ip6_make_skb()` and unconditionally jumps to `out_no_dst`, the `dst` is completely leaked.
> >
> > The fix is simply to add `ip6_cork_release(cork)` in the `ip6_setup_cork()` error path inside `ip6_make_skb()`.
> >
> > I will submit a v3 patch shortly addressing this true root cause and using your suggested Fixes tag. Thank you again for steering me in the exact right direction!
> >
> > Best regards,
> > Mingyu Wang
> >
> >
> > > -----原始邮件-----
> > > 发件人: "Willem de Bruijn" <willemdebruijn.kernel@gmail.com>
> > > 发送时间:2026-04-23 22:59:45 (星期四)
> > > 收件人: "Mingyu Wang" <25181214217@stu.xidian.edu.cn>, willemdebruijn.kernel@gmail.com, davem@davemloft.net, dsahern@kernel.org, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com
> > > 抄送: sd@queasysnail.net, horms@kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, "Mingyu Wang" <25181214217@stu.xidian.edu.cn>, syzbot+e5d6936b9f4545fd88ab@syzkaller.appspotmail.com
> > > 主题: Re: [PATCH v2] ipv6: fix memory leak in __ip6_make_skb() when queue is empty
> > >
> > > Mingyu Wang wrote:
> > > > During fuzzing with failslab enabled, a memory leak was observed in the
> > > > IPv6 UDP send path.
> > > >
> > > > The root cause resides in __ip6_make_skb(). In extremely rare cases
> > > > (such as fault injection or specific empty payload conditions),
> > >
> > > Can you elaborate on this? Which fault injection lets
> > > __ip6_append_data succeed without writing data?
> > >
> > > > __ip6_append_data() may succeed but leave the socket's write queue
> > > > empty.
> > > >
> > > > When __ip6_make_skb() is subsequently called, __skb_dequeue(queue)
> > > > returns NULL. The previous logic handled this by executing a 'goto out;',
> > > > which completely bypassed the call to ip6_cork_release(cork).
> > > >
> > > > Since the 'cork' structure actively holds a reference to the routing
> > > > entry (dst_entry) and potentially other allocated options, skipping
> > > > the release cleanly leaks these resources.
> > > >
> > > > Fix this by introducing an 'out_cork_release' label and jumping to it
> > > > when skb is NULL, ensuring the cork state is always properly cleaned up.
> > > > The now-unused 'out' label is also removed to prevent compiler warnings.
> > > >
> > > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > >
> > > I think this is
> > >
> > > Fixes: 6422398c2ab0 ("ipv6: introduce ipv6_make_skb")
> > >
> > > > Reported-by: syzbot+e5d6936b9f4545fd88ab@syzkaller.appspotmail.com
> > > > Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
^ 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