usbip: Fix uninitialized symbol 'nents' in stub_recv_cmd_submit()

commit 2a9125317b247f2cf35c196f968906dcf062ae2d upstream.

Smatch reported that nents is not initialized and used in
stub_recv_cmd_submit(). nents is currently initialized by sgl_alloc()
and used to allocate multiple URBs when host controller doesn't
support scatter-gather DMA. The use of uninitialized nents means that
buf_len is zero and use_sg is true. But buffer length should not be
zero when an URB uses scatter-gather DMA.

To prevent this situation, add the conditional that checks buf_len
and use_sg. And move the use of nents right after the sgl_alloc() to
avoid the use of uninitialized nents.

If the error occurs, it adds SDEV_EVENT_ERROR_MALLOC and stub_priv
will be released by stub event handler and connection will be shut
down.

Fixes: ea44d190764b ("usbip: Implement SG support to vhci-hcd and stub driver")
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Suwan Kim <suwan.kim027@gmail.com>
Acked-by: Shuah Khan <skhan@linuxfoundation.org>
Cc: stable <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20191111141035.27788-1-suwan.kim027@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
index 75d8756..8c55cd8 100644 (file)
--- a/drivers/usb/usbip/stub_rx.c
+++ b/drivers/usb/usbip/stub_rx.c
@@ -470,18 +470,50 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
        if (pipe == -1)
                return;
 
+       /*
+        * Smatch reported the error case where use_sg is true and buf_len is 0.
+        * In this case, It adds SDEV_EVENT_ERROR_MALLOC and stub_priv will be
+        * released by stub event handler and connection will be shut down.
+        */
        priv = stub_priv_alloc(sdev, pdu);
        if (!priv)
                return;
 
        buf_len = (unsigned long long)pdu->u.cmd_submit.transfer_buffer_length;
 
+       if (use_sg && !buf_len) {
+               dev_err(&udev->dev, "sg buffer with zero length\n");
+               goto err_malloc;
+       }
+
        /* allocate urb transfer buffer, if needed */
        if (buf_len) {
                if (use_sg) {
                        sgl = sgl_alloc(buf_len, GFP_KERNEL, &nents);
                        if (!sgl)
                                goto err_malloc;
+
+                       /* Check if the server's HCD supports SG */
+                       if (!udev->bus->sg_tablesize) {
+                               /*
+                                * If the server's HCD doesn't support SG, break
+                                * a single SG request into several URBs and map
+                                * each SG list entry to corresponding URB
+                                * buffer. The previously allocated SG list is
+                                * stored in priv->sgl (If the server's HCD
+                                * support SG, SG list is stored only in
+                                * urb->sg) and it is used as an indicator that
+                                * the server split single SG request into
+                                * several URBs. Later, priv->sgl is used by
+                                * stub_complete() and stub_send_ret_submit() to
+                                * reassemble the divied URBs.
+                                */
+                               support_sg = 0;
+                               num_urbs = nents;
+                               priv->completed_urbs = 0;
+                               pdu->u.cmd_submit.transfer_flags &=
+                                                               ~URB_DMA_MAP_SG;
+                       }
                } else {
                        buffer = kzalloc(buf_len, GFP_KERNEL);
                        if (!buffer)
@@ -489,24 +521,6 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
                }
        }
 
-       /* Check if the server's HCD supports SG */
-       if (use_sg && !udev->bus->sg_tablesize) {
-               /*
-                * If the server's HCD doesn't support SG, break a single SG
-                * request into several URBs and map each SG list entry to
-                * corresponding URB buffer. The previously allocated SG
-                * list is stored in priv->sgl (If the server's HCD support SG,
-                * SG list is stored only in urb->sg) and it is used as an
-                * indicator that the server split single SG request into
-                * several URBs. Later, priv->sgl is used by stub_complete() and
-                * stub_send_ret_submit() to reassemble the divied URBs.
-                */
-               support_sg = 0;
-               num_urbs = nents;
-               priv->completed_urbs = 0;
-               pdu->u.cmd_submit.transfer_flags &= ~URB_DMA_MAP_SG;
-       }
-
        /* allocate urb array */
        priv->num_urbs = num_urbs;
        priv->urbs = kmalloc_array(num_urbs, sizeof(*priv->urbs), GFP_KERNEL);