usb: gadget: ncm: validate the NDP chain before parsing

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.

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

index 64eabda2f5..085aea142f 100644
@@ -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

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,

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.

---

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(+)

index 64eabda2f5..085aea142f 100644
@@ -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