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

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

[Aamir Ahmed]

The Get Parameters (GP) response handler iterates over MAC address
and VLAN filter tables using counts (mac_cnt, vlan_cnt) taken directly
from the response packet.  These counts are not validated against either
the actual packet length or the filter arrays allocated by the earlier
Get Capabilities response handler.

A malformed GP response can cause:
 - out-of-bounds read past the packet data when iterating pdata
 - out-of-bounds write to mac_filter.addrs[] when mac_cnt exceeds
   the allocated filter size (n_uc + n_mc + n_mixed)
 - out-of-bounds write to vlan_filter.vids[] when vlan_cnt exceeds
   n_vids
 - bitmap corruption via set_bit()/clear_bit() beyond u64 width

Add three validation checks before the filter table loops:
 1. Packet is large enough for the claimed MAC and VLAN entries
 2. mac_cnt does not exceed the allocated MAC filter capacity
 3. vlan_cnt does not exceed the allocated VLAN filter capacity

Also check for NULL filter arrays in case Get Capabilities was not
processed first.

Fixes: 0b49507fc090 ("net/ncsi: Resource management")
Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
---
 net/ncsi/ncsi-rsp.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 1401528dbd6c..88f3b8db0289 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -884,10 +884,38 @@ static int ncsi_rsp_handler_gp(struct ncsi_request *nr)
 	nc->modes[NCSI_MODE_AEN].enable = 1;
 	nc->modes[NCSI_MODE_AEN].data[0] = ntohl(rsp->aen_mode);
 
+	/* Validate the response carries enough data for the MAC and VLAN
+	 * tables it claims to contain.  The variable-length area begins
+	 * at offset 48 (right after the fixed Get Parameters fields).
+	 */
+	if (nr->rsp->len < 48 + (unsigned int)rsp->mac_cnt * ETH_ALEN +
+			    (unsigned int)rsp->vlan_cnt * sizeof(__be16)) {
+		netdev_warn(ndp->ndev.dev,
+			    "NCSI: GP response too short for %u MACs + %u VLANs\n",
+			    rsp->mac_cnt, rsp->vlan_cnt);
+		return -EINVAL;
+	}
+
+	ncmf = &nc->mac_filter;
+	ncvf = &nc->vlan_filter;
+
+	if (!ncmf->addrs ||
+	    rsp->mac_cnt > ncmf->n_uc + ncmf->n_mc + ncmf->n_mixed) {
+		netdev_warn(ndp->ndev.dev,
+			    "NCSI: GP MAC count %u exceeds filter capacity\n",
+			    rsp->mac_cnt);
+		return -EINVAL;
+	}
+	if (!ncvf->vids || rsp->vlan_cnt > ncvf->n_vids) {
+		netdev_warn(ndp->ndev.dev,
+			    "NCSI: GP VLAN count %u exceeds filter capacity\n",
+			    rsp->vlan_cnt);
+		return -EINVAL;
+	}
+
 	/* MAC addresses filter table */
 	pdata = (unsigned char *)rsp + 48;
 	enable = rsp->mac_enable;
-	ncmf = &nc->mac_filter;
 	spin_lock_irqsave(&nc->lock, flags);
 	bitmap = &ncmf->bitmap;
 	for (i = 0; i < rsp->mac_cnt; i++, pdata += 6) {
@@ -902,7 +930,6 @@ static int ncsi_rsp_handler_gp(struct ncsi_request *nr)
 
 	/* VLAN filter table */
 	enable = ntohs(rsp->vlan_enable);
-	ncvf = &nc->vlan_filter;
 	bitmap = &ncvf->bitmap;
 	spin_lock_irqsave(&nc->lock, flags);
 	for (i = 0; i < rsp->vlan_cnt; i++, pdata += 2) {
-- 
2.43.0

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

[Simon Horman]

On Mon, Sep 07, 2026 at 12:37:52AM +0100, Aamir Ahmed wrote:
> The Get Parameters (GP) response handler iterates over MAC address
> and VLAN filter tables using counts (mac_cnt, vlan_cnt) taken directly
> from the response packet.  These counts are not validated against either
> the actual packet length or the filter arrays allocated by the earlier
> Get Capabilities response handler.
> 
> A malformed GP response can cause:
>  - out-of-bounds read past the packet data when iterating pdata
>  - out-of-bounds write to mac_filter.addrs[] when mac_cnt exceeds
>    the allocated filter size (n_uc + n_mc + n_mixed)
>  - out-of-bounds write to vlan_filter.vids[] when vlan_cnt exceeds
>    n_vids
>  - bitmap corruption via set_bit()/clear_bit() beyond u64 width
> 
> Add three validation checks before the filter table loops:
>  1. Packet is large enough for the claimed MAC and VLAN entries
>  2. mac_cnt does not exceed the allocated MAC filter capacity
>  3. vlan_cnt does not exceed the allocated VLAN filter capacity
> 
> Also check for NULL filter arrays in case Get Capabilities was not
> processed first.
> 
> Fixes: 0b49507fc090 ("net/ncsi: Resource management")
> Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
> ---
>  net/ncsi/ncsi-rsp.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index 1401528dbd6c..88f3b8db0289 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -884,10 +884,38 @@ static int ncsi_rsp_handler_gp(struct ncsi_request *nr)
>  	nc->modes[NCSI_MODE_AEN].enable = 1;
>  	nc->modes[NCSI_MODE_AEN].data[0] = ntohl(rsp->aen_mode);
>  
> +	/* Validate the response carries enough data for the MAC and VLAN
> +	 * tables it claims to contain.  The variable-length area begins
> +	 * at offset 48 (right after the fixed Get Parameters fields).
> +	 */
> +	if (nr->rsp->len < 48 + (unsigned int)rsp->mac_cnt * ETH_ALEN +
> +			    (unsigned int)rsp->vlan_cnt * sizeof(__be16)) {
> +		netdev_warn(ndp->ndev.dev,
> +			    "NCSI: GP response too short for %u MACs + %u VLANs\n",
> +			    rsp->mac_cnt, rsp->vlan_cnt);
> +		return -EINVAL;
> +	}

My concern with this patch is the same as for patch 1/2.

I'm not sure that there is any guarantee that nr->rsp is a linear skb
and if not then checking len is insufficient and I think the check
should be implemented using pskb_may_pull.

Not that any pointers referring to skb data may be invalidated
by that call, so, f.e. rsp should probably be set after the
call to pskb_may_pull().

...


As noted collectively by Greg and Paolo elsewhere:
1. Please include an Assisted-by tag
2. Please keep the number of patches in-flight to 15 or less

-- 
pw-bot: changes-requested