Re: [PATCH net 2/6] xsk: pass TX metadata pointer by reference

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] `xsk_tx_metadata_to_compl()` returns early when `meta == NULL` without clearing the `compl` output structure, leading to arbitrary kernel memory writes during TX completion.
--

--- Patch [2]: [PATCH net 2/6] xsk: pass TX metadata pointer by reference ---
commit 83fcec7a0c0075ec991849b8d99f6d6cd083c335
Author: Stanislav Fomichev <sdf.kernel@gmail.com>

xsk: pass TX metadata pointer by reference

Completion handling needs to know whether a timestamp was requested when
the metadata was processed. Let xsk_tx_metadata_request() update the
caller's metadata pointer so that decision can be carried forward without
rereading user-controlled flags.

[ ... ]

> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index 8b51876efbed1..06c081feff42d 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -153,17 +153,19 @@ static inline void xsk_tx_metadata_to_compl(struct xsk_tx_metadata *meta,

[Severity: Critical]
This isn't a bug introduced by this patch, but I noticed that 
xsk_tx_metadata_to_compl() returns early without explicitly clearing the 
compl structure when meta is NULL:

static inline void xsk_tx_metadata_to_compl(struct xsk_tx_metadata *meta,
                                            struct xsk_tx_metadata_compl *compl)
{
        if (!meta)
                return;

In drivers like mlx5e_xsk_tx(), compl is declared on the stack and passed in 
uninitialized:

mlx5e_xsk_tx() {
        if (xp_tx_metadata_enabled(sq->xsk_pool)) {
                struct xsk_tx_metadata_compl compl;

                xsk_tx_metadata_to_compl(meta, &compl);
                ...
                mlx5e_xdpi_fifo_push(..., { .xsk_meta = compl });
        }
}

If an untrusted user sends a packet with invalid flags, causing meta to be 
NULL, could the uninitialized stack garbage in compl be pushed to the NAPI TX 
completion FIFO? Would this cause the completion path to dereference a garbage 
pointer and write a hardware timestamp to arbitrary kernel memory?

Similarly, in stmmac_xdp_xmit_zc():

stmmac_xdp_xmit_zc() {
        ...
        xsk_tx_metadata_to_compl(meta,
                                 &tx_q->tx_skbuff_dma[entry].xsk_meta);
}

Could this early return leave a stale pointer from a previous packet in the 
descriptor ring, leading to an accidental overwrite of userspace UMEM?

>  /**
>   *  xsk_tx_metadata_request - Evaluate AF_XDP TX metadata at submission
> - *  @meta: pointer to AF_XDP metadata area
> + *  @pmeta: pointer to pointer to AF_XDP metadata area

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727161959.885642-1-sdf@fomichev.me?part=2