[PATCH v2 0/2] net: usb: cdc_ncm: fix NDP nframes bounds check

[PATCH v2 0/2] net: usb: cdc_ncm: fix NDP nframes bounds check

[tobgaertner]

The nframes bounds check in cdc_ncm_rx_fixup() validates that the NDP
fits within the skb, but omits ndpoffset from the calculation. This
allows a malicious USB device to place a valid-looking NDP at a large
offset near the end of the skb, where the frame entries extend past the
skb boundary, causing an out-of-bounds read when iterating the NDP
entries.

Fix by including ndpoffset in the size comparison, so the check
correctly verifies that the entire NDP (header + frame entries) starting
at ndpoffset fits within skb->len.

Also use struct_size_t() for the NDP size calculation instead of manual
sizeof() arithmetic, as suggested by review.

Changes since v1:
- Split into two patches with separate Fixes tags for NDP16 and NDP32
- Use struct_size_t() instead of manual sizeof() + count * sizeof()
- Verified fix prevents out-of-bounds read via fuzzer regression test

tobgaertner (2):
  net: usb: cdc_ncm: add ndpoffset to NDP16 nframes bounds check
  net: usb: cdc_ncm: add ndpoffset to NDP32 nframes bounds check

 drivers/net/usb/cdc_ncm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] net: usb: cdc_ncm: add ndpoffset to NDP16 nframes bounds check

[tobgaertner]

cdc_ncm_rx_verify_ndp16() validates that the NDP header and its DPE
entries fit within the skb. The first check correctly accounts for
ndpoffset:

  if ((ndpoffset + sizeof(struct usb_cdc_ncm_ndp16)) > skb_in->len)

but the second check omits it:

  if ((sizeof(struct usb_cdc_ncm_ndp16) +
       ret * (sizeof(struct usb_cdc_ncm_dpe16))) > skb_in->len)

This validates the DPE array size against the total skb length as if
the NDP were at offset 0, rather than at ndpoffset. When the NDP is
placed near the end of the NTB (large wNdpIndex), the DPE entries can
extend past the skb data buffer even though the check passes.
cdc_ncm_rx_fixup() then reads out-of-bounds memory when iterating
the DPE array.

Add ndpoffset to the nframes bounds check and use struct_size_t() to
express the NDP-plus-DPE-array size more clearly.

Found by coverage-guided fuzzing with QEMU system-mode emulation.

Fixes: ff06ab13a4cc ("net: cdc_ncm: splitting rx_fixup for code reuse")
Signed-off-by: Tobi Gaertner <tob.gaertner@me.com>
---
 drivers/net/usb/cdc_ncm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 5d123df0a..846f8b997 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1675,8 +1675,8 @@ int cdc_ncm_rx_verify_ndp16(struct sk_buff *skb_in, int ndpoffset)
 					sizeof(struct usb_cdc_ncm_dpe16));
 	ret--; /* we process NDP entries except for the last one */
 
-	if ((sizeof(struct usb_cdc_ncm_ndp16) +
-	     ret * (sizeof(struct usb_cdc_ncm_dpe16))) > skb_in->len) {
+	if (ndpoffset + struct_size_t(struct usb_cdc_ncm_ndp16,
+				     dpe16, ret) > skb_in->len) {
 		netif_dbg(dev, rx_err, dev->net, "Invalid nframes = %d\n", ret);
 		ret = -EINVAL;
 	}
-- 
2.43.0

[PATCH v2 2/2] net: usb: cdc_ncm: add ndpoffset to NDP32 nframes bounds check

[tobgaertner]

The same bounds-check bug fixed for NDP16 in the previous patch also
exists in cdc_ncm_rx_verify_ndp32(). The DPE array size is validated
against the total skb length without accounting for ndpoffset, allowing
out-of-bounds reads when the NDP32 is placed near the end of the NTB.

Add ndpoffset to the nframes bounds check and use struct_size_t() to
express the NDP-plus-DPE-array size more clearly.

Fixes: 0fa81b304a79 ("cdc_ncm: Implement the 32-bit version of NCM Transfer Block")
Signed-off-by: Tobi Gaertner <tob.gaertner@me.com>compile-tested only.
---
 drivers/net/usb/cdc_ncm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 846f8b997..0689169d3 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1711,8 +1711,8 @@ int cdc_ncm_rx_verify_ndp32(struct sk_buff *skb_in, int ndpoffset)
 					sizeof(struct usb_cdc_ncm_dpe32));
 	ret--; /* we process NDP entries except for the last one */
 
-	if ((sizeof(struct usb_cdc_ncm_ndp32) +
-	     ret * (sizeof(struct usb_cdc_ncm_dpe32))) > skb_in->len) {
+	if (ndpoffset + struct_size_t(struct usb_cdc_ncm_ndp32,
+				     dpe32, ret) > skb_in->len) {
 		netif_dbg(dev, rx_err, dev->net, "Invalid nframes = %d\n", ret);
 		ret = -EINVAL;
 	}
-- 
2.43.0

Re: [PATCH v2 0/2] net: usb: cdc_ncm: fix NDP nframes bounds check

[Simon Horman]

On Mon, Mar 09, 2026 at 01:34:47PM -0700, tobgaertner wrote:
> The nframes bounds check in cdc_ncm_rx_fixup() validates that the NDP
> fits within the skb, but omits ndpoffset from the calculation. This
> allows a malicious USB device to place a valid-looking NDP at a large
> offset near the end of the skb, where the frame entries extend past the
> skb boundary, causing an out-of-bounds read when iterating the NDP
> entries.
> 
> Fix by including ndpoffset in the size comparison, so the check
> correctly verifies that the entire NDP (header + frame entries) starting
> at ndpoffset fits within skb->len.
> 
> Also use struct_size_t() for the NDP size calculation instead of manual
> sizeof() arithmetic, as suggested by review.
> 
> Changes since v1:
> - Split into two patches with separate Fixes tags for NDP16 and NDP32
> - Use struct_size_t() instead of manual sizeof() + count * sizeof()
> - Verified fix prevents out-of-bounds read via fuzzer regression test

Thanks for the update,

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

Re: [PATCH v2 1/2] net: usb: cdc_ncm: add ndpoffset to NDP16 nframes bounds check

[Jakub Kicinski]

On Mon,  9 Mar 2026 13:34:48 -0700 tobgaertner wrote:
> From: tobgaertner <tob.gaertner@me.com>

the From line should match your SoB tag
You probably need to commit --amend --reset-author

> -	if ((sizeof(struct usb_cdc_ncm_ndp16) +
> -	     ret * (sizeof(struct usb_cdc_ncm_dpe16))) > skb_in->len) {
> +	if (ndpoffset + struct_size_t(struct usb_cdc_ncm_ndp16,
> +				     dpe16, ret) > skb_in->len) {
>  		netif_dbg(dev, rx_err, dev->net, "Invalid nframes = %d\n", ret);

nit: maybe save the size of the struct to a temp variable?
To avoid the awkward line wrap

Re: [PATCH v2 2/2] net: usb: cdc_ncm: add ndpoffset to NDP32 nframes bounds check

[Jakub Kicinski]

On Mon,  9 Mar 2026 13:34:49 -0700 tobgaertner wrote:
> Signed-off-by: Tobi Gaertner <tob.gaertner@me.com>compile-tested only.

The "Compile-tested only" belongs in the body of the commit msg,
please don't pollute the sob line with it