[PATCH] usb: gadget: ncm: validate the NDP chain before parsing

[PATCH] usb: gadget: ncm: validate the NDP chain before parsing

[Aldo Ariel Panzardo]

The next-NDP pointers form a chain supplied by the USB host.  A cyclic
chain therefore makes the receive path loop indefinitely and repeatedly
allocate datagram skbs.

Prewalk the chain using only bounded header reads before parsing any
NDP.  NDP offsets must be four-byte aligned, so following more than
block_len / 4 valid offsets proves that the chain contains a cycle.
This terminates cyclic chains without imposing an arbitrary limit on
valid NTBs or allocating skbs before the chain is known to terminate.

Fixes: 370af734dfaf ("usb: gadget: NCM: RX function support multiple NDPs")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 drivers/usb/gadget/function/f_ncm.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 64eabda2f5..085aea142f 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1175,6 +1175,7 @@ static int ncm_unwrap_ntb(struct gether *port,
 	unsigned	dg_len, dg_len2;
 	unsigned	ndp_len;
 	unsigned	block_len;
+	unsigned int	ndp_count, next_ndp_index;
 	struct sk_buff	*skb2;
 	int		ret = -EINVAL;
 	unsigned	ntb_max = le32_to_cpu(ntb_parameters.dwNtbOutMaxSize);
@@ -1224,6 +1225,30 @@ static int ncm_unwrap_ntb(struct gether *port,
 	}
 
 	ndp_index = get_ncm(&tmp, opts->ndp_index);
+	next_ndp_index = ndp_index;
+	ndp_count = 0;
+
+	/* Validate the NDP chain before allocating datagram skbs. */
+	while (next_ndp_index) {
+		if (next_ndp_index % 4 ||
+		    next_ndp_index < opts->nth_size ||
+		    next_ndp_index > block_len - opts->ndp_size) {
+			INFO(port->func.config->cdev, "Bad index: %#X\n",
+			     next_ndp_index);
+			goto err;
+		}
+
+		/* More aligned offsets than fit in the NTB imply a cycle. */
+		if (++ndp_count > block_len / 4) {
+			INFO(port->func.config->cdev, "NDP chain cycle\n");
+			goto err;
+		}
+
+		tmp = (__le16 *)(ntb_ptr + next_ndp_index);
+		tmp += 3; /* skip the signature and length */
+		tmp += opts->reserved1;
+		next_ndp_index = get_ncm(&tmp, opts->next_ndp_index);
+	}
 
 	/* Run through all the NDP's in the NTB */
 	do {
-- 
2.43.0

Re: [PATCH] usb: gadget: ncm: validate the NDP chain before parsing

[Krishna Kurapati]

On 9/15/2026 9:41 AM, Aldo Ariel Panzardo wrote:
> The next-NDP pointers form a chain supplied by the USB host.  A cyclic
> chain therefore makes the receive path loop indefinitely and repeatedly
> allocate datagram skbs.
> 

Is this an issue with NTB creation on host side ? (seems like it). If 
so, can you update commit message with an example packet details 
indicating the same (or a walkthrough of a packet with such headers 
indicating a cyclic chain).

Regards,
Krishna,

> Prewalk the chain using only bounded header reads before parsing any
> NDP.  NDP offsets must be four-byte aligned, so following more than
> block_len / 4 valid offsets proves that the chain contains a cycle.
> This terminates cyclic chains without imposing an arbitrary limit on
> valid NTBs or allocating skbs before the chain is known to terminate.
> 
> Fixes: 370af734dfaf ("usb: gadget: NCM: RX function support multiple NDPs")
> Cc: stable@vger.kernel.org
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
>   drivers/usb/gadget/function/f_ncm.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
> index 64eabda2f5..085aea142f 100644
> --- a/drivers/usb/gadget/function/f_ncm.c
> +++ b/drivers/usb/gadget/function/f_ncm.c
> @@ -1175,6 +1175,7 @@ static int ncm_unwrap_ntb(struct gether *port,
>   	unsigned	dg_len, dg_len2;
>   	unsigned	ndp_len;
>   	unsigned	block_len;
> +	unsigned int	ndp_count, next_ndp_index;
>   	struct sk_buff	*skb2;
>   	int		ret = -EINVAL;
>   	unsigned	ntb_max = le32_to_cpu(ntb_parameters.dwNtbOutMaxSize);
> @@ -1224,6 +1225,30 @@ static int ncm_unwrap_ntb(struct gether *port,
>   	}
>   
>   	ndp_index = get_ncm(&tmp, opts->ndp_index);
> +	next_ndp_index = ndp_index;
> +	ndp_count = 0;
> +
> +	/* Validate the NDP chain before allocating datagram skbs. */
> +	while (next_ndp_index) {
> +		if (next_ndp_index % 4 ||
> +		    next_ndp_index < opts->nth_size ||
> +		    next_ndp_index > block_len - opts->ndp_size) {
> +			INFO(port->func.config->cdev, "Bad index: %#X\n",
> +			     next_ndp_index);
> +			goto err;
> +		}
> +
> +		/* More aligned offsets than fit in the NTB imply a cycle. */
> +		if (++ndp_count > block_len / 4) {
> +			INFO(port->func.config->cdev, "NDP chain cycle\n");
> +			goto err;
> +		}
> +
> +		tmp = (__le16 *)(ntb_ptr + next_ndp_index);
> +		tmp += 3; /* skip the signature and length */
> +		tmp += opts->reserved1;
> +		next_ndp_index = get_ncm(&tmp, opts->next_ndp_index);
> +	}
>   
>   	/* Run through all the NDP's in the NTB */
>   	do {

[PATCH v2] usb: gadget: ncm: validate the NDP chain before parsing

[Aldo Ariel Panzardo]

The (d)wNextNdpIndex fields form a linked chain supplied entirely by
the USB host.  ncm_unwrap_ntb() follows this chain in a do-while loop
without cycle detection, so a malicious host can force the gadget to
repeatedly parse the same NDP and allocate datagram skbs until a
GFP_ATOMIC allocation fails.  This causes avoidable memory pressure
and allows a malicious host to deny service to the gadget receive path.

Example: a 64-byte NTB16 from the host with a single NDP whose
wNextNdpIndex points back to its own offset:

  Offset  0: NTH16  dwSignature     = "NCMH"
                     wHeaderLength   = 12
                     wBlockLength    = 64
                     wNdpIndex       = 12        <- first NDP at byte 12

  Offset 12: NDP16  dwSignature     = "NCM0"
                     wLength         = 16
                     wNextNdpIndex   = 12        <- points to itself

  Offset 20: DPE16[0]  wDatagramIndex  = 32
                        wDatagramLength = 14

  Offset 24: DPE16[1]  wDatagramIndex  = 0       <- terminator
                        wDatagramLength = 0

  Offset 32: 14-byte Ethernet frame (payload)

The existing do-while loop reads this NDP, processes the datagram
entry, reads wNextNdpIndex (12), and jumps back to the same NDP
indefinitely.

Fix this by prewalking the NDP chain using only bounded header reads
before parsing any NDP.  NDP offsets must be four-byte aligned, so
following more than block_len / 4 valid offsets proves that the chain
contains a cycle.  This terminates cyclic chains without imposing an
arbitrary limit on valid NTBs or allocating skbs before the chain is
known to terminate.  The prewalk is safe for both NDP16 and NDP32.

Fixes: 370af734dfaf ("usb: gadget: NCM: RX function support multiple NDPs")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---

Changes in v2:
  - Added NTB packet walkthrough with DPE entries showing the cyclic
    chain, as requested by Krishna Kurapati.
  - Softened impact description: allocation eventually fails and err:
    purges the skbs, so the loop is not infinite but causes avoidable
    memory pressure and receive-path DoS.
  - Use "(d)wNextNdpIndex" to cover both NCM16 and NCM32.

 drivers/usb/gadget/function/f_ncm.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/usb/gadget/function/f_ncm.c b/drivers/usb/gadget/function/f_ncm.c
index 64eabda2f5..085aea142f 100644
--- a/drivers/usb/gadget/function/f_ncm.c
+++ b/drivers/usb/gadget/function/f_ncm.c
@@ -1175,6 +1175,7 @@ static int ncm_unwrap_ntb(struct gether *port,
 	unsigned	dg_len, dg_len2;
 	unsigned	ndp_len;
 	unsigned	block_len;
+	unsigned int	ndp_count, next_ndp_index;
 	struct sk_buff	*skb2;
 	int		ret = -EINVAL;
 	unsigned	ntb_max = le32_to_cpu(ntb_parameters.dwNtbOutMaxSize);
@@ -1224,6 +1225,30 @@ static int ncm_unwrap_ntb(struct gether *port,
 	}

 	ndp_index = get_ncm(&tmp, opts->ndp_index);
+	next_ndp_index = ndp_index;
+	ndp_count = 0;
+
+	/* Validate the NDP chain before allocating datagram skbs. */
+	while (next_ndp_index) {
+		if (next_ndp_index % 4 ||
+		    next_ndp_index < opts->nth_size ||
+		    next_ndp_index > block_len - opts->ndp_size) {
+			INFO(port->func.config->cdev, "Bad index: %#X\n",
+			     next_ndp_index);
+			goto err;
+		}
+
+		/* More aligned offsets than fit in the NTB imply a cycle. */
+		if (++ndp_count > block_len / 4) {
+			INFO(port->func.config->cdev, "NDP chain cycle\n");
+			goto err;
+		}
+
+		tmp = (__le16 *)(ntb_ptr + next_ndp_index);
+		tmp += 3; /* skip the signature and length */
+		tmp += opts->reserved1;
+		next_ndp_index = get_ncm(&tmp, opts->next_ndp_index);
+	}

 	/* Run through all the NDP's in the NTB */
 	do {
--
2.43.0