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

[Simon Horman <horms@kernel.org>]

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 <elb12345@hotmail.co.uk>
> ---
> 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/

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.