lkml.org 
[lkml]   [2009]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[patch 09/37] USB: handle zero-length usbfs submissions correctly
    2.6.27-stable review patch.  If anyone has any objections, please let us know.

    ------------------

    From: Alan Stern <stern@rowland.harvard.edu>

    commit 9180135bc80ab11199d482b6111e23f74d65af4a upstream.

    This patch (as1262) fixes a bug in usbfs: It refuses to accept
    zero-length transfers, and it insists that the buffer pointer be valid
    even if there is no data being transferred.

    The patch also consolidates a bunch of repetitive access_ok() checks
    into a single check, which incidentally fixes the lack of such a check
    for Isochronous URBs.

    Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
    Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

    ---
    drivers/usb/core/devio.c | 41 ++++++++++++++++++++---------------------
    1 file changed, 20 insertions(+), 21 deletions(-)

    --- a/drivers/usb/core/devio.c
    +++ b/drivers/usb/core/devio.c
    @@ -976,7 +976,7 @@ static int proc_do_submiturb(struct dev_
    USBDEVFS_URB_ZERO_PACKET |
    USBDEVFS_URB_NO_INTERRUPT))
    return -EINVAL;
    - if (!uurb->buffer)
    + if (uurb->buffer_length > 0 && !uurb->buffer)
    return -EINVAL;
    if (uurb->signr != 0 && (uurb->signr < SIGRTMIN ||
    uurb->signr > SIGRTMAX))
    @@ -1035,11 +1035,6 @@ static int proc_do_submiturb(struct dev_
    is_in = 0;
    uurb->endpoint &= ~USB_DIR_IN;
    }
    - if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
    - uurb->buffer, uurb->buffer_length)) {
    - kfree(dr);
    - return -EFAULT;
    - }
    snoop(&ps->dev->dev, "control urb: bRequest=%02x "
    "bRrequestType=%02x wValue=%04x "
    "wIndex=%04x wLength=%04x\n",
    @@ -1059,9 +1054,6 @@ static int proc_do_submiturb(struct dev_
    uurb->number_of_packets = 0;
    if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
    return -EINVAL;
    - if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
    - uurb->buffer, uurb->buffer_length))
    - return -EFAULT;
    snoop(&ps->dev->dev, "bulk urb\n");
    break;

    @@ -1103,28 +1095,35 @@ static int proc_do_submiturb(struct dev_
    return -EINVAL;
    if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
    return -EINVAL;
    - if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
    - uurb->buffer, uurb->buffer_length))
    - return -EFAULT;
    snoop(&ps->dev->dev, "interrupt urb\n");
    break;

    default:
    return -EINVAL;
    }
    - as = alloc_async(uurb->number_of_packets);
    - if (!as) {
    + if (uurb->buffer_length > 0 &&
    + !access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
    + uurb->buffer, uurb->buffer_length)) {
    kfree(isopkt);
    kfree(dr);
    - return -ENOMEM;
    + return -EFAULT;
    }
    - as->urb->transfer_buffer = kmalloc(uurb->buffer_length, GFP_KERNEL);
    - if (!as->urb->transfer_buffer) {
    + as = alloc_async(uurb->number_of_packets);
    + if (!as) {
    kfree(isopkt);
    kfree(dr);
    - free_async(as);
    return -ENOMEM;
    }
    + if (uurb->buffer_length > 0) {
    + as->urb->transfer_buffer = kmalloc(uurb->buffer_length,
    + GFP_KERNEL);
    + if (!as->urb->transfer_buffer) {
    + kfree(isopkt);
    + kfree(dr);
    + free_async(as);
    + return -ENOMEM;
    + }
    + }
    as->urb->dev = ps->dev;
    as->urb->pipe = (uurb->type << 30) |
    __create_pipe(ps->dev, uurb->endpoint & 0xf) |
    @@ -1166,7 +1165,7 @@ static int proc_do_submiturb(struct dev_
    kfree(isopkt);
    as->ps = ps;
    as->userurb = arg;
    - if (uurb->endpoint & USB_DIR_IN)
    + if (is_in && uurb->buffer_length > 0)
    as->userbuffer = uurb->buffer;
    else
    as->userbuffer = NULL;
    @@ -1176,9 +1175,9 @@ static int proc_do_submiturb(struct dev_
    as->uid = current->uid;
    as->euid = current->euid;
    security_task_getsecid(current, &as->secid);
    - if (!is_in) {
    + if (!is_in && uurb->buffer_length > 0) {
    if (copy_from_user(as->urb->transfer_buffer, uurb->buffer,
    - as->urb->transfer_buffer_length)) {
    + uurb->buffer_length)) {
    free_async(as);
    return -EFAULT;
    }



    \
     
     \ /
      Last update: 2009-07-29 01:27    [W:4.117 / U:0.092 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site