From: Suwan Kim suwan.kim027@gmail.com
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
--- drivers/usb/usbip/stub_rx.c | 50 ++++++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 18 deletions(-)
--- a/drivers/usb/usbip/stub_rx.c +++ b/drivers/usb/usbip/stub_rx.c @@ -487,18 +487,50 @@ static void stub_recv_cmd_submit(struct 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) @@ -506,24 +538,6 @@ static void stub_recv_cmd_submit(struct } }
- /* 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);