USBFS Bugfix

[Chase Douglas]

Hi Markus,

On Wed, Mar 24, 2010 at 12:21 PM, Markus Rechberger
<mrechberger at gmail.com> wrote:
> Hi,
>
> on IRC I was recommended to submit this information to the mailinglist
>
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/544527

The bug itself doesn't list much information. It basically says "usbfs
is broken." We need some proof of what is going wrong, preferably
something that can be tested when a fix is available.

> Here's a better solution.  In theory we could copy just the individual
> packets from within the transfer buffer, but that would probably take
> longer than simply copying the whole buffer.
>
> (This was a little hasty; I haven't even compile-tested the patch.
> Some small fixes may be needed.)

The general procedure for fixes to reach Ubuntu is to have them sent
upstream and be included in the mainline linux, and then to have the
fix read the -stable tree. We then pull patches from the -stable tree
for inclusion in Ubuntu.

If you can show that a specific patch has been found to fix an issue
in Ubuntu and it has been accepted upstream by the maintainers of the
kernel subsystem this impacts, we may be able to accept the patch as a
pre-stable fix, in anticipation that it will be accepted into the
-stable tree in the future.

What is the origin of this patch? Is it from upstream somewhere? A
commit hash from linux-2.6 or some other tree would be very helpful.

> Alan Stern
>
>
> -----------------------------------------------------------------------
> This patch fixes a bug in the way isochronous input data is returned
> to userspace for usbfs transfers.  The entire buffer must be copied,
> not just the first actual_length bytes, because the individual packets
> will be discontiguous if any of them are short.
>
> Signed-off-by: Alan Stern <stern at rowland.harvard.edu>
> CC: stable <stable at kernel.org>
>
> ---
> Index: usb-2.6/drivers/usb/core/devio.c
> ===================================================================
> --- usb-2.6.orig/drivers/usb/core/devio.c
> +++ usb-2.6/drivers/usb/core/devio.c
> @@ -1176,6 +1176,13 @@ static int proc_do_submiturb(struct dev_
>                        free_async(as);
>                        return -ENOMEM;
>                }
> +               /* Isochronous input data may end up being discontiguous
> +                * if some of the packets are short.  Clear the buffer so
> +                * that the gaps don't leak kernel data to userspace.
> +                */
> +               if (is_in && uurb->type == USBDEVFS_URB_TYPE_ISO)
> +                       memset(as->urb->transfer_buffer, 0,
> +                                       uurb->buffer_length);
>        }
>        as->urb->dev = ps->dev;
>        as->urb->pipe = (uurb->type << 30) |
> @@ -1312,10 +1319,14 @@ static int processcompl(struct async *as
>        void __user *addr = as->userurb;
>        unsigned int i;
>
> -       if (as->userbuffer && urb->actual_length)
> -               if (copy_to_user(as->userbuffer, urb->transfer_buffer,
> -                                urb->actual_length))
> +       if (as->userbuffer && urb->actual_length) {
> +               if (urb->number_of_packets > 0)         /* Isochronous */
> +                       i = urb->transfer_buffer_length;
> +               else                                    /* Non-Isoc */
> +                       i = urb->actual_length;
> +               if (copy_to_user(as->userbuffer, urb->transfer_buffer, i))
>                        goto err_out;
> +       }
>        if (put_user(as->status, &userurb->status))
>                goto err_out;
>        if (put_user(urb->actual_length, &userurb->actual_length))
>
> It would be nice if this patch could go into the ubuntu lucid kernel
> as soon as possible.
>
> Thanks,
> Markus

-- Chase