Re: [PATCH net v2 2/2] net: ncsi: validate MAC and VLAN counts in Get Parameters response

[Simon Horman <horms@kernel.org>]

On Sat, Sep 12, 2026 at 07:17:45PM +0100, Aamir Ahmed wrote:
> ncsi_rsp_handler_gp() walks the MAC address and VLAN filter tables using
> counts taken straight from the response, without checking them against
> the data the packet carries or against the filter arrays the earlier Get
> Capabilities response sized.
> 
> A malformed response can therefore read past the packet, write past
> mac_filter.addrs[] when mac_cnt exceeds n_uc + n_mc + n_mixed, write
> past vlan_filter.vids[] when vlan_cnt exceeds n_vids, and set bits past
> the u64 bitmaps that track the enabled entries - which for both filters
> overwrites the array pointer stored just after the bitmap, so the same
> loop iteration then writes through it.
> 
> The response is not guaranteed to be linear, so make the fixed part
> available with pskb_may_pull() before the counts are read, then the
> tables they claim, and take the pointer again afterwards because
> pskb_may_pull() may have moved the data. Reject the response if either
> count exceeds the array it indexes, the bitmap it sets bits in, or if
> Get Capabilities has not run and the arrays are absent.
> 
> Fixes: 062b3e1b6d4f ("net/ncsi: Refactor MAC, VLAN filters")
> Assisted-by: LLM
> Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
> ---
> v2:
>   - use pskb_may_pull() instead of testing skb->len, in two stages: the
>     fixed part before the counts are read, then the tables they claim,
>     taking the pointer again afterwards (Simon)
>   - also bound both counts by the width of the bitmaps they index; the
>     v1 check covered only the arrays, so set_bit()/clear_bit() past the
>     u64 could still overwrite the array pointer stored after it
>   - correct the Fixes: tag; v1 quoted a hash that does not resolve, and
>     the filter arrays came in with the refactor
>   - use offsetof() for the start of the tables rather than the literal 48
>   - read the counts into locals so the loops do not re-read device data
>     across the pull
>   - add the Assisted-by: LLM tag (Simon, Greg)
>   - name the target tree in the subject
> v1: https://lore.kernel.org/netdev/AS8P251MB0001E99A1865DFEC3C3A16E7C8B32@AS8P251MB0001.EURP251.PROD.OUTLOOK.COM/
> 
> Compile-tested only; I have no NC-SI hardware. The OEM and GMCMA
> handlers read past the header at their own device-supplied offsets and
> counts and are not covered by either patch; those are separate changes.

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