xsk: fix bugs around xsk skb allocation
Subsystems (4)
v5 cover xsk: fix bugs around xsk skb allocation
From: Jason Xing <[email protected]>
There are rare issues around xsk_build_skb(). Some of them were founded by Sashiko[1][2].
[1]: https://lore.kernel.org/all/[email protected]/
[2]: https://lore.kernel.org/all/[email protected]/
---
v5
1. reword patch 3
2. adjust the order of patch 6 that is now moved up to be patch 2)
3. use helper in patch (Stan)
v4
1. fix 32-bit arch issue in patch 8 (Alexander && Maciej)
2. add acked-by tags (Stan)
v3
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
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: free the skb when hitting the upper bound MAX_SKB_FRAGS
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: fix xsk_addrs slab leak on multi-buffer error path
xsk: fix u64 descriptor address truncation on 32-bit architectures
net/xdp/xsk.c | 115 ++++++++++++++++++++++++++-------------- net/xdp/xsk_buff_pool.c | 3 ++ 2 files changed, 77 insertions(+), 41 deletions(-)
Signature · 1 line
2.41.3
PATCH v5 1/8 xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices
From: Jason Xing <[email protected]>
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.
--- net/xdp/xsk_buff_pool.c | 3 +++ 1 file changed, 3 insertions(+)
net/xdp/xsk_buff_pool.c +3 -0
@@ -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 ;
Signature · 1 line
2.41.3
Re: [PATCH net v5 1/8] xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Unconditional rejection of `XDP_UMEM_TX_SW_CSUM` breaks valid native zero-copy configurations.…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditional rejection of XDP_UMEM_TX_SW_CSUM breaks valid native zero-copy configurations.
- [High] The root cause of the 'wedged TX ring' remains unaddressed, allowing trivial userspace triggers. --
commit a1862dee263d88bc3b6ca0898f858a88e2e37fce
Author: Jason Xing <[email protected]>
xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices
This commit attempts to prevent silent per-packet failures and wedged TX rings when using sw-csum with drivers advertising IFF_TX_SKB_NO_LINEAR. It does so by rejecting the bind operation with -EOPNOTSUPP during setup.
Quoted 9 lines
Without the patch, every descriptor carrying '
XDP_TX_METADATA|XDP_TXMD_FLAGS_CHECKSUM' produces:
1) aWARN_ONCE"offset (N) >=skb_headlen()(0)" fromskb_checksum_help(),
2)sendmsg()returning-EINVALwithout consuming the descriptor (invalid_descsis not incremented),
3) a wedged TX ring:__xsk_generic_xmit()does not advance the consumer on non-EOVERFLOW errors, so the nextsendmsg()re-reads the same descriptor and re-hits the same WARN until the socket is closed.
Does this completely resolve the wedged TX ring issue? While this patch prevents skb_checksum_help() from returning -EINVAL, xsk_skb_metadata() can still return -EINVAL for other reasons, such as an out-of-bounds csum_start or csum_offset provided by userspace.
If xsk_build_skb() returns -EINVAL, wouldn't __xsk_generic_xmit() still evaluate if (err != -EOVERFLOW) goto out; and break its loop without consuming the descriptor? Might the generic transmit path need to be updated to consume the descriptor and drop the packet on terminal errors like -EINVAL to fully prevent userspace from wedging the ring?
net/xdp/xsk_buff_pool.c int xp_assign_dev(struct xsk_buff_pool *pool, · line 180
Quoted 11 lines
net/xdp/xsk_buff_pool.c +3
-0@@ -175,6 +175,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 ; +
Will this check unconditionally reject valid native zero-copy configurations?
In native zero-copy mode (when force_zc is set), the transmission path bypasses __xsk_generic_xmit() entirely and processes descriptors natively, meaning it does not use xsk_build_skb_zerocopy() or invoke skb_checksum_help().
If a UMEM legitimately has the XDP_UMEM_TX_SW_CSUM flag set (for example, when shared across multiple devices), it appears this will now return -EOPNOTSUPP. Should this check be restricted to only apply when using the generic datapath?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Re: [PATCH net v5 1/8] xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices
Quoted 35 lines · On Sun, May 3, 2026 at 11:09 PM <[email protected]> wrote:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditional rejection ofXDP_UMEM_TX_SW_CSUMbreaks valid native zero-copy configurations.
- [High] The root cause of the 'wedged TX ring' remains unaddressed, allowing trivial userspace triggers. --commit
a1862dee263d88bc3b6ca0898f858a88e2e37fce
Author: Jason Xing <[email protected]>xsk: reject sw-csum UMEM binding to
IFF_TX_SKB_NO_LINEARdevicesThis commit attempts to prevent silent per-packet failures and wedged TX rings when using sw-csum with drivers advertising
IFF_TX_SKB_NO_LINEAR. It does so by rejecting the bind operation with-EOPNOTSUPPduring setup.Fixes 30c3055f9c0d ("xsk: wrap generic metadata handling onto separate function")Signed-off-by Jason Xing <[email protected]>Quoted 9 lines
Without the patch, every descriptor carrying '
XDP_TX_METADATA|XDP_TXMD_FLAGS_CHECKSUM' produces:
1) aWARN_ONCE"offset (N) >=skb_headlen()(0)" fromskb_checksum_help(),
2)sendmsg()returning-EINVALwithout consuming the descriptor (invalid_descsis not incremented),
3) a wedged TX ring:__xsk_generic_xmit()does not advance the consumer on non-EOVERFLOW errors, so the nextsendmsg()re-reads the same descriptor and re-hits the same WARN until the socket is closed.Does this completely resolve the wedged TX ring issue? While this patch prevents
skb_checksum_help()from returning-EINVAL,xsk_skb_metadata()can still return-EINVALfor other reasons, such as an out-of-boundscsum_startorcsum_offsetprovided by userspace.
Quoted 5 lines
If
xsk_build_skb()returns-EINVAL, wouldn't__xsk_generic_xmit()still evaluate if (err !=-EOVERFLOW) goto out; and break its loop without consuming the descriptor? Might the generic transmit path need to be updated to consume the descriptor and drop the packet on terminal errors like-EINVALto fully prevent userspace from wedging the ring?
I believe we've 'discussed' this before. This is a fresh new and pre-existing bug (happending) unrelated to this one. A few days ago, thanks to your reminder, it was put on my to-do list.
Quoted 20 lines
net/xdp/xsk_buff_pool.c int xp_assign_dev(struct xsk_buff_pool *pool, · line 180
Quoted 12 lines
net/xdp/xsk_buff_pool.c +3
-0@@ -175,6 +175,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 ; +Will this check unconditionally reject valid native zero-copy configurations?
In native zero-copy mode (when
force_zcis set), the transmission path bypasses__xsk_generic_xmit()entirely and processes descriptors natively, meaning it does not usexsk_build_skb_zerocopy()or invokeskb_checksum_help().
Quoted 4 lines
If a UMEM legitimately has the
XDP_UMEM_TX_SW_CSUMflag set (for example, when shared across multiple devices), it appears this will now return-EOPNOTSUPP. Should this check be restricted to only apply when using the generic datapath?
The same topic we 'discussed' before. No one is using the software checksum feature on zerocopy mode. People will be warned because of wrong usage and then adjust their own codes correspondingly.
Thanks,
Jason
Quoted 3 lines
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
PATCH v5 2/8 xsk: free the skb when hitting the upper bound MAX_SKB_FRAGS
In reply to Jason XingFrom: Jason Xing <[email protected]>
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.
--- net/xdp/xsk.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
net/xdp/xsk.c +4 -1
@@ -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);
Signature · 1 line
2.41.3
[email protected]
AI reviewRe: [PATCH net v5 2/8] xsk: free the skb when hitting the upper bound MAX_SKB_FRAGS
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Critical] This problem wasn't introduced by this patch, but returning -EOVERFLOW when xs->skb is NULL…
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] This problem wasn't introduced by this patch, but returning -EOVERFLOW when xs->skb is NULL leads to an unconditional NULL pointer dereference in the caller's error path.
- [High] Calling kfree_skb(skb) directly on a partially built SKB triggers the custom XSK destructor, which incorrectly submits the failed descriptor to the AF_XDP completion queue.
- [High] This problem wasn't introduced by this patch, but an identical SKB memory leak exists when xsk_skb_metadata() fails.
--
commit cf402952a5ccf9981b4749df15227be947716b0d
Author: Jason Xing <[email protected]>
xsk: free the skb when hitting the upper bound MAX_SKB_FRAGS
This patch explicitly adds kfree_skb() before returning -EOVERFLOW to its caller when hitting the MAX_SKB_FRAGS limit on a newly allocated SKB. This prevents the SKB from being permanently leaked.
net/xdp/xsk.c static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs, · line 861
Quoted 12 lines
net/xdp/xsk.c +3
-1@@ -856,8 +856,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);
Does calling kfree_skb() directly here incorrectly trigger the custom XSK destructor for a dropped packet?
Earlier in xsk_build_skb_zerocopy(), the newly allocated SKB is initialized via xsk_skb_init_misc(), which explicitly sets skb->destructor = xsk_destruct_skb.
When xsk_destruct_skb() runs, it calls xsk_cq_submit_addr_locked(), which erroneously generates a spurious TX completion for userspace and bypasses the queue's cached_prod reservation state, corrupting the completion queue logic.
Should the destructor be cleared first to safely discard the partially built SKB?
Note that this regression was fixed later in the patch series by commit 88b75ff1d12b ("xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()").
This problem wasn't introduced by this patch, but can returning -EOVERFLOW when xs->skb is NULL cause a NULL pointer dereference in the caller's error path?
In the caller xsk_build_skb(), the local skb pointer is set to NULL, and execution jumps to the free_err label. Because err == -EOVERFLOW, the error handler unconditionally calls:
xsk_inc_num_desc(xs->skb); xsk_drop_skb(xs->skb);
Since xs->skb is strictly NULL, these functions will immediately dereference the NULL pointer via skb_shinfo(NULL) inside xsk_skb_destructor_is_addr(), causing a panic.
Note that this regression was fixed later in the patch series by commit 819a57cd3753 ("xsk: handle NULL dereference of the skb without frags issue").
This problem wasn't introduced by this patch either, but is there an identical SKB memory leak earlier in this function when xsk_skb_metadata() fails?
If desc->options & XDP_TX_METADATA is set and xsk_skb_metadata() fails, the function returns ERR_PTR(err) directly without freeing the newly allocated skb:
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))
return ERR_PTR(err);
}The caller xsk_build_skb() explicitly overrides its local skb pointer to NULL before jumping to free_err, causing this SKB to be permanently leaked.
An unprivileged user could trigger this leak by providing malformed metadata, leading to memory exhaustion.
Note that this regression was fixed later in the patch series by commit f1079e939977 ("xsk: avoid skb leak in XDP_TX_METADATA case").
Quoted 2 lines
net/xdp/xsk.c +1
-0return ERR_PTR (-EOVERFLOW ); + }
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Re: [PATCH net v5 2/8] xsk: free the skb when hitting the upper bound MAX_SKB_FRAGS
Quoted 44 lines · On Sun, May 3, 2026 at 11:09 PM <[email protected]> wrote:
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] This problem wasn't introduced by this patch, but returning-EOVERFLOWwhenxs->skbis NULL leads to an unconditional NULL pointer dereference in the caller's error path.
- [High] Callingkfree_skb(skb)directly on a partially built SKB triggers the custom XSK destructor, which incorrectly submits the failed descriptor to theAF_XDPcompletion queue.
- [High] This problem wasn't introduced by this patch, but an identical SKB memory leak exists whenxsk_skb_metadata()fails.
--commit
cf402952a5ccf9981b4749df15227be947716b0d
Author: Jason Xing <[email protected]>xsk: free the skb when hitting the upper bound
MAX_SKB_FRAGSThis patch explicitly adds
kfree_skb()before returning-EOVERFLOWto its caller when hitting theMAX_SKB_FRAGSlimit on a newly allocated SKB. This prevents the SKB from being permanently leaked.net/xdp/xsk.c static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs, · line 861
Quoted 12 lines
net/xdp/xsk.c +3
-1@@ -856,8 +856,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);Does calling
kfree_skb()directly here incorrectly trigger the custom XSK destructor for a dropped packet?Earlier in
xsk_build_skb_zerocopy(), the newly allocated SKB is initialized viaxsk_skb_init_misc(), which explicitly setsskb->destructor=xsk_destruct_skb.When
xsk_destruct_skb()runs, it callsxsk_cq_submit_addr_locked(), which erroneously generates a spurious TX completion for userspace and bypasses the queue'scached_prodreservation state, corrupting the completion queue logic.Should the destructor be cleared first to safely discard the partially built SKB?
Quoted 7 lines
Note that this regression was fixed later in the patch series by commit
88b75ff1d12b("xsk: prevent CQ desync when freeing half-built skbs inxsk_build_skb()").This problem wasn't introduced by this patch, but can returning
-EOVERFLOWwhenxs->skbis NULL cause a NULL pointer dereference in the caller's error path?
It's a very carefully designed order of this series. Without this patch the skb is unconditionally leaked (permanent memory loss), which is strictly worse than a single spurious CQ completion that userspace can handle. Patch 5/8 then eliminates the destructor issue by postponing xsk_skb_init_misc() past all error paths.
This is why I move this patch ahead in this version.
Quoted 12 lines
In the caller
xsk_build_skb(), the local skb pointer is set to NULL, and execution jumps to thefree_errlabel. Because err ==-EOVERFLOW, the error handler unconditionally calls:xsk_inc_num_desc(xs->skb); xsk_drop_skb(xs->skb);Since
xs->skbis strictly NULL, these functions will immediately dereference the NULL pointer viaskb_shinfo(NULL)insidexsk_skb_destructor_is_addr(), causing a panic.
Quoted 7 lines
Note that this regression was fixed later in the patch series by commit
819a57cd3753("xsk: handle NULL dereference of the skb without frags issue").This problem wasn't introduced by this patch either, but is there an identical SKB memory leak earlier in this function when
xsk_skb_metadata()fails?
As you noticed, this patch doesn't introduce new bugs at all and the following patches take care of it.
Quoted 12 lines
If
desc->options&XDP_TX_METADATAis set andxsk_skb_metadata()fails, the function returnsERR_PTR(err)directly without freeing the newly allocated skb: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)) return ERR_PTR(err); }
Quoted 8 lines
The caller
xsk_build_skb()explicitly overrides its local skb pointer to NULL before jumping tofree_err, causing this SKB to be permanently leaked.An unprivileged user could trigger this leak by providing malformed metadata, leading to memory exhaustion.
Note that this regression was fixed later in the patch series by commit
f1079e939977("xsk: avoid skb leak inXDP_TX_METADATAcase").
Right, it has been fixed...
No action needs to be taken so far.
Thanks,
Jason
Quoted 6 lines
net/xdp/xsk.c static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs, · line 783
Quoted 3 lines
net/xdp/xsk.c +1
-0return ERR_PTR (-EOVERFLOW ); + }--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
PATCH v5 3/8 xsk: handle NULL dereference of the skb without frags issue
In reply to Jason XingFrom: Jason Xing <[email protected]>
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.
Fix this by guarding 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 one reserved CQ slot and increments invalid_descs by one to account for the single invalid descriptor.
--- net/xdp/xsk.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
net/xdp/xsk.c +8 -3
@@ -896,9 +896,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 */
Signature · 1 line
2.41.3
[email protected]
AI reviewRe: [PATCH net v5 3/8] xsk: handle NULL dereference of the skb without frags issue
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Completion Queue (CQ) state corruption when handling -EOVERFLOW for the first descriptor. --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Completion Queue (CQ) state corruption when handling -EOVERFLOW for the first descriptor. --
commit 819a57cd3753db9f503ac093f83dccc180da574d
Author: Jason Xing <[email protected]>
xsk: handle NULL dereference of the skb without frags issue
This commit fixes a NULL pointer dereference when a first descriptor triggers -EOVERFLOW in xsk_build_skb_zerocopy() by adding a check for xs->skb. It handles the first-descriptor case by manually canceling the CQ slot and incrementing invalid_descs.
Quoted 6 lines
net/xdp/xsk.c +0
-0@@ -976,9 +976,14 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,kfree_skb (skb);
net/xdp/xsk.c static struct sk_buff *xsk_build_skb(struct xdp_sock *xs, · line 984
Quoted 10 lines
net/xdp/xsk.c +6
-3if (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 );
Does this cause a Completion Queue (CQ) state corruption?
When a first descriptor triggers -EOVERFLOW in xsk_build_skb_zerocopy(), the free_err path first calls kfree_skb(skb) just above this code. Because xsk_skb_init_misc() was called earlier, the SKB's destructor is set to xsk_destruct_skb(). The kfree_skb() call invokes this destructor, which submits a completion and increments the ring's producer index.
If we then call xsk_cq_cancel_locked() here, it decrements the reservation index while the producer index was already incremented, breaking the core invariant cached_prod >= ring->producer.
This causes space calculations to underflow and can overwrite unconsumed entries in the CQ.
I see this was addressed later in the series by commit 88b75ff1d12b ("xsk:
prevent CQ desync when freeing half-built skbs in xsk_build_skb()").
Quoted 5 lines
net/xdp/xsk.c +2
-0+ xs->tx->invalid_descs++; + }xskq_cons_release (xs->tx); }else {/* Let application retry */
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
Re: [PATCH net v5 3/8] xsk: handle NULL dereference of the skb without frags issue
Quoted 44 lines · On Sun, May 3, 2026 at 11:09 PM <[email protected]> wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Completion Queue (CQ) state corruption when handling-EOVERFLOWfor the first descriptor. --commit
819a57cd3753db9f503ac093f83dccc180da574d
Author: Jason Xing <[email protected]>xsk: handle NULL dereference of the skb without frags issue
This commit fixes a NULL pointer dereference when a first descriptor triggers
-EOVERFLOWinxsk_build_skb_zerocopy()by adding a check forxs->skb. It handles the first-descriptor case by manually canceling the CQ slot and incrementinginvalid_descs.Quoted 6 lines
net/xdp/xsk.c +0
-0@@ -976,9 +976,14 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,kfree_skb (skb);net/xdp/xsk.c static struct sk_buff *xsk_build_skb(struct xdp_sock *xs, · line 984
Quoted 10 lines
net/xdp/xsk.c +6
-3if (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 );Does this cause a Completion Queue (CQ) state corruption?
When a first descriptor triggers
-EOVERFLOWinxsk_build_skb_zerocopy(), thefree_errpath first callskfree_skb(skb)just above this code. Becausexsk_skb_init_misc()was called earlier, the SKB's destructor is set toxsk_destruct_skb(). Thekfree_skb()call invokes this destructor, which submits a completion and increments the ring's producer index.If we then call
xsk_cq_cancel_locked()here, it decrements the reservation index while the producer index was already incremented, breaking the core invariantcached_prod>=ring->producer.
Quoted 5 lines
This causes space calculations to underflow and can overwrite unconsumed entries in the CQ.
I see this was addressed later in the series by commit
88b75ff1d12b("xsk:
prevent CQ desync when freeing half-built skbs inxsk_build_skb()").
Yep, it has been resolved in the series.
Thanks,
Jason
Quoted 9 lines
net/xdp/xsk.c static struct sk_buff *xsk_build_skb(struct xdp_sock *xs, · line 909
Quoted 6 lines
net/xdp/xsk.c +2
-0+ xs->tx->invalid_descs++; + }xskq_cons_release (xs->tx); }else {/* Let application retry */ --
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
PATCH v5 4/8 xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path
In reply to Jason XingFrom: Jason Xing <[email protected]>
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.
--- net/xdp/xsk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/xdp/xsk.c +1 -1
@@ -892,7 +892,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 ) {
Signature · 1 line
2.41.3
[email protected]
AI reviewRe: [PATCH net v5 4/8] xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Memory leak of SKB in xsk_build_skb_zerocopy() when TX metadata processing fails. - [High] Memory…
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Memory leak of SKB in xsk_build_skb_zerocopy() when TX metadata processing fails.
- [High] Memory leak of xsk_addr array when an SKB fragment allocation fails and the packet is subsequently dropped.
- [High] Double completion and Completion Queue (CQ) state desynchronization when xsk_skb_metadata() fails. --
commit 7c8b265d19cf8eaa2671c3f79683f55810dfc68a
Author: Jason Xing <[email protected]>
xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path
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.
Fix by using a !xs->skb check to handle first frag situation, ensuring we only free skbs that were freshly allocated in this call.
net/xdp/xsk.c static struct sk_buff *xsk_build_skb(struct xdp_sock *xs, · line 975
Quoted 10 lines
net/xdp/xsk.c +1
-1@@ -972,7 +972,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)
Does this code leak the SKB if xsk_build_skb_zerocopy() allocations fail during TX metadata processing?
If xsk_build_skb_zerocopy() allocates a new SKB using sock_alloc_send_skb() but metadata parsing fails (e.g., xsk_skb_metadata() returns an error), it returns ERR_PTR(err). The caller xsk_build_skb() explicitly overwrites its local pointer with skb = NULL before jumping to free_err:
skb = xsk_build_skb_zerocopy(xs, desc);
if (IS_ERR(skb)) {
err = PTR_ERR(skb);
skb = NULL;
goto free_err;
}Since skb is NULL, this check is bypassed, and the newly allocated SKB and its associated socket write memory quota are leaked.
I understand this is fixed later in the patch series by commit "xsk: avoid skb leak in XDP_TX_METADATA case".
Does this code leak the xsk_addr array when an SKB fragment allocation fails and the packet is subsequently dropped?
When appending a fragment to an in-progress packet (xs->skb != NULL), the code identifies an inline address tag and replaces it by allocating an xsk_addr array via kmem_cache_zalloc(), initializing xsk_addr->num_descs to 1.
If alloc_page() subsequently fails, the code returns -EAGAIN to retry later, leaving xs->skb intact.
If the packet is dropped before a successful retry (e.g., socket close triggering xsk_release() and xsk_drop_skb()), xsk_consume_skb() attempts to clean up the array. However, xsk_consume_skb() uses the check if (unlikely(num_descs > 1)) to determine if it should call kmem_cache_free().
Since num_descs is exactly 1, the check evaluates to false, and the xsk_addrs allocation is leaked.
I see this is fixed later in the patch series by commit "xsk: fix xsk_addrs slab leak on multi-buffer error path".
net/xdp/xsk.c static struct sk_buff *xsk_build_skb(struct xdp_sock *xs, · line 896
Quoted 1 line
net/xdp/xsk.c +0
-0kfree_skb (skb);
Can this kfree_skb() call lead to a double completion and Completion Queue (CQ) state desynchronization if xsk_skb_metadata() fails?
In xsk_build_skb() copy mode, xsk_skb_init_misc() sets skb->destructor = xsk_destruct_skb. Immediately after, if xsk_skb_metadata() fails, execution jumps to free_err and triggers this kfree_skb(skb).
This executes the SKB destructor, which unconditionally submits the failed descriptor to the Completion Queue by advancing the user-visible ring->producer pointer.
Immediately following the free, the free_err handler calls xsk_cq_cancel_locked(), which decrements the internally cached cached_prod reservation counter. This desynchronizes the internal reservation counter from the user-visible ring pointer and falsely pushes a completion event to user space for a descriptor that failed transmission.
I note this is fixed later in the patch series by commit "xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()".
Quoted 2 lines
net/xdp/xsk.c +0
-0if (err == -EOVERFLOW ) {
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
Re: [PATCH net v5 4/8] xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path
Quoted 90 lines · On Sun, May 3, 2026 at 11:09 PM <[email protected]> wrote:
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Memory leak of SKB inxsk_build_skb_zerocopy()when TX metadata processing fails.
- [High] Memory leak ofxsk_addrarray when an SKB fragment allocation fails and the packet is subsequently dropped.
- [High] Double completion and Completion Queue (CQ) state desynchronization whenxsk_skb_metadata()fails. --commit
7c8b265d19cf8eaa2671c3f79683f55810dfc68a
Author: Jason Xing <[email protected]>xsk: fix use-after-free of
xs->skbinxsk_build_skb()free_errpathWhen
xsk_build_skb()processes multi-buffer packets in copy mode, the first descriptor stores data into the skb linear area without adding any frags, sonr_fragsstays at 0. The caller then setsxs->skb= skb to accumulate subsequent descriptors.Fix by using a !xs->skb check to handle first frag situation, ensuring we only free skbs that were freshly allocated in this call.
net/xdp/xsk.c static struct sk_buff *xsk_build_skb(struct xdp_sock *xs, · line 975
Quoted 10 lines
net/xdp/xsk.c +1
-1@@ -972,7 +972,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)Does this code leak the SKB if
xsk_build_skb_zerocopy()allocations fail during TX metadata processing?If
xsk_build_skb_zerocopy()allocates a new SKB usingsock_alloc_send_skb()but metadata parsing fails (e.g.,xsk_skb_metadata()returns an error), it returnsERR_PTR(err). The callerxsk_build_skb()explicitly overwrites its local pointer with skb = NULL before jumping tofree_err:skb = xsk_build_skb_zerocopy(xs, desc); if (IS_ERR(skb)) { err = PTR_ERR(skb); skb = NULL; goto free_err; }Since skb is NULL, this check is bypassed, and the newly allocated SKB and its associated socket write memory quota are leaked.
I understand this is fixed later in the patch series by commit "xsk: avoid skb leak in
XDP_TX_METADATAcase".Does this code leak the
xsk_addrarray when an SKB fragment allocation fails and the packet is subsequently dropped?When appending a fragment to an in-progress packet (
xs->skb!= NULL), the code identifies an inline address tag and replaces it by allocating anxsk_addrarray viakmem_cache_zalloc(), initializingxsk_addr->num_descsto 1.If
alloc_page()subsequently fails, the code returns-EAGAINto retry later, leavingxs->skbintact.If the packet is dropped before a successful retry (e.g., socket close triggering
xsk_release()andxsk_drop_skb()),xsk_consume_skb()attempts to clean up the array. However,xsk_consume_skb()uses the check if (unlikely(num_descs > 1)) to determine if it should callkmem_cache_free().Since
num_descsis exactly 1, the check evaluates to false, and thexsk_addrsallocation is leaked.I see this is fixed later in the patch series by commit "xsk: fix
xsk_addrsslab leak on multi-buffer error path".net/xdp/xsk.c static struct sk_buff *xsk_build_skb(struct xdp_sock *xs, · line 896
Quoted 1 line
net/xdp/xsk.c +0
-0kfree_skb (skb);Can this
kfree_skb()call lead to a double completion and Completion Queue (CQ) state desynchronization ifxsk_skb_metadata()fails?In
xsk_build_skb()copy mode,xsk_skb_init_misc()setsskb->destructor=xsk_destruct_skb. Immediately after, ifxsk_skb_metadata()fails, execution jumps tofree_errand triggers thiskfree_skb(skb).This executes the SKB destructor, which unconditionally submits the failed descriptor to the Completion Queue by advancing the user-visible
ring->producerpointer.
Quoted 8 lines
Immediately following the free, the
free_errhandler callsxsk_cq_cancel_locked(), which decrements the internally cachedcached_prodreservation counter. This desynchronizes the internal reservation counter from the user-visible ring pointer and falsely pushes a completion event to user space for a descriptor that failed transmission.I note this is fixed later in the patch series by commit "xsk: prevent CQ desync when freeing half-built skbs in
xsk_build_skb()".
Those three points you brought up are all handled in the series. Again, the order of this series is designed deliberately to avoid new/severe problems.
Thanks,
Jason
Quoted 6 lines
net/xdp/xsk.c static struct sk_buff *xsk_build_skb(struct xdp_sock *xs, · line 898
Quoted 3 lines
net/xdp/xsk.c +0
-0if (err == -EOVERFLOW ) {--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
PATCH v5 5/8 xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()
In reply to Jason XingFrom: Jason Xing <[email protected]>
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().
--- net/xdp/xsk.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
net/xdp/xsk.c +2 -3
@@ -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))
@@ -837,7 +835,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);
@@ -887,6 +884,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;
Signature · 1 line
2.41.3
[email protected]
AI reviewRe: [PATCH net v5 5/8] xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] In `xsk_build_skb_zerocopy()`, if `xsk_skb_metadata()` fails, the newly allocated `skb` is leaked.…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] In xsk_build_skb_zerocopy(), if xsk_skb_metadata() fails, the newly allocated skb is leaked. --
commit 88b75ff1d12b482a55e1724cc457909cf3ea19dd
Author: Jason Xing <[email protected]>
xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()
Postpone the skb initialization phase in xsk_build_skb() until the first frag allocation is completed to prevent the skb destructor from running on error paths. Before the initialization, the skb can be safely freed by kfree_skb().
net/xdp/xsk.c static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs, · line 824
Quoted 13 lines
net/xdp/xsk.c +0
-2@@ -819,8 +819,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))
Could this leak the newly allocated skb if xsk_skb_metadata() fails?
If xsk_skb_metadata() returns an error, xsk_build_skb_zerocopy() will return ERR_PTR(err) directly without freeing the skb.
The caller receives the error pointer, sets its local skb variable to NULL, and jumps to the free_err label:
net/xdp/xsk.c:xsk_build_skb() {
...
if (dev->priv_flags & IFF_TX_SKB_NO_LINEAR) {
skb = xsk_build_skb_zerocopy(xs, desc);
if (IS_ERR(skb)) {
err = PTR_ERR(skb);
skb = NULL;
goto free_err;
}
...
}Because the skb is explicitly nullified here, the kfree_skb(skb) call in the cleanup path is bypassed, leaving the socket buffer and its associated sk_wmem_alloc charge leaked.
While this pre-existing issue is explicitly fixed later in this patch series in commit "xsk: avoid skb leak in XDP_TX_METADATA case", should it be addressed earlier to ensure intermediate states remain safe during a bisect?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
Re: [PATCH net v5 5/8] xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()
Quoted 51 lines · On Sun, May 3, 2026 at 11:09 PM <[email protected]> wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Inxsk_build_skb_zerocopy(), ifxsk_skb_metadata()fails, the newly allocatedskbis leaked. --commit
88b75ff1d12b482a55e1724cc457909cf3ea19dd
Author: Jason Xing <[email protected]>xsk: prevent CQ desync when freeing half-built skbs in
xsk_build_skb()Postpone the skb initialization phase in
xsk_build_skb()until the first frag allocation is completed to prevent the skb destructor from running on error paths. Before the initialization, the skb can be safely freed bykfree_skb().net/xdp/xsk.c static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs, · line 824
Quoted 13 lines
net/xdp/xsk.c +0
-2@@ -819,8 +819,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))Could this leak the newly allocated skb if
xsk_skb_metadata()fails?If
xsk_skb_metadata()returns an error,xsk_build_skb_zerocopy()will returnERR_PTR(err)directly without freeing the skb.The caller receives the error pointer, sets its local skb variable to NULL, and jumps to the
free_errlabel:net/xdp/xsk.c:xsk_build_skb() { ... if (dev->priv_flags & IFF_TX_SKB_NO_LINEAR) { skb = xsk_build_skb_zerocopy(xs, desc); if (IS_ERR(skb)) { err = PTR_ERR(skb); skb = NULL; goto free_err; } ... }
Quoted 7 lines
Because the skb is explicitly nullified here, the
kfree_skb(skb)call in the cleanup path is bypassed, leaving the socket buffer and its associatedsk_wmem_alloccharge leaked.While this pre-existing issue is explicitly fixed later in this patch series in commit "xsk: avoid skb leak in
XDP_TX_METADATAcase", should it be addressed earlier to ensure intermediate states remain safe during a bisect?
No, again, I've mentioned this previously. Not the same issue here. No more severe issue is caused because of this. And it is fixed in the series.
Thanks,
Jason
Quoted 3 lines
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
PATCH v5 6/8 xsk: avoid skb leak in XDP_TX_METADATA case
In reply to Jason XingFrom: Jason Xing <[email protected]>
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.
--- net/xdp/xsk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
net/xdp/xsk.c +3 -1
@@ -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;
Signature · 1 line
2.41.3
[email protected]
AI reviewRe: [PATCH net v5 6/8] xsk: avoid skb leak in XDP_TX_METADATA case
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Critical] Time-of-Check to Time-of-Use (TOCTOU) vulnerability on shared UMEM metadata allows bypassing…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Time-of-Check to Time-of-Use (TOCTOU) vulnerability on shared UMEM metadata allows bypassing checksum bounds validation.
- [High] Leaked xsk_addrs structure in the non-zerocopy xsk_build_skb() error path when page allocation fails. --
commit f1079e9399771f267ff5b6767db05016e5ee4840
Author: Jason Xing <[email protected]>
xsk: avoid skb leak in XDP_TX_METADATA case
This commit fixes an skb leak in the XDP_TX_METADATA case by explicitly adding kfree_skb() before returning back to its caller. The issue happens when xsk_skb_metadata() returns an error, leading the caller to clear the skb without freeing it.
net/xdp/xsk.c static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs, · line 823
Quoted 8 lines
net/xdp/xsk.c +0
-0@@ -821,8 +821,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);
This wasn't introduced by this patch, but is it possible for a shared UMEM metadata TOCTOU race to bypass the checksum bounds validation?
In xsk_skb_metadata(), meta points to the metadata area within the umem buffer (buffer - pool->tx_metadata_len), which is memory-mapped and concurrently writable by userspace. The function performs a bounds check:
if (unlikely(meta->request.csum_start +
meta->request.csum_offset +
sizeof(__sum16) > desc->len))
return -EINVAL;If this passes, it re-reads the values directly from the shared memory:
skb->csum_start = hr + meta->request.csum_start;
skb->csum_offset = meta->request.csum_offset;Could a malicious user application race to overwrite these values with arbitrarily large 16-bit integers between the validation check and the assignment, leading to out-of-bounds DMA accesses? Might we need to copy the metadata to a local variable (e.g., using READ_ONCE()) before validation?
net/xdp/xsk.c static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs, · line 750
Quoted 8 lines
net/xdp/xsk.c +3
-1-if (unlikely (err)) +if (unlikely (err)) { +kfree_skb (skb);return ERR_PTR (err); + } } }else {struct xsk_addrs *xsk_addr;
This wasn't introduced by this patch, but does the non-zerocopy xsk_build_skb() leak the xsk_addrs structure on its error path?
When appending a fragment to an existing SKB (!skb is false) in xsk_build_skb(), the code converts the destructor_arg from a direct address to an allocated xsk_addrs structure and initializes xsk_addr->num_descs = 1.
If the subsequent alloc_page() fails, it jumps to free_err and returns -EAGAIN, leaving the partially built SKB in xs->skb without incrementing num_descs.
When the socket is closed or the SKB is subsequently dropped via xsk_drop_skb(), xsk_consume_skb() is called. Because destructor_arg is now a valid pointer, it fetches num_descs as 1. The cleanup logic relies on:
if (unlikely(num_descs > 1))
to free the xsk_addrs structure. Since num_descs is exactly 1, does it skip the free, permanently leaking the xsk_tx_generic_cache object?
I saw this issue is addressed later in the patch series (xsk: fix xsk_addrs slab leak on multi-buffer error path).
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
Re: [PATCH net v5 6/8] xsk: avoid skb leak in XDP_TX_METADATA case
Quoted 58 lines · On Sun, May 3, 2026 at 11:09 PM <[email protected]> wrote:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Time-of-Check to Time-of-Use (TOCTOU) vulnerability on shared UMEM metadata allows bypassing checksum bounds validation.
- [High] Leakedxsk_addrsstructure in the non-zerocopyxsk_build_skb()error path when page allocation fails. --commit
f1079e9399771f267ff5b6767db05016e5ee4840
Author: Jason Xing <[email protected]>xsk: avoid skb leak in
XDP_TX_METADATAcaseThis commit fixes an skb leak in the
XDP_TX_METADATAcase by explicitly addingkfree_skb()before returning back to its caller. The issue happens whenxsk_skb_metadata()returns an error, leading the caller to clear the skb without freeing it.net/xdp/xsk.c static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs, · line 823
Quoted 8 lines
net/xdp/xsk.c +0
-0@@ -821,8 +821,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);This wasn't introduced by this patch, but is it possible for a shared UMEM metadata TOCTOU race to bypass the checksum bounds validation?
In
xsk_skb_metadata(), meta points to the metadata area within the umem buffer (buffer -pool->tx_metadata_len), which is memory-mapped and concurrently writable by userspace. The function performs a bounds check:if (unlikely(meta->request.csum_start + meta->request.csum_offset + sizeof(__sum16) > desc->len)) return -EINVAL;If this passes, it re-reads the values directly from the shared memory:
skb->csum_start = hr + meta->request.csum_start; skb->csum_offset = meta->request.csum_offset;Could a malicious user application race to overwrite these values with arbitrarily large 16-bit integers between the validation check and the assignment, leading to out-of-bounds DMA accesses? Might we need to copy the metadata to a local variable (e.g., using
READ_ONCE()) before validation?net/xdp/xsk.c static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs, · line 750
Quoted 8 lines
net/xdp/xsk.c +3
-1-if (unlikely (err)) +if (unlikely (err)) { +kfree_skb (skb);return ERR_PTR (err); + } } }else {struct xsk_addrs *xsk_addr;
Quoted 2 lines
This wasn't introduced by this patch, but does the non-zerocopy
xsk_build_skb()leak thexsk_addrsstructure on its error path?
This issue will be fixed by a standalone patch then :)
Quoted 13 lines
When appending a fragment to an existing SKB (!skb is false) in
xsk_build_skb(), the code converts thedestructor_argfrom a direct address to an allocatedxsk_addrsstructure and initializesxsk_addr->num_descs= 1.If the subsequent
alloc_page()fails, it jumps tofree_errand returns-EAGAIN, leaving the partially built SKB inxs->skbwithout incrementingnum_descs.When the socket is closed or the SKB is subsequently dropped via
xsk_drop_skb(),xsk_consume_skb()is called. Becausedestructor_argis now a valid pointer, it fetchesnum_descsas 1. The cleanup logic relies on:
Quoted 7 lines
if (
unlikely(num_descs > 1))to free the
xsk_addrsstructure. Sincenum_descsis exactly 1, does it skip the free, permanently leaking thexsk_tx_generic_cacheobject?I saw this issue is addressed later in the patch series (xsk: fix
xsk_addrsslab leak on multi-buffer error path).
Yep, it is handled by the patch you said.
Thanks,
Jason
Quoted 3 lines
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
PATCH v5 7/8 xsk: fix xsk_addrs slab leak on multi-buffer error path
In reply to Jason XingFrom: Jason Xing <[email protected]>
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.
--- net/xdp/xsk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
net/xdp/xsk.c +2 -2
@@ -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); }
Signature · 1 line
2.41.3
PATCH v5 8/8 xsk: fix u64 descriptor address truncation on 32-bit architectures
In reply to Jason XingFrom: Jason Xing <[email protected]>
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. The existing tagged-pointer logic in xsk_skb_destructor_is_addr() stays unchanged: slab pointers returned from kmem_cache_zalloc() are always word-aligned and therefore have bit 0 clear, which correctly identifies them as a struct pointer rather than an inline tagged address on every architecture.
Factor the shared kmem_cache_zalloc + destructor_arg assignment into __xsk_addrs_alloc() and add a wrapper xsk_addrs_alloc() that handles the inline-to-list upgrade (is_addr check + get_addr + num_descs = 1). The three former open-coded kmem_cache_zalloc call sites now reduce to a single call each.
Propagate the -ENOMEM from xsk_skb_destructor_set_addr() through xsk_skb_init_misc() so the caller can clean up the skb via kfree_skb() before skb->destructor is installed.
The overhead is one extra kmem_cache_zalloc per first descriptor on 32-bit only; 64-bit builds are completely unchanged.
--- net/xdp/xsk.c | 88 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 56 insertions(+), 32 deletions(-)
net/xdp/xsk.c +56 -32
@@ -566,9 +566,42 @@ 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 struct xsk_addrs *__xsk_addrs_alloc (struct sk_buff *skb,u64 addr) { -skb_shinfo (skb)->destructor_arg = (void *)((uintptr_t )addr |0x1UL ); +struct xsk_addrs *xsk_addr; + + xsk_addr =kmem_cache_zalloc (xsk_tx_generic_cache,GFP_KERNEL ); +if (unlikely (!xsk_addr)) +return NULL ; + + xsk_addr->addrs[0 ] = addr; +skb_shinfo (skb)->destructor_arg = (void *)xsk_addr; +return xsk_addr; +} + +static struct xsk_addrs *xsk_addrs_alloc (struct sk_buff *skb) +{ +struct xsk_addrs *xsk_addr; + +if (!xsk_skb_destructor_is_addr (skb)) +return (struct xsk_addrs *)skb_shinfo (skb)->destructor_arg; + + xsk_addr =__xsk_addrs_alloc (skb,xsk_skb_destructor_get_addr (skb)); +if (likely (xsk_addr)) + xsk_addr->num_descs =1 ; +return xsk_addr; +} + +static int xsk_skb_destructor_set_addr (struct sk_buff *skb,u64 addr) +{ +if (IS_ENABLED (CONFIG_64BIT )) { +skb_shinfo (skb)->destructor_arg = (void *)((uintptr_t )addr |0x1UL ); +return 0 ; + } + +if (unlikely (!__xsk_addrs_alloc (skb, addr))) +return -ENOMEM ; +return 0 ; }static void xsk_inc_num_desc (struct sk_buff *skb)
@@ -644,14 +677,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 (unlikely (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)
@@ -749,18 +788,9 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs, }else {struct xsk_addrs *xsk_addr; -if (xsk_skb_destructor_is_addr (skb)) { - xsk_addr =kmem_cache_zalloc (xsk_tx_generic_cache, -GFP_KERNEL ); -if (!xsk_addr) -return ERR_PTR (-ENOMEM ); - - xsk_addr->num_descs =1 ; - xsk_addr->addrs[0 ] =xsk_skb_destructor_get_addr (skb); -skb_shinfo (skb)->destructor_arg = (void *)xsk_addr; - }else { - xsk_addr = (struct xsk_addrs *)skb_shinfo (skb)->destructor_arg; - } + xsk_addr =xsk_addrs_alloc (skb); +if (!xsk_addr) +return ERR_PTR (-ENOMEM );/* in case of -EOVERFLOW that could happen below, * xsk_consume_skb() will release this node as whole skb
@@ -849,19 +879,10 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,struct page *page;u8 *vaddr; -if (xsk_skb_destructor_is_addr (skb)) { - xsk_addr =kmem_cache_zalloc (xsk_tx_generic_cache, -GFP_KERNEL ); -if (!xsk_addr) { - err = -ENOMEM ; - goto free_err; - } - - xsk_addr->num_descs =1 ; - xsk_addr->addrs[0 ] =xsk_skb_destructor_get_addr (skb); -skb_shinfo (skb)->destructor_arg = (void *)xsk_addr; - }else { - xsk_addr = (struct xsk_addrs *)skb_shinfo (skb)->destructor_arg; + xsk_addr =xsk_addrs_alloc (skb); +if (!xsk_addr) { + err = -ENOMEM ; + goto free_err; }if (unlikely (nr_frags == (MAX_SKB_FRAGS -1 ) &&xp_mb_desc (desc))) {
@@ -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;
Signature · 1 line
2.41.3
[email protected]
AI reviewRe: [PATCH net v5 8/8] xsk: fix u64 descriptor address truncation on 32-bit architectures
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] In the copy-mode TX path (`xsk_build_skb`), if a multi-buffer fragment exceeds the `MAX_SKB_FRAGS`…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] In the copy-mode TX path (xsk_build_skb), if a multi-buffer fragment exceeds the MAX_SKB_FRAGS limit, the buffer's address is leaked to userspace because 0 is erroneously submitted to the completion queue. --
commit 4a776aefc60aa4d3123e7e63fd8f927f3748d008
Author: Jason Xing <[email protected]>
xsk: fix u64 descriptor address truncation on 32-bit architectures
This commit fixes a 32-bit truncation issue when storing 64-bit descriptor addresses in the SKB destructor argument by allocating an xsk_addrs struct. It consolidates the allocation logic into xsk_addrs_alloc() and uses it in both zerocopy and copy modes.
Quoted 4 lines
net/xdp/xsk.c +0
-0
[ ... ]
Quoted 14 lines
net/xdp/xsk.c +0
-10@@ -929,19 +959,10 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,struct page *page;u8 *vaddr; -if (xsk_skb_destructor_is_addr (skb)) { - xsk_addr =kmem_cache_zalloc (xsk_tx_generic_cache, -GFP_KERNEL ); -if (!xsk_addr) { - err = -ENOMEM ; - goto free_err; - } - - xsk_addr->num_descs =1 ; - xsk_addr->addrs[0 ] =xsk_skb_destructor_get_addr (skb);
net/xdp/xsk.c static struct sk_buff *xsk_build_skb(struct xdp_sock *xs, · line 968
Quoted 10 lines
net/xdp/xsk.c +4
-3-skb_shinfo (skb)->destructor_arg = (void *)xsk_addr; - }else { - xsk_addr = (struct xsk_addrs *)skb_shinfo (skb)->destructor_arg; + xsk_addr =xsk_addrs_alloc (skb); +if (!xsk_addr) { + err = -ENOMEM ; + goto free_err; }if (unlikely (nr_frags == (MAX_SKB_FRAGS -1 ) &&xp_mb_desc (desc))) {
This isn't a regression introduced by this patch, but while looking at the modified code, does this error handling path leak the buffer's address to userspace?
If this overflow check evaluates to true, the code sets err = -EOVERFLOW and jumps to the free_err label before assigning desc->addr to xsk_addr->addrs[xsk_addr->num_descs].
In the free_err block, it handles -EOVERFLOW by calling xsk_inc_num_desc(xs->skb) and xsk_drop_skb(xs->skb). This increments num_descs to account for the dropped descriptor and eventually calls xsk_destruct_skb() which calls xsk_cq_submit_addr_locked().
Since xsk_addr->addrs was never initialized with desc->addr for this descriptor, does this mean it submits 0 (from kmem_cache_zalloc) to the completion queue instead of the actual buffer address, causing userspace to permanently lose track of the buffer?
By comparison, xsk_build_skb_zerocopy() assigns desc->addr to the array before checking the fragment limit:
net/xdp/xsk.c:xsk_build_skb_zerocopy() {
...
xsk_addr = xsk_addrs_alloc(skb);
if (!xsk_addr)
return ERR_PTR(-ENOMEM);
/* in case of -EOVERFLOW that could happen below,
* xsk_consume_skb() will release this node as whole skb
* would be dropped, which implies freeing all list elements
*/
xsk_addr->addrs[xsk_addr->num_descs] = desc->addr;
}
len = desc->len;
...
}Should xsk_build_skb() also record the descriptor address before checking the -EOVERFLOW condition?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
Re: [PATCH net v5 8/8] xsk: fix u64 descriptor address truncation on 32-bit architectures
Quoted 87 lines · On Sun, May 3, 2026 at 11:09 PM <[email protected]> wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] In the copy-mode TX path (xsk_build_skb), if a multi-buffer fragment exceeds theMAX_SKB_FRAGSlimit, the buffer's address is leaked to userspace because0is erroneously submitted to the completion queue. --commit
4a776aefc60aa4d3123e7e63fd8f927f3748d008
Author: Jason Xing <[email protected]>xsk: fix u64 descriptor address truncation on 32-bit architectures
This commit fixes a 32-bit truncation issue when storing 64-bit descriptor addresses in the SKB destructor argument by allocating an
xsk_addrsstruct. It consolidates the allocation logic intoxsk_addrs_alloc()and uses it in both zerocopy and copy modes.Quoted 4 lines
net/xdp/xsk.c +0
-0[ ... ]
Quoted 14 lines
net/xdp/xsk.c +0
-10@@ -929,19 +959,10 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,struct page *page;u8 *vaddr; -if (xsk_skb_destructor_is_addr (skb)) { - xsk_addr =kmem_cache_zalloc (xsk_tx_generic_cache, -GFP_KERNEL ); -if (!xsk_addr) { - err = -ENOMEM ; - goto free_err; - } - - xsk_addr->num_descs =1 ; - xsk_addr->addrs[0 ] =xsk_skb_destructor_get_addr (skb);net/xdp/xsk.c static struct sk_buff *xsk_build_skb(struct xdp_sock *xs, · line 968
Quoted 10 lines
net/xdp/xsk.c +4
-3-skb_shinfo (skb)->destructor_arg = (void *)xsk_addr; - }else { - xsk_addr = (struct xsk_addrs *)skb_shinfo (skb)->destructor_arg; + xsk_addr =xsk_addrs_alloc (skb); +if (!xsk_addr) { + err = -ENOMEM ; + goto free_err; }if (unlikely (nr_frags == (MAX_SKB_FRAGS -1 ) &&xp_mb_desc (desc))) {This isn't a regression introduced by this patch, but while looking at the modified code, does this error handling path leak the buffer's address to userspace?
If this overflow check evaluates to true, the code sets err =
-EOVERFLOWand jumps to thefree_errlabel before assigningdesc->addrto xsk_addr->addrs[xsk_addr->num_descs].In the
free_errblock, it handles-EOVERFLOWby callingxsk_inc_num_desc(xs->skb)andxsk_drop_skb(xs->skb). This incrementsnum_descsto account for the dropped descriptor and eventually callsxsk_destruct_skb()which callsxsk_cq_submit_addr_locked().Since
xsk_addr->addrswas never initialized withdesc->addrfor this descriptor, does this mean it submits 0 (fromkmem_cache_zalloc) to the completion queue instead of the actual buffer address, causing userspace to permanently lose track of the buffer?By comparison,
xsk_build_skb_zerocopy()assignsdesc->addrto the array before checking the fragment limit:net/xdp/xsk.c:xsk_build_skb_zerocopy() { ... xsk_addr = xsk_addrs_alloc(skb); if (!xsk_addr) return ERR_PTR(-ENOMEM); /* in case of -EOVERFLOW that could happen below, * xsk_consume_skb() will release this node as whole skb * would be dropped, which implies freeing all list elements */ xsk_addr->addrs[xsk_addr->num_descs] = desc->addr; } len = desc->len; ... }
Quoted 2 lines
Should
xsk_build_skb()also record the descriptor address before checking the-EOVERFLOWcondition?
It has already been uncovered by you, maybe in v4 or v3 :) Sure, it will be fixed after this patch gets merged.
Thanks,
Jason
Quoted 3 lines
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
Re: [PATCH net v5 8/8] xsk: fix u64 descriptor address truncation on 32-bit architectures
In reply to Jason XingQuoted 34 lines · Jason Xing
From: Jason Xing <[email protected]>
In copy mode TX,
xsk_skb_destructor_set_addr()stores the 64-bit descriptor address into skb_shinfo(skb)->destructor_arg (void *) via auintptr_tcast:skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t)addr | 0x1UL);On 32-bit architectures
uintptr_tis 32 bits, so the upper 32 bits of the descriptor address are silently dropped. InXDP_ZEROCOPYunaligned 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 anxsk_addrsstruct (the same path already used for multi-descriptor SKBs) to store the full u64 address. The existing tagged-pointer logic inxsk_skb_destructor_is_addr()stays unchanged: slab pointers returned fromkmem_cache_zalloc()are always word-aligned and therefore have bit 0 clear, which correctly identifies them as a struct pointer rather than an inline tagged address on every architecture.Factor the shared
kmem_cache_zalloc+destructor_argassignment into__xsk_addrs_alloc()and add a wrapperxsk_addrs_alloc()that handles the inline-to-list upgrade (is_addrcheck +get_addr+num_descs= 1). The three former open-codedkmem_cache_zalloccall sites now reduce to a single call each.Propagate the
-ENOMEMfromxsk_skb_destructor_set_addr()throughxsk_skb_init_misc()so the caller can clean up the skb viakfree_skb()beforeskb->destructoris installed.
Quoted 6 lines
The overhead is one extra
kmem_cache_zallocper first descriptor on 32-bit only; 64-bit builds are completely unchanged.Fixes 0ebc27a4c67d ("xsk: avoid data corruption on cq descriptor number")Signed-off-by Jason Xing <[email protected]>
LGTM, thanks!
From: Jason Xing <[email protected]>
Date: Sat, 2 May 2026 23:07:14 +0300
Quoted 4 lines
From: Jason Xing <[email protected]>
There are rare issues around
xsk_build_skb(). Some of them were founded by Sashiko[1][2].
For the series:
Just don't forget to look at Sashiko's replies, I haven't looked deep into them, so can't say whether anything there makes sense.
Quoted 42 lines
[1]: https://lore.kernel.org/all/[email protected]/
[2]: https://lore.kernel.org/all/[email protected]/---
v51. reword patch 3
2. adjust the order of patch 6 that is now moved up to be patch 2)
3. use helper in patch (Stan)v4
1. fix 32-bit arch issue in patch 8 (Alexander && Maciej)
2. add acked-by tags (Stan)v3
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
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 toIFF_TX_SKB_NO_LINEARdevices
xsk: free the skb when hitting the upper boundMAX_SKB_FRAGS
xsk: handle NULL dereference of the skb without frags issue
xsk: fix use-after-free ofxs->skbinxsk_build_skb()free_errpath
xsk: prevent CQ desync when freeing half-built skbs inxsk_build_skb()
xsk: avoid skb leak inXDP_TX_METADATAcase
xsk: fixxsk_addrsslab leak on multi-buffer error path
xsk: fix u64 descriptor address truncation on 32-bit architectures
Quoted 3 lines
net/xdp/xsk.c | 115 ++++++++++++++++++++++++++-------------- net/xdp/xsk_buff_pool.c | 3 ++ 2 files changed, 77 insertions(+), 41 deletions(-)
Thanks,
Olek
Quoted 9 lines · Alexander Lobakin
From: Jason Xing <[email protected]>
Date: Sat, 2 May 2026 23:07:14 +0300Quoted 4 lines
From: Jason Xing <[email protected]>
There are rare issues around
xsk_build_skb(). Some of them were founded by Sashiko[1][2].
Quoted 6 lines
For the series:
Reviewed-by Alexander Lobakin <[email protected]>Just don't forget to look at Sashiko's replies, I haven't looked deep into them, so can't say whether anything there makes sense.
Thanks for the review. I'm currently away from the keyboard because of the long vacation. Now I'll handle them one by one in the following two days. Hopefully, I can manage it today :P
Thanks,
Jason
Quoted 49 lines
Quoted 43 lines
[1]: https://lore.kernel.org/all/[email protected]/
[2]: https://lore.kernel.org/all/[email protected]/---
v51. reword patch 3
2. adjust the order of patch 6 that is now moved up to be patch 2)
3. use helper in patch (Stan)v4
1. fix 32-bit arch issue in patch 8 (Alexander && Maciej)
2. add acked-by tags (Stan)v3
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
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 toIFF_TX_SKB_NO_LINEARdevices
xsk: free the skb when hitting the upper boundMAX_SKB_FRAGS
xsk: handle NULL dereference of the skb without frags issue
xsk: fix use-after-free ofxs->skbinxsk_build_skb()free_errpath
xsk: prevent CQ desync when freeing half-built skbs inxsk_build_skb()
xsk: avoid skb leak inXDP_TX_METADATAcase
xsk: fixxsk_addrsslab leak on multi-buffer error path
xsk: fix u64 descriptor address truncation on 32-bit architecturesQuoted 3 lines
net/xdp/xsk.c | 115 ++++++++++++++++++++++++++-------------- net/xdp/xsk_buff_pool.c | 3 ++ 2 files changed, 77 insertions(+), 41 deletions(-)Thanks,
Olek
This series was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
Quoted 9 lines · On Sat, 2 May 2026 23:07:14 +0300 you wrote:
From: Jason Xing <[email protected]>
There are rare issues around
xsk_build_skb(). Some of them were founded by Sashiko[1][2].[1]: https://lore.kernel.org/all/[email protected]/
[2]: https://lore.kernel.org/all/[email protected]/[...]
Here is the summary with links:
- [net,v5,1/8] xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices
https://git.kernel.org/netdev/net/c/d73a9a63f9f7 - [net,v5,2/8] xsk: free the skb when hitting the upper bound MAX_SKB_FRAGS https://git.kernel.org/netdev/net/c/0bb7a9caf5c1 - [net,v5,3/8] xsk: handle NULL dereference of the skb without frags issue https://git.kernel.org/netdev/net/c/8cd3c1c6e7d9 - [net,v5,4/8] xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path https://git.kernel.org/netdev/net/c/0f3776583d28 - [net,v5,5/8] xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb() https://git.kernel.org/netdev/net/c/3dec153ae484 - [net,v5,6/8] xsk: avoid skb leak in XDP_TX_METADATA case https://git.kernel.org/netdev/net/c/8c2cff50afdd - [net,v5,7/8] xsk: fix xsk_addrs slab leak on multi-buffer error path https://git.kernel.org/netdev/net/c/e0f229025a8e - [net,v5,8/8] xsk: fix u64 descriptor address truncation on 32-bit architectures https://git.kernel.org/netdev/net/c/203cee647f55
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html