* [PATCH net 1/6] net/ncsi: validate response packet lengths against the skb
2026-04-22 16:03 [PATCH net 0/6] net/ncsi: harden packet parsing against malformed BMC replies Michael Bommarito
@ 2026-04-22 16:03 ` Michael Bommarito
2026-04-22 16:03 ` [PATCH net 2/6] net/ncsi: bound filter table state to software limits Michael Bommarito
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Michael Bommarito @ 2026-04-22 16:03 UTC (permalink / raw)
To: Samuel Mendoza-Jonas, Paul Fertser, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, linux-kernel, Michael Bommarito, stable
ncsi_rcv_rsp() reads the common packet header before checking that the
skb contains enough data for it, and ncsi_validate_rsp_pkt() trusts
the response payload length before accessing the checksum field.
Malformed NC-SI replies can therefore drive header and checksum reads
past the received packet body. Make the dispatcher pull the common
header first, then have ncsi_validate_rsp_pkt() pull the full response
body before validating the packet.
This keeps malformed responses on the error path instead of letting the
parser walk past the skb payload.
Fixes: 138635cc27c9 ("net/ncsi: NCSI response packet handler")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
net/ncsi/ncsi-rsp.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index fbd84bc8026a..1fe061ede26d 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -38,11 +38,18 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
struct ncsi_rsp_pkt_hdr *h;
u32 checksum;
__be32 *pchecksum;
+ unsigned int len;
/* Check NCSI packet header. We don't need validate
* the packet type, which should have been checked
* before calling this function.
*/
+ len = skb_network_offset(nr->rsp) + sizeof(*h) + ALIGN(payload, 4);
+ if (!pskb_may_pull(nr->rsp, len)) {
+ netdev_dbg(nr->ndp->ndev.dev, "NCSI: packet too short\n");
+ return -EINVAL;
+ }
+
h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(nr->rsp);
if (h->common.revision != NCSI_PKT_REVISION) {
@@ -1182,6 +1189,11 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
}
/* Check if it is AEN packet */
+ if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*hdr))) {
+ ret = -EINVAL;
+ goto err_free_skb;
+ }
+
hdr = (struct ncsi_pkt_hdr *)skb_network_header(skb);
if (hdr->type == NCSI_PKT_AEN)
return ncsi_aen_handler(ndp, skb);
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net 2/6] net/ncsi: bound filter table state to software limits
2026-04-22 16:03 [PATCH net 0/6] net/ncsi: harden packet parsing against malformed BMC replies Michael Bommarito
2026-04-22 16:03 ` [PATCH net 1/6] net/ncsi: validate response packet lengths against the skb Michael Bommarito
@ 2026-04-22 16:03 ` Michael Bommarito
2026-04-22 16:03 ` [PATCH net 3/6] net/ncsi: validate GMCMA address counts against the payload Michael Bommarito
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Michael Bommarito @ 2026-04-22 16:03 UTC (permalink / raw)
To: Samuel Mendoza-Jonas, Paul Fertser, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, linux-kernel, Michael Bommarito, stable
The NCSI filter state uses single-word bitmaps for both MAC and VLAN
entries, but Get Capabilities and Get Parameters responses can still
feed larger counts into that state.
Cap the stored VLAN table size to the bitmap width before it reaches
the manage-side bitmap walkers, reject GP tables that exceed the sizes
advertised by GC, and stop indexing the MAC filter bitmap past its
software capacity. Also stop shifting past the width of the enable
bitfields when GP reports more entries than fit in those masks.
This keeps oversized or inconsistent filter counts from turning into
out-of-bounds bitmap accesses and oversized table walks in the response
and manage paths. A follow-up patch in this series separately validates
that the GP payload actually covers the consumed MAC/VLAN table bytes.
A live x86_64/KASAN QEMU repro can drive this after GC advertises a
single MAC filter slot and GP then reports mac_cnt=65. Without this
change, KASAN reports a slab-out-of-bounds write in
ncsi_rsp_handler_gp(); with this change applied, the same reply is
rejected with -ERANGE.
Fixes: 062b3e1b6d4f ("net/ncsi: Refactor MAC, VLAN filters")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
net/ncsi/ncsi-rsp.c | 46 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 37 insertions(+), 9 deletions(-)
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 1fe061ede26d..47ddf2bbb13b 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -22,6 +22,8 @@
/* Nibbles within [0xA, 0xF] add zero "0" to the returned value.
* Optional fields (encoded as 0xFF) will default to zero.
*/
+#define NCSI_FILTER_BITS BITS_PER_TYPE(u64)
+
static u8 decode_bcd_u8(u8 x)
{
int lo = x & 0xF;
@@ -32,6 +34,12 @@ static u8 decode_bcd_u8(u8 x)
return lo + hi * 10;
}
+static bool ncsi_filter_is_enabled(unsigned long enable, unsigned int index,
+ unsigned int nbits)
+{
+ return index < nbits && (enable & BIT(index));
+}
+
static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
unsigned short payload)
{
@@ -481,7 +489,8 @@ static int ncsi_rsp_handler_sma(struct ncsi_request *nr)
bitmap = &ncf->bitmap;
if (cmd->index == 0 ||
- cmd->index > ncf->n_uc + ncf->n_mc + ncf->n_mixed)
+ cmd->index > ncf->n_uc + ncf->n_mc + ncf->n_mixed ||
+ cmd->index > NCSI_FILTER_BITS)
return -ERANGE;
index = (cmd->index - 1) * ETH_ALEN;
@@ -798,6 +807,7 @@ static int ncsi_rsp_handler_gc(struct ncsi_request *nr)
struct ncsi_channel *nc;
struct ncsi_package *np;
size_t size;
+ unsigned int vlan_cnt;
/* Find the channel */
rsp = (struct ncsi_rsp_gc_pkt *)skb_network_header(nr->rsp);
@@ -819,6 +829,12 @@ static int ncsi_rsp_handler_gc(struct ncsi_request *nr)
nc->caps[NCSI_CAP_VLAN].cap = rsp->vlan_mode &
NCSI_CAP_VLAN_MASK;
+ vlan_cnt = min_t(unsigned int, rsp->vlan_cnt, NCSI_FILTER_BITS);
+ if (vlan_cnt != rsp->vlan_cnt)
+ netdev_warn(ndp->ndev.dev,
+ "NCSI: VLAN filter count %u exceeds software limit %u\n",
+ rsp->vlan_cnt, (unsigned int)NCSI_FILTER_BITS);
+
size = (rsp->uc_cnt + rsp->mc_cnt + rsp->mixed_cnt) * ETH_ALEN;
nc->mac_filter.addrs = kzalloc(size, GFP_ATOMIC);
if (!nc->mac_filter.addrs)
@@ -827,7 +843,7 @@ static int ncsi_rsp_handler_gc(struct ncsi_request *nr)
nc->mac_filter.n_mc = rsp->mc_cnt;
nc->mac_filter.n_mixed = rsp->mixed_cnt;
- nc->vlan_filter.vids = kcalloc(rsp->vlan_cnt,
+ nc->vlan_filter.vids = kcalloc(vlan_cnt,
sizeof(*nc->vlan_filter.vids),
GFP_ATOMIC);
if (!nc->vlan_filter.vids)
@@ -836,7 +852,7 @@ static int ncsi_rsp_handler_gc(struct ncsi_request *nr)
* configuration state
*/
nc->vlan_filter.bitmap = U64_MAX;
- nc->vlan_filter.n_vids = rsp->vlan_cnt;
+ nc->vlan_filter.n_vids = vlan_cnt;
np->ndp->channel_count = rsp->channel_cnt;
return 0;
@@ -853,6 +869,9 @@ static int ncsi_rsp_handler_gp(struct ncsi_request *nr)
unsigned char *pdata;
unsigned long flags;
void *bitmap;
+ unsigned int mac_cnt;
+ unsigned int mac_nbits;
+ unsigned int vlan_cnt;
int i;
/* Find the channel */
@@ -862,6 +881,15 @@ static int ncsi_rsp_handler_gp(struct ncsi_request *nr)
if (!nc)
return -ENODEV;
+ ncmf = &nc->mac_filter;
+ ncvf = &nc->vlan_filter;
+ mac_cnt = min_t(unsigned int, rsp->mac_cnt, NCSI_FILTER_BITS);
+ mac_nbits = ncmf->n_uc + ncmf->n_mc + ncmf->n_mixed;
+ vlan_cnt = min_t(unsigned int, rsp->vlan_cnt, ncvf->n_vids);
+
+ if (rsp->mac_cnt > mac_nbits || rsp->vlan_cnt > ncvf->n_vids)
+ return -ERANGE;
+
/* Modes with explicit enabled indications */
if (ntohl(rsp->valid_modes) & 0x1) { /* BC filter mode */
nc->modes[NCSI_MODE_BC].enable = 1;
@@ -887,11 +915,11 @@ static int ncsi_rsp_handler_gp(struct ncsi_request *nr)
/* 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) {
- if (!(enable & (0x1 << i)))
+ for (i = 0; i < mac_cnt; i++, pdata += 6) {
+ if (!ncsi_filter_is_enabled(enable, i,
+ BITS_PER_TYPE(rsp->mac_enable)))
clear_bit(i, bitmap);
else
set_bit(i, bitmap);
@@ -902,11 +930,11 @@ 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) {
- if (!(enable & (0x1 << i)))
+ for (i = 0; i < vlan_cnt; i++, pdata += 2) {
+ if (!ncsi_filter_is_enabled(enable, i,
+ BITS_PER_TYPE(rsp->vlan_enable)))
clear_bit(i, bitmap);
else
set_bit(i, bitmap);
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net 3/6] net/ncsi: validate GMCMA address counts against the payload
2026-04-22 16:03 [PATCH net 0/6] net/ncsi: harden packet parsing against malformed BMC replies Michael Bommarito
2026-04-22 16:03 ` [PATCH net 1/6] net/ncsi: validate response packet lengths against the skb Michael Bommarito
2026-04-22 16:03 ` [PATCH net 2/6] net/ncsi: bound filter table state to software limits Michael Bommarito
@ 2026-04-22 16:03 ` Michael Bommarito
2026-04-22 16:03 ` [PATCH net 4/6] net/ncsi: validate OEM response payloads before parsing Michael Bommarito
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Michael Bommarito @ 2026-04-22 16:03 UTC (permalink / raw)
To: Samuel Mendoza-Jonas, Paul Fertser, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, linux-kernel, Michael Bommarito, stable
Get MC MAC Address responses carry a flexible array of provisioned
addresses, but the handler currently trusts address_count without first
checking that the advertised payload actually contains that many MAC
entries.
Validate the fixed GMCMA fields plus checksum, then make sure the
address_count fits in the remaining payload before the handler walks
the address array.
Fixes: b8291cf3d118 ("net/ncsi: Add NC-SI 1.2 Get MC MAC Address command")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
net/ncsi/ncsi-rsp.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 47ddf2bbb13b..cbddb2012f90 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -40,6 +40,14 @@ static bool ncsi_filter_is_enabled(unsigned long enable, unsigned int index,
return index < nbits && (enable & BIT(index));
}
+static unsigned int ncsi_rsp_payload(struct sk_buff *skb)
+{
+ struct ncsi_rsp_pkt_hdr *h;
+
+ h = (struct ncsi_rsp_pkt_hdr *)skb_network_header(skb);
+ return ntohs(h->common.length);
+}
+
static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
unsigned short payload)
{
@@ -1127,9 +1135,21 @@ static int ncsi_rsp_handler_gmcma(struct ncsi_request *nr)
struct sockaddr_storage *saddr = &ndp->pending_mac;
struct net_device *ndev = ndp->ndev.dev;
struct ncsi_rsp_gmcma_pkt *rsp;
+ unsigned int addr_bytes;
+ unsigned int payload;
int i;
rsp = (struct ncsi_rsp_gmcma_pkt *)skb_network_header(nr->rsp);
+ payload = ncsi_rsp_payload(nr->rsp);
+ if (payload < sizeof(rsp->address_count) + sizeof(rsp->reserved) +
+ sizeof(__be32))
+ return -EINVAL;
+
+ addr_bytes = payload - sizeof(rsp->address_count) -
+ sizeof(rsp->reserved) - sizeof(__be32);
+ if (rsp->address_count > addr_bytes / ETH_ALEN)
+ return -EINVAL;
+
ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
netdev_info(ndev, "NCSI: Received %d provisioned MAC addresses\n",
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net 4/6] net/ncsi: validate OEM response payloads before parsing
2026-04-22 16:03 [PATCH net 0/6] net/ncsi: harden packet parsing against malformed BMC replies Michael Bommarito
` (2 preceding siblings ...)
2026-04-22 16:03 ` [PATCH net 3/6] net/ncsi: validate GMCMA address counts against the payload Michael Bommarito
@ 2026-04-22 16:03 ` Michael Bommarito
2026-04-22 16:03 ` [PATCH net 5/6] net/ncsi: validate AEN packet lengths against the skb Michael Bommarito
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Michael Bommarito @ 2026-04-22 16:03 UTC (permalink / raw)
To: Samuel Mendoza-Jonas, Paul Fertser, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, linux-kernel, Michael Bommarito, stable
Reject truncated OEM responses before reading the manufacturer ID,
vendor-specific subheaders, or vendor MAC address payloads.
The OEM response dispatcher reads rsp->mfr_id without verifying that the
skb contains the manufacturer field and checksum. The Mellanox,
Broadcom, and Intel handlers then read their command-specific headers
without checking that the payload is large enough for those fields. The
shared GMA helper finally copies a MAC address from a
manufacturer-specific offset without validating that the payload reaches
that offset.
Validate the advertised payload before each of those reads so malformed
or truncated BMC responses are rejected before the parser touches data
past the end of the skb.
Fixes: fb4ee67529ff ("net/ncsi: Add NCSI OEM command support")
Fixes: cb10c7c0dfd9 ("net/ncsi: Add NCSI Broadcom OEM command")
Fixes: 16e8c4ca21a2 ("net/ncsi: Add NCSI Mellanox OEM command")
Fixes: 205b95fe658d ("net/ncsi: add get MAC address command to get Intel i210 MAC address")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
net/ncsi/ncsi-rsp.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index cbddb2012f90..94354dca23ea 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -656,6 +656,7 @@ static int ncsi_rsp_handler_oem_gma(struct ncsi_request *nr, int mfr_id)
struct net_device *ndev = ndp->ndev.dev;
struct ncsi_rsp_oem_pkt *rsp;
u32 mac_addr_off = 0;
+ unsigned int payload;
/* Get the response header */
rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
@@ -668,6 +669,11 @@ static int ncsi_rsp_handler_oem_gma(struct ncsi_request *nr, int mfr_id)
else if (mfr_id == NCSI_OEM_MFR_INTEL_ID)
mac_addr_off = INTEL_MAC_ADDR_OFFSET;
+ payload = ncsi_rsp_payload(nr->rsp);
+ if (payload < sizeof(rsp->mfr_id) + mac_addr_off + ETH_ALEN +
+ sizeof(__be32))
+ return -EINVAL;
+
saddr->ss_family = ndev->type;
memcpy(saddr->__data, &rsp->data[mac_addr_off], ETH_ALEN);
if (mfr_id == NCSI_OEM_MFR_BCM_ID || mfr_id == NCSI_OEM_MFR_INTEL_ID)
@@ -686,9 +692,14 @@ static int ncsi_rsp_handler_oem_mlx(struct ncsi_request *nr)
{
struct ncsi_rsp_oem_mlx_pkt *mlx;
struct ncsi_rsp_oem_pkt *rsp;
+ unsigned int payload;
/* Get the response header */
rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+ payload = ncsi_rsp_payload(nr->rsp);
+ if (payload < sizeof(rsp->mfr_id) + sizeof(*mlx) + sizeof(__be32))
+ return -EINVAL;
+
mlx = (struct ncsi_rsp_oem_mlx_pkt *)(rsp->data);
if (mlx->cmd == NCSI_OEM_MLX_CMD_GMA &&
@@ -702,9 +713,14 @@ static int ncsi_rsp_handler_oem_bcm(struct ncsi_request *nr)
{
struct ncsi_rsp_oem_bcm_pkt *bcm;
struct ncsi_rsp_oem_pkt *rsp;
+ unsigned int payload;
/* Get the response header */
rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+ payload = ncsi_rsp_payload(nr->rsp);
+ if (payload < sizeof(rsp->mfr_id) + sizeof(*bcm) + sizeof(__be32))
+ return -EINVAL;
+
bcm = (struct ncsi_rsp_oem_bcm_pkt *)(rsp->data);
if (bcm->type == NCSI_OEM_BCM_CMD_GMA)
@@ -717,9 +733,14 @@ static int ncsi_rsp_handler_oem_intel(struct ncsi_request *nr)
{
struct ncsi_rsp_oem_intel_pkt *intel;
struct ncsi_rsp_oem_pkt *rsp;
+ unsigned int payload;
/* Get the response header */
rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+ payload = ncsi_rsp_payload(nr->rsp);
+ if (payload < sizeof(rsp->mfr_id) + sizeof(*intel) + sizeof(__be32))
+ return -EINVAL;
+
intel = (struct ncsi_rsp_oem_intel_pkt *)(rsp->data);
if (intel->cmd == NCSI_OEM_INTEL_CMD_GMA)
@@ -742,10 +763,15 @@ static int ncsi_rsp_handler_oem(struct ncsi_request *nr)
{
struct ncsi_rsp_oem_handler *nrh = NULL;
struct ncsi_rsp_oem_pkt *rsp;
+ unsigned int payload;
unsigned int mfr_id, i;
/* Get the response header */
rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+ payload = ncsi_rsp_payload(nr->rsp);
+ if (payload < sizeof(rsp->mfr_id) + sizeof(__be32))
+ return -EINVAL;
+
mfr_id = ntohl(rsp->mfr_id);
/* Check for manufacturer id and Find the handler */
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net 5/6] net/ncsi: validate AEN packet lengths against the skb
2026-04-22 16:03 [PATCH net 0/6] net/ncsi: harden packet parsing against malformed BMC replies Michael Bommarito
` (3 preceding siblings ...)
2026-04-22 16:03 ` [PATCH net 4/6] net/ncsi: validate OEM response payloads before parsing Michael Bommarito
@ 2026-04-22 16:03 ` Michael Bommarito
2026-04-22 16:03 ` [PATCH net 6/6] net/ncsi: validate GP payload lengths before parsing Michael Bommarito
2026-04-22 16:44 ` [PATCH net 0/6] net/ncsi: harden packet parsing against malformed BMC replies Paul Fertser
6 siblings, 0 replies; 9+ messages in thread
From: Michael Bommarito @ 2026-04-22 16:03 UTC (permalink / raw)
To: Samuel Mendoza-Jonas, Paul Fertser, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, linux-kernel, Michael Bommarito, stable
AEN packets are dispatched after only pulling the 16-byte common
header. ncsi_aen_handler() then reads the 20-byte AEN header to
select a per-type handler, and ncsi_validate_aen_pkt() walks
farther into the payload and checksum without first ensuring the
skb contains those bytes.
Pull the AEN-specific header before reading h->type, and pull the
full AEN header plus aligned payload before checksum validation.
That keeps short AEN packets from reading past the skb tail on the
AEN path.
Fixes: 2d283bdd079c ("net/ncsi: Resource management")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
net/ncsi/ncsi-aen.c | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index 040a31557201..cd34ef144cf8 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -16,11 +16,19 @@
#include "internal.h"
#include "ncsi-pkt.h"
-static int ncsi_validate_aen_pkt(struct ncsi_aen_pkt_hdr *h,
+static int ncsi_validate_aen_pkt(struct sk_buff *skb,
const unsigned short payload)
{
+ struct ncsi_aen_pkt_hdr *h;
u32 checksum;
__be32 *pchecksum;
+ unsigned int len;
+
+ len = skb_network_offset(skb) + sizeof(*h) + ALIGN(payload, 4);
+ if (!pskb_may_pull(skb, len))
+ return -EINVAL;
+
+ h = (struct ncsi_aen_pkt_hdr *)skb_network_header(skb);
if (h->common.revision != NCSI_PKT_REVISION)
return -EINVAL;
@@ -31,7 +39,7 @@ static int ncsi_validate_aen_pkt(struct ncsi_aen_pkt_hdr *h,
* sender doesn't support checksum according to NCSI
* specification.
*/
- pchecksum = (__be32 *)((void *)(h + 1) + payload - 4);
+ pchecksum = (__be32 *)((void *)(h + 1) + ALIGN(payload, 4) - 4);
if (ntohl(*pchecksum) == 0)
return 0;
@@ -210,12 +218,19 @@ int ncsi_aen_handler(struct ncsi_dev_priv *ndp, struct sk_buff *skb)
{
struct ncsi_aen_pkt_hdr *h;
struct ncsi_aen_handler *nah = NULL;
+ unsigned char type;
int i, ret;
+ if (!pskb_may_pull(skb, skb_network_offset(skb) + sizeof(*h))) {
+ ret = -EINVAL;
+ goto out;
+ }
+
/* Find the handler */
h = (struct ncsi_aen_pkt_hdr *)skb_network_header(skb);
+ type = h->type;
for (i = 0; i < ARRAY_SIZE(ncsi_aen_handlers); i++) {
- if (ncsi_aen_handlers[i].type == h->type) {
+ if (ncsi_aen_handlers[i].type == type) {
nah = &ncsi_aen_handlers[i];
break;
}
@@ -223,24 +238,25 @@ int ncsi_aen_handler(struct ncsi_dev_priv *ndp, struct sk_buff *skb)
if (!nah) {
netdev_warn(ndp->ndev.dev, "Invalid AEN (0x%x) received\n",
- h->type);
+ type);
ret = -ENOENT;
goto out;
}
- ret = ncsi_validate_aen_pkt(h, nah->payload);
+ ret = ncsi_validate_aen_pkt(skb, nah->payload);
if (ret) {
netdev_warn(ndp->ndev.dev,
"NCSI: 'bad' packet ignored for AEN type 0x%x\n",
- h->type);
+ type);
goto out;
}
+ h = (struct ncsi_aen_pkt_hdr *)skb_network_header(skb);
ret = nah->handler(ndp, h);
if (ret)
netdev_err(ndp->ndev.dev,
"NCSI: Handler for AEN type 0x%x returned %d\n",
- h->type, ret);
+ type, ret);
out:
consume_skb(skb);
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH net 6/6] net/ncsi: validate GP payload lengths before parsing
2026-04-22 16:03 [PATCH net 0/6] net/ncsi: harden packet parsing against malformed BMC replies Michael Bommarito
` (4 preceding siblings ...)
2026-04-22 16:03 ` [PATCH net 5/6] net/ncsi: validate AEN packet lengths against the skb Michael Bommarito
@ 2026-04-22 16:03 ` Michael Bommarito
2026-04-22 16:44 ` [PATCH net 0/6] net/ncsi: harden packet parsing against malformed BMC replies Paul Fertser
6 siblings, 0 replies; 9+ messages in thread
From: Michael Bommarito @ 2026-04-22 16:03 UTC (permalink / raw)
To: Samuel Mendoza-Jonas, Paul Fertser, netdev
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, linux-kernel, Michael Bommarito, stable
ncsi_rsp_handler_gp() now bounds MAC and VLAN counts to software
and GC-reported limits, but it still assumes the advertised GP
payload is large enough for the fixed fields plus the consumed
filter-table bytes. A short GP reply can still make parsing start
past the payload or walk beyond its tail.
Validate that the declared GP payload covers the fixed GP prefix,
the consumed MAC and VLAN entries, and the checksum before parsing
the filter tables.
Fixes: 062b3e1b6d4f ("net/ncsi: Refactor MAC, VLAN filters")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
net/ncsi/ncsi-rsp.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 94354dca23ea..565d38fd4b92 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -899,6 +899,8 @@ static int ncsi_rsp_handler_gp(struct ncsi_request *nr)
struct ncsi_dev_priv *ndp = nr->ndp;
struct ncsi_rsp_gp_pkt *rsp;
struct ncsi_channel *nc;
+ size_t needed;
+ unsigned int payload;
unsigned short enable;
unsigned char *pdata;
unsigned long flags;
@@ -924,6 +926,14 @@ static int ncsi_rsp_handler_gp(struct ncsi_request *nr)
if (rsp->mac_cnt > mac_nbits || rsp->vlan_cnt > ncvf->n_vids)
return -ERANGE;
+ payload = ncsi_rsp_payload(nr->rsp);
+ needed = offsetof(struct ncsi_rsp_gp_pkt, mac) - sizeof(rsp->rsp);
+ needed += mac_cnt * ETH_ALEN;
+ needed += vlan_cnt * sizeof(__be16);
+ needed += sizeof(rsp->checksum);
+ if (payload < needed)
+ return -EINVAL;
+
/* Modes with explicit enabled indications */
if (ntohl(rsp->valid_modes) & 0x1) { /* BC filter mode */
nc->modes[NCSI_MODE_BC].enable = 1;
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net 0/6] net/ncsi: harden packet parsing against malformed BMC replies
2026-04-22 16:03 [PATCH net 0/6] net/ncsi: harden packet parsing against malformed BMC replies Michael Bommarito
` (5 preceding siblings ...)
2026-04-22 16:03 ` [PATCH net 6/6] net/ncsi: validate GP payload lengths before parsing Michael Bommarito
@ 2026-04-22 16:44 ` Paul Fertser
2026-04-22 17:06 ` Michael Bommarito
6 siblings, 1 reply; 9+ messages in thread
From: Paul Fertser @ 2026-04-22 16:44 UTC (permalink / raw)
To: Michael Bommarito
Cc: Samuel Mendoza-Jonas, netdev, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel
Hello Michael,
On Wed, Apr 22, 2026 at 12:03:36PM -0400, Michael Bommarito wrote:
> NC-SI treats the management controller as privileged, but the Linux
...
> The threat model here is a compromised BMC or management-channel MITM
> on the NC-SI link.
The subject of the cover letter and the quoted fragment suggest that
you have a wrong impression of where NC-SI links exist and what they
carry, let me try to clarify.
On motherboards with BMC (the management controller) there often is a
way for the BMC (dedicated SoC these days) to talk to the
host-controlled NIC via NC-SI which is basically RMII (normally used
to talk to Ethernet PHY but here it's used to talk to a whole big NIC)
on hardware level plus special kind of frames sent in-band for
(partial) control and monitoring of the NIC. And regular frames are
transmitted over the same set of signals, there's no dedicated channel
for any kind of management inside NC-SI.
The code your patches modify always runs only on the BMC itself, the
packets parsed are generated by a NIC directly.
So if anything, the threat model here is compromised NIC
firmware. MITMing sounds unlikely as that would require tricky
hardware modifications and if you can do that it's easier to put a
modified NIC instead.
The idea to not trust anything coming from a NIC too much is good in
general but please take the correct context into account when
reasoning about the patches.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net 0/6] net/ncsi: harden packet parsing against malformed BMC replies
2026-04-22 16:44 ` [PATCH net 0/6] net/ncsi: harden packet parsing against malformed BMC replies Paul Fertser
@ 2026-04-22 17:06 ` Michael Bommarito
0 siblings, 0 replies; 9+ messages in thread
From: Michael Bommarito @ 2026-04-22 17:06 UTC (permalink / raw)
To: Paul Fertser
Cc: Samuel Mendoza-Jonas, netdev, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, linux-kernel
On Wed, Apr 22, 2026 at 12:44 PM Paul Fertser <fercerpav@gmail.com> wrote:
> The code your patches modify always runs only on the BMC itself, the
> packets parsed are generated by a NIC directly.
>
> So if anything, the threat model here is compromised NIC
> firmware. MITMing sounds unlikely as that would require tricky
> hardware modifications and if you can do that it's easier to put a
> modified NIC instead.
Thanks for the correction, I definitely got that backwards!
Should I submit a v2 with the corrected notes or wait for any other
feedback / review on patch substance (assuming you think it's actually
worth merging)?
Thanks,
Mike Bommarito
^ permalink raw reply [flat|nested] 9+ messages in thread