[PATCH] usb: gadget: f_ncm: bound NDP length against the NTB

[PATCH] usb: gadget: f_ncm: bound NDP length against the NTB

[Noah Weinberger]

Hello,

ncm_unwrap_ntb() validates the NDP wLength only for a minimum size and
alignment; the sum ndp_index + ndp_len is never checked against
block_len. The NDP *start* is bounded, but the datagram-pointer-entry
walk then advances through up to wLength/8 entries regardless of where
the block ends.

A hostile USB host can send an NTB whose NDP claims wLength close to
64K while the block is 16K: the walk reads 2-4 byte entries past the end
of the RX skb, continuing as long as the out-of-bounds-derived values
happen to pass the per-datagram validation (index/length checks). The
practical overread distance depends on adjacent heap contents, but KASAN
flags the first OOB access. The bytes are not returned to the host, so
this is an out-of-bounds read/DoS rather than a disclosure channel.

Bound the NDP length against the remaining block, mirroring the
existing minimum-size check.

Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), drivers/usb/gadget/function/f_ncm.c:1254-1346 (ncm_unwrap_ntb NDP walk).
Longstanding code.

This was found during an assisted (AI-supported) static source review of
mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
reporter against that tree. The patch below has NOT been compiled and has
NOT been runtime tested, and no reproducer was produced - the triggering
conditions are described above instead. Please treat this as a report for
verification rather than a battle-tested fix.

Reported-by: Noah Weinberger <noahweinberger2003@proton.me>
---
diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 500c9c1..a0e2049 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1262,7 +1262,8 @@ parse_ntb:
 		 */
 		if ((ndp_len < opts->ndp_size
 				+ 2 * 2 * (opts->dgram_item_len * 2)) ||
-				(ndp_len % opts->ndplen_align != 0)) {
+				(ndp_len % opts->ndplen_align != 0) ||
+					(ndp_index + ndp_len > block_len)) {
 			INFO(port->func.config->cdev, "Bad NDP length: %#X\n",
 			     ndp_len);
 			goto err;

Re: [PATCH] usb: gadget: f_ncm: bound NDP length against the NTB

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] usb: gadget: f_ncm: missing Fixes trailer for ncm_unwrap_ntb()
  bounds fix
- [Low] usb: gadget: f_ncm: missing Signed-off-by and raw report text in
  changelog
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Critical] usb: gadget: f_ncm: infinite loop in ncm_unwrap_ntb() [1]
--

commit 11f0a9e46d1b84c8d72b0c4bd312240ac8168fb4
Author: Noah Weinberger <NoahWeinberger2003@proton.me>

> usb: gadget: f_ncm: bound NDP length against the NTB
>
> Hello,
>
> ncm_unwrap_ntb() validates the NDP wLength only for a minimum size and
> alignment; the sum ndp_index + ndp_len is never checked against
> block_len. The NDP *start* is bounded, but the datagram-pointer-entry
> walk then advances through up to wLength/8 entries regardless of where
> the block ends.
>
> A hostile USB host can send an NTB whose NDP claims wLength close to
> 64K while the block is 16K: the walk reads 2-4 byte entries past the end
> of the RX skb, continuing as long as the out-of-bounds-derived values
> happen to pass the per-datagram validation (index/length checks). The
> practical overread distance depends on adjacent heap contents, but KASAN
> flags the first OOB access. The bytes are not returned to the host, so
> this is an out-of-bounds read/DoS rather than a disclosure channel.
>
> Bound the NDP length against the remaining block, mirroring the
> existing minimum-size check.
>
> Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), drivers/usb/gadget/function/f_ncm.c:1254-1346 (ncm_unwrap_ntb NDP walk).
> Longstanding code.
>
> This was found during an assisted (AI-supported) static source review of
> mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
> reporter against that tree. The patch below has NOT been compiled and has
> NOT been runtime tested, and no reproducer was produced - the triggering
> conditions are described above instead. Please treat this as a report for
> verification rather than a battle-tested fix.
>
> Reported-by: Noah Weinberger <noahweinberger2003@proton.me>

[Severity: Low]
This fixes an out-of-bounds read in ncm_unwrap_ntb().  Should it include a
Fixes: tag, such as:

  Fixes: 9f6ce4240a2b ("usb: gadget: f_ncm.c added")

[Severity: Low]
This commit is missing a Signed-off-by: trailer required by the Developer's
Certificate of Origin, and the changelog still contains the conversational
salutation ("Hello,"), report metadata, and untested-patch disclaimer that
do not belong in git history.

> diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
> index ab6da3ef87836..3cc90fc02e270 100644
> --- a/drivers/usb/gadget/function/f_ncm.c
> +++ b/drivers/usb/gadget/function/f_ncm.c
> @@ -1267,7 +1267,8 @@ static int ncm_unwrap_ntb(struct gether *port,
>  		 */
>  		if ((ndp_len < opts->ndp_size
>  				+ 2 * 2 * (opts->dgram_item_len * 2)) ||
> -				(ndp_len % opts->ndplen_align != 0)) {
> +				(ndp_len % opts->ndplen_align != 0) ||
> +					(ndp_index + ndp_len > block_len)) {
>  			INFO(port->func.config->cdev, "Bad NDP length: %#X\n",
>  			     ndp_len);
>  			goto err;

[Severity: Critical]
This problem wasn't introduced by this patch, but can a crafted NTB trigger
an infinite loop in ncm_unwrap_ntb() [1]?

Right below this check, the outer NDP loop updates ndp_index from the packet
without checking that the next NDP offset strictly advances:

drivers/usb/gadget/function/f_ncm.c:ncm_unwrap_ntb() {
	do {
		...
		/* Check for another NDP (d)wNextNdpIndex */
		ndp_index = get_ncm(&tmp, opts->next_ndp_index);
		...
	} while (ndp_index);
}

A hostile USB host can supply an NDP whose next_ndp_index points back to
itself or forms a cycle within block_len.  The loop then spins forever in
softirq context while repeatedly allocating and queueing skbs via
netdev_alloc_skb_ip_align() until atomic memory is exhausted.

[1] https://sashiko.dev/#/bug/linux-7e5803c6-1df0-4e59-9dcc-28f332907cc2

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/VFFcAz18YOCdAGLJxQp8TZxPPL6-T5bPEHLSdIkdyWPj4Rr7dRgBoUrlYHMc_CYxsZXLbk2_xPPKx8uMzHzck8QUAC84Q2ZlluLo7JG-Ry8=@proton.me?part=1