lkml.org 
[lkml]   [2019]   [Nov]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.4 55/66] usbip: Fix uninitialized symbol nents in stub_recv_cmd_submit()
    Date
    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
    @@ -470,18 +470,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)
    @@ -489,24 +521,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);

    \
     
     \ /
      Last update: 2019-11-27 22:16    [W:3.833 / U:0.184 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site