Re: [PATCH net v2 1/2] net: ncsi: validate response packet length before accessing fields

On Sat, Sep 12, 2026 at 07:09:37PM +0100, Aamir Ahmed wrote:

ncsi_validate_rsp_pkt() takes a pointer to the response header and then
reads the checksum at the end of the padded payload, without checking
that the skb holds either. ncsi_rcv_rsp() reads the common header the
same way before that.

For response types with a fixed payload the header length check is not
enough: a short frame whose header claims the expected length passes it.
For the variable-length types (GP, OEM, PLDM, GMCMA) the payload comes
from the header itself, so the check is tautological.

The response skb is not guaranteed to be linear, so use pskb_may_pull()
rather than testing skb->len, and take the header pointers afterwards -
pskb_may_pull() may move the data. The payload is padded to four bytes
and the checksum occupies the last four, so the validator pulls
ALIGN(payload, 4) rather than payload. ncsi_rcv_rsp() keeps a copy of
the packet type for its error paths, as its own header pointer does not
survive the validator.

Fixes: 138635cc27c9 ("net/ncsi: NCSI response packet handler")
Assisted-by: LLM
Signed-off-by: Aamir Ahmed <redacted>
---
v2:
  - use pskb_may_pull() instead of testing skb->len, and take the header
    pointer after the call (Simon)
  - pull ALIGN(payload, 4), not payload: the checksum sits in the last
    four bytes of the padded payload, so the v1 bound did not cover it
  - guard the common-header read in ncsi_rcv_rsp() too, and keep a copy
    of the packet type, since its header pointer does not survive the
    validator's pull
  - correct the Fixes: tag; v1 quoted a hash that does not resolve, and
    the blame for this file is the commit that added it
  - drop the GMCMA hunk; it belongs with its own handler
  - add the Assisted-by: LLM tag (Simon, Greg)
  - name the target tree in the subject
v1: https://lore.kernel.org/netdev/AS8P251MB0001E6ABBE0B6809D3E21B9DC8B32@AS8P251MB0001.EURP251.PROD.OUTLOOK.COM/ (local)

Reviewed-by: Simon Horman <horms@kernel.org>

For future reference:

This patch-set has two patches.  But they seem to have been sent as two
separate email threads.  Which confuses some tooling, such as Sashiko.
Using tools such as b4 or git send-email (in conjunction with git
format-patch) should avoid this problem.