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

[sashiko-bot@kernel.org]

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