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>

diff --git a/drivers/net/usb/ipheth.c b/drivers/net/usb/ipheth.c
index c385623596d2f70137385438df36cf96d3aec661..069979e2bb6ed9681a280b3cac211562a4761868 100644 (file)
--- a/drivers/net/usb/ipheth.c
+++ b/drivers/net/usb/ipheth.c
@@ -213,9 +213,9 @@ static int ipheth_rcvbulk_callback_ncm(struct urb *urb)
        struct usb_cdc_ncm_ndp16 *ncm0;
        struct usb_cdc_ncm_dpe16 *dpe;
        struct ipheth_device *dev;
+       u16 dg_idx, dg_len;
        int retval = -EINVAL;
        char *buf;
-       int len;
 
        dev = urb->context;
 
@@ -227,39 +227,43 @@ static int ipheth_rcvbulk_callback_ncm(struct urb *urb)
        ncmh = urb->transfer_buffer;
        if (ncmh->dwSignature != cpu_to_le32(USB_CDC_NCM_NTH16_SIGN) ||
            /* On iOS, NDP16 directly follows NTH16 */
-           ncmh->wNdpIndex != cpu_to_le16(sizeof(struct usb_cdc_ncm_nth16))) {
-               dev->net->stats.rx_errors++;
-               return retval;
-       }
+           ncmh->wNdpIndex != cpu_to_le16(sizeof(struct usb_cdc_ncm_nth16)))
+               goto rx_error;
 
        ncm0 = urb->transfer_buffer + sizeof(struct usb_cdc_ncm_nth16);
-       if (ncm0->dwSignature != cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN)) {
-               dev->net->stats.rx_errors++;
-               return retval;
-       }
+       if (ncm0->dwSignature != cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN))
+               goto rx_error;
 
        dpe = ncm0->dpe16;
-       while (le16_to_cpu(dpe->wDatagramIndex) != 0 &&
-              le16_to_cpu(dpe->wDatagramLength) != 0) {
-               if (le16_to_cpu(dpe->wDatagramIndex) < IPHETH_NCM_HEADER_SIZE ||
-                   le16_to_cpu(dpe->wDatagramIndex) >= urb->actual_length ||
-                   le16_to_cpu(dpe->wDatagramLength) > urb->actual_length -
-                   le16_to_cpu(dpe->wDatagramIndex)) {
+       while (true) {
+               dg_idx = le16_to_cpu(dpe->wDatagramIndex);
+               dg_len = le16_to_cpu(dpe->wDatagramLength);
+
+               /* Null DPE must be present after last datagram pointer entry
+                * (3.3.1 USB CDC NCM spec v1.0)
+                */
+               if (dg_idx == 0 && dg_len == 0)
+                       return 0;
+
+               if (dg_idx < IPHETH_NCM_HEADER_SIZE ||
+                   dg_idx >= urb->actual_length ||
+                   dg_len > urb->actual_length - dg_idx) {
                        dev->net->stats.rx_length_errors++;
                        return retval;
                }
 
-               buf = urb->transfer_buffer + le16_to_cpu(dpe->wDatagramIndex);
-               len = le16_to_cpu(dpe->wDatagramLength);
+               buf = urb->transfer_buffer + dg_idx;
 
-               retval = ipheth_consume_skb(buf, len, dev);
+               retval = ipheth_consume_skb(buf, dg_len, dev);
                if (retval != 0)
                        return retval;
 
                dpe++;
        }
 
-       return 0;
+rx_error:
+       dev->net->stats.rx_errors++;
+       return retval;
 }
 
 static void ipheth_rcvbulk_callback(struct urb *urb)