Re: [PATCH v5 12/18] net/funeth: Switch to higher-level allocator API

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Untrusted packet length from hardware CQE leads to stack buffer overflow and OOB memory corruption.
--

commit fd0faaa7ff432d936af6fe6e5da6c09cbf2f16eb
Author: Brendan Jackman <jackmanb@xxxxxxxxxx>

net/funeth: Switch to higher-level allocator API

This commit replaces the deprecated __alloc_pages_node() with
alloc_pages_node(). The new API supports passing NUMA_NO_NODE. No functional
changes are intended by this switch.

> diff --git a/drivers/net/ethernet/fungible/funeth/funeth_rx.c b/drivers/net/ethernet/fungible/funeth/funeth_rx.c
> index 7e2584895de39..d7000017ac2bd 100644
> --- a/drivers/net/ethernet/fungible/funeth/funeth_rx.c
> +++ b/drivers/net/ethernet/fungible/funeth/funeth_rx.c
> @@ -103,7 +103,7 @@ static int funeth_alloc_page(struct funeth_rxq *q, struct funeth_rxbuf *rb,
>  	if (cache_get(q, rb))
>  		return 0;
>  
> -	p = __alloc_pages_node(node, gfp | __GFP_NOWARN, 0);
> +	p = alloc_pages_node(node, gfp | __GFP_NOWARN, 0);

[Severity: Critical]
This is a pre-existing issue, but while reviewing this file I noticed a
potential stack buffer overflow when handling untrusted packet lengths
from the hardware CQE.

In fun_handle_cqe_pkt(), the untrusted packet length and fragment count
are read from the CQE:

    unsigned int i, tot_len, pkt_len = be32_to_cpu(rxreq->pkt_len);
    ...
    tot_len = pkt_len;
    ...
    ref_ok = fun_gather_pkt(q, tot_len, frags);
    ...
    skb = napi_build_skb(va, ALIGN(tot_len, FUN_EPRQ_PKT_ALIGN));
    ...
    si->nr_frags = rxreq->nsgl;
    for (i = 0; i < si->nr_frags; i++)
        si->frags[i] = frags[i];

Does the loop in fun_gather_pkt() iterate until tot_len is exhausted without
validating the size?

    for (;;) {
        ...
        skb_frag_fill_page_desc(frags++, buf->page, q->buf_offset, frag_len);
        tot_len -= frag_len;
        if (!tot_len)
            break;
    }

If rxreq->pkt_len is maliciously large, could this increment the frags array
pointer past the bounds of the stack-allocated frags array (size 4),
resulting in a stack buffer overflow?

Furthermore, could napi_build_skb() use this untrusted length to place
skb_shared_info at an out-of-bounds offset, corrupting unrelated memory?

>  	if (unlikely(!p))
>  		return -ENOMEM;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@xxxxxxxxxx?part=12