Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

usbnet: ipheth: refactor NCM datagram loop

Introduce an rx_error label to reduce repetitions in the header
signature checks.

Store wDatagramIndex and wDatagramLength after endianness conversion to
avoid repeated le16_to_cpu() calls.

Rewrite the loop to return on a null trailing DPE, which is required
by the CDC NCM spec. In case it is missing, fall through to rx_error.

This change does not fix any particular issue. Its purpose is to
simplify a subsequent commit that fixes a potential OoB read by limiting
the maximum amount of processed DPEs.

Cc: stable@vger.kernel.org # 6.5.x
Signed-off-by: Foster Snowhill <forst@pen.gy>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

authored by

Foster Snowhill and committed by
Paolo Abeni
2a9a1964 86586dcb

+23 -19
+23 -19
drivers/net/usb/ipheth.c
··· 213 213 struct usb_cdc_ncm_ndp16 *ncm0; 214 214 struct usb_cdc_ncm_dpe16 *dpe; 215 215 struct ipheth_device *dev; 216 + u16 dg_idx, dg_len; 216 217 int retval = -EINVAL; 217 218 char *buf; 218 - int len; 219 219 220 220 dev = urb->context; 221 221 ··· 227 227 ncmh = urb->transfer_buffer; 228 228 if (ncmh->dwSignature != cpu_to_le32(USB_CDC_NCM_NTH16_SIGN) || 229 229 /* On iOS, NDP16 directly follows NTH16 */ 230 - ncmh->wNdpIndex != cpu_to_le16(sizeof(struct usb_cdc_ncm_nth16))) { 231 - dev->net->stats.rx_errors++; 232 - return retval; 233 - } 230 + ncmh->wNdpIndex != cpu_to_le16(sizeof(struct usb_cdc_ncm_nth16))) 231 + goto rx_error; 234 232 235 233 ncm0 = urb->transfer_buffer + sizeof(struct usb_cdc_ncm_nth16); 236 - if (ncm0->dwSignature != cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN)) { 237 - dev->net->stats.rx_errors++; 238 - return retval; 239 - } 234 + if (ncm0->dwSignature != cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN)) 235 + goto rx_error; 240 236 241 237 dpe = ncm0->dpe16; 242 - while (le16_to_cpu(dpe->wDatagramIndex) != 0 && 243 - le16_to_cpu(dpe->wDatagramLength) != 0) { 244 - if (le16_to_cpu(dpe->wDatagramIndex) < IPHETH_NCM_HEADER_SIZE || 245 - le16_to_cpu(dpe->wDatagramIndex) >= urb->actual_length || 246 - le16_to_cpu(dpe->wDatagramLength) > urb->actual_length - 247 - le16_to_cpu(dpe->wDatagramIndex)) { 238 + while (true) { 239 + dg_idx = le16_to_cpu(dpe->wDatagramIndex); 240 + dg_len = le16_to_cpu(dpe->wDatagramLength); 241 + 242 + /* Null DPE must be present after last datagram pointer entry 243 + * (3.3.1 USB CDC NCM spec v1.0) 244 + */ 245 + if (dg_idx == 0 && dg_len == 0) 246 + return 0; 247 + 248 + if (dg_idx < IPHETH_NCM_HEADER_SIZE || 249 + dg_idx >= urb->actual_length || 250 + dg_len > urb->actual_length - dg_idx) { 248 251 dev->net->stats.rx_length_errors++; 249 252 return retval; 250 253 } 251 254 252 - buf = urb->transfer_buffer + le16_to_cpu(dpe->wDatagramIndex); 253 - len = le16_to_cpu(dpe->wDatagramLength); 255 + buf = urb->transfer_buffer + dg_idx; 254 256 255 - retval = ipheth_consume_skb(buf, len, dev); 257 + retval = ipheth_consume_skb(buf, dg_len, dev); 256 258 if (retval != 0) 257 259 return retval; 258 260 259 261 dpe++; 260 262 } 261 263 262 - return 0; 264 + rx_error: 265 + dev->net->stats.rx_errors++; 266 + return retval; 263 267 } 264 268 265 269 static void ipheth_rcvbulk_callback(struct urb *urb)