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

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

[Aamir Ahmed]

ncsi_validate_rsp_pkt() computes a pointer to the checksum field based
on the payload length without first verifying that the skb actually
contains enough data.  For response types whose expected payload is
fixed (e.g., GCPS with 204 bytes), a short frame whose header falsely
claims the expected length passes the "ntohs(h->common.length) !=
payload" check, but the skb may be much smaller, leading to an
out-of-bounds read when dereferencing the checksum pointer.

For response types with variable-length payloads (GP, OEM, PLDM,
GMCMA), the payload value comes directly from ntohs(h->common.length),
so the header check is tautological and provides no protection at all.

Add an skb length check in ncsi_validate_rsp_pkt() to ensure the
packet has at least sizeof(ncsi_rsp_pkt_hdr) + payload bytes before
any field access.

Additionally, validate address_count bounds in the GMCMA response
handler.  The handler iterates over rsp->address_count entries from
the flexible array member without verifying that the packet is large
enough to contain them, leading to a heap out-of-bounds read if
address_count exceeds what the packet actually carries.

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

diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index fbd84bc8026a..1401528dbd6c 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -45,6 +45,13 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
 	 */
 	h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
 
+	/* Ensure the packet is large enough for header + payload */
+	if (nr->rsp->len < sizeof(*h) + payload) {
+		netdev_dbg(nr->ndp->ndev.dev,
+			   "NCSI: packet too short\n");
+		return -EINVAL;
+	}
+
 	if (h->common.revision != NCSI_PKT_REVISION) {
 		netdev_dbg(nr->ndp->ndev.dev,
 			   "NCSI: unsupported header revision\n");
@@ -1097,6 +1104,12 @@ static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr)
 	rsp = (struct ncsi_rsp_gmcma_pkt *)skb_network_header(nr->rsp);
 	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
 
+	if (nr->rsp->len < sizeof(*rsp) +
+			    rsp->address_count * ETH_ALEN) {
+		netdev_warn(ndev, "NCSI: GMCMA response too short\n");
+		return -EINVAL;
+	}
+
 	netdev_info(ndev, "NCSI: Received %d provisioned MAC addresses\n",
 		    rsp->address_count);
 	for (i = 0; i < rsp->address_count; i++) {
-- 
2.43.0

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

[Simon Horman]

On Mon, Sep 07, 2026 at 12:37:51AM +0100, Aamir Ahmed wrote:
> ncsi_validate_rsp_pkt() computes a pointer to the checksum field based
> on the payload length without first verifying that the skb actually
> contains enough data.  For response types whose expected payload is
> fixed (e.g., GCPS with 204 bytes), a short frame whose header falsely
> claims the expected length passes the "ntohs(h->common.length) !=
> payload" check, but the skb may be much smaller, leading to an
> out-of-bounds read when dereferencing the checksum pointer.
> 
> For response types with variable-length payloads (GP, OEM, PLDM,
> GMCMA), the payload value comes directly from ntohs(h->common.length),
> so the header check is tautological and provides no protection at all.
> 
> Add an skb length check in ncsi_validate_rsp_pkt() to ensure the
> packet has at least sizeof(ncsi_rsp_pkt_hdr) + payload bytes before
> any field access.
> 
> Additionally, validate address_count bounds in the GMCMA response
> handler.  The handler iterates over rsp->address_count entries from
> the flexible array member without verifying that the packet is large
> enough to contain them, leading to a heap out-of-bounds read if
> address_count exceeds what the packet actually carries.
> 
> Fixes: 0b49507fc090 ("net/ncsi: Resource management")
> Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
> ---
>  net/ncsi/ncsi-rsp.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> index fbd84bc8026a..1401528dbd6c 100644
> --- a/net/ncsi/ncsi-rsp.c
> +++ b/net/ncsi/ncsi-rsp.c
> @@ -45,6 +45,13 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
>  	 */
>  	h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
>  
> +	/* Ensure the packet is large enough for header + payload */
> +	if (nr->rsp->len < sizeof(*h) + payload) {

I am concerned that there is no guarantee that the skb is linear and thus
this check is not sufficient. If so, I think the check should be
implemented using pskb_may_pull().  And h should be set after it is called.

...

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