Re: [PATCH net 1/6] net/ncsi: validate response packet lengths against the skb

Jakub Kicinski
2026-04-23T19:12:07Z

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net/ncsi: validate response packet lengths against the skb

This commit adds packet length validation for NC-SI responses. It pulls the
common header first, then pulls the full response body before validating the
packet to prevent reading past the received packet body.

> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index fbd84bc8026a3..1fe061ede26dd 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -38,11 +38,18 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
> struct ncsi_rsp_pkt_hdr *h;
> u32 checksum;
> __be32 *pchecksum;
> + unsigned int len;
>
> /* Check NCSI packet header. We don't need validate
> * the packet type, which should have been checked
> * before calling this function.
> */
> + len = skb_network_offset(nr->rsp) + sizeof(*h) + ALIGN(payload, 4);
> + if (!pskb_may_pull(nr->rsp, len)) {

Does this pskb_may_pull() call potentially invalidate the hdr pointer cached
earlier in ncsi_rcv_rsp()?

Looking at ncsi_rcv_rsp(), it caches the header pointer before validating:

ncsi_rcv_rsp() {
...
hdr = (struct ncsi_pkt_hdr *)skb_network_header(skb);
...
ret = ncsi_validate_rsp_pkt(nr, payload);

If the SKB is fragmented and pskb_may_pull() needs to pull data into the
linear area, it can reallocate the SKB's data buffer and free the old buffer.

When ncsi_validate_rsp_pkt() returns, the local hdr pointer in ncsi_rcv_rsp()
would still point to the freed memory.

This pointer is then dereferenced in multiple error paths:

if (ret) {
netdev_warn(ndp->ndev.dev,
"NCSI: 'bad' packet ignored for type 0x%x\n",
hdr->type);
...
ret = nrh->handler(nr);
if (ret)
netdev_err(ndp->ndev.dev,
"NCSI: Handler for packet type 0x%x returned %d\n",
hdr->type, ret);

Could this cause a use-after-free when reading hdr->type?
--
pw-bot: cr