usbfs is bugged with >2.6.32.9 and <=2.6.33 (breaks VMWare, Qemu, sane scanners, ...)

Bug #544527 reported by MarkusRechberger
16
This bug affects 3 people
Affects Status Importance Assigned to Milestone
QEMU
Undecided
Unassigned
Virtualbox
Fix Committed
Undecided
Unassigned
sane-backends
Fix Committed
Undecided
Unassigned
linux (Ubuntu)
Undecided
Unassigned

Bug Description

Binary package hint: tvtime

There's a problem with isochronous and usbfs, suse tried to improve usbfs but it end up that it broke usbfs.
For isochronous the entire packet needs to be copied and not only a part of it.

http://lkml.org/lkml/2010/2/26/490 (Report)
http://lkml.org/lkml/2010/2/27/226 (Bugfix)

please merge this bugfix asap.

ProblemType: Bug
Architecture: amd64
Date: Mon Mar 22 21:09:00 2010
DistroRelease: Ubuntu 10.04
LiveMediaBuild: Ubuntu 10.04 "Lucid Lynx" - Alpha amd64 (20100322)
Package: tvtime 1.0.2-5ubuntu2
ProcEnviron:
 LANG=de_DE.UTF-8
 SHELL=/bin/bash
ProcVersionSignature: Ubuntu 2.6.32-16.25-generic
SourcePackage: tvtime
Uname: Linux 2.6.32-16-generic x86_64

Revision history for this message
MarkusRechberger (mrechberger) wrote :
Changed in tvtime (Ubuntu):
status: New → Confirmed
affects: tvtime (Ubuntu) → linux (Ubuntu)
Changed in linux (Ubuntu):
status: Confirmed → Fix Committed
Changed in sane-backends:
status: New → Fix Committed
Changed in virtualbox:
status: New → Fix Committed
Changed in tvtime:
status: New → Fix Committed
Changed in qemu:
status: New → Fix Committed
Revision history for this message
David (dvdkhlng) wrote :

This bugfix is incomplete. Isochronous transfers are still broken, when running 32-bit software on a 64-bit kernel. Function processcompl_compat() in devio.c needs a similar fix to the fix that was applied to processcompl(). Looking at processcompl_compat() I see:

 if (as->userbuffer && urb->actual_length)
  if (copy_to_user(as->userbuffer, urb->transfer_buffer,
     urb->actual_length))
   return -EFAULT;

correct code would be something like

 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;
 }

(note the difference between urb->actual_length and urb->transfer_buffer_length).

With kernel 2.6.32-23-generic x86_64 on Ubuntu 10.04, using proprietary USB-hardware hooked up to the USB bus (with software compiled for 32-bit), I can directly observe how isochronous transfers retrieved via ioctl(.. USBDEVFS_REAPURB ..) are too short, i.e. the kernel does not write the end of the data packet to the supplied buffer. Booting on the 2.6.31 kernel still present from before I upgraded from Ubuntu 9.10, the same software runs flawlessly.

As a workaround I'll use the older kernel for now (also I could compile for 64-bit, actually...).

cheers,

David

Revision history for this message
MarkusRechberger (mrechberger) wrote : Re: [Bug 544527] Re: usbfs is bugged with >2.6.32.9 and <=2.6.33 (breaks VMWare, Qemu, sane scanners, ...)

On Tue, Jul 13, 2010 at 5:14 PM, David Kühling
<email address hidden> wrote:
> This bugfix is incomplete.  Isochronous transfers are still broken, when
> running 32-bit software on a 64-bit kernel.  Function
> processcompl_compat() in devio.c needs a similar fix to the fix that was
> applied to processcompl().  Looking at processcompl_compat() I see:
>
>        if (as->userbuffer && urb->actual_length)
>                if (copy_to_user(as->userbuffer, urb->transfer_buffer,
>                                 urb->actual_length))
>                        return -EFAULT;
>
> correct code would be something like
>
>        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;
>        }
>
> (note the difference between urb->actual_length and
> urb->transfer_buffer_length).
>
> With kernel 2.6.32-23-generic x86_64 on Ubuntu 10.04, using proprietary
> USB-hardware hooked up to the USB bus (with software compiled for
> 32-bit), I can directly observe how isochronous transfers retrieved via
> ioctl(.. USBDEVFS_REAPURB ..) are too short, i.e. the kernel does not
> write the end of the data packet to the supplied buffer.  Booting on the
> 2.6.31 kernel still present from before I upgraded from Ubuntu 9.10, the
> same software runs flawlessly.
>
> As a workaround I'll use the older kernel for now (also I could compile
> for 64-bit, actually...).
>

yes you're right, since we distribute 64 and 32bit drivers it doesn't
really affect us.
before applying any change you can submit some patches to us and we
can test them if needed.

Things should definitely not go upstream untested anymore as it used
to happen in the past with various kernel releases. (isochronous is
bugged with 2.6.26/27/28 (memory leak) and 32/33 (copying wrong memory
area). Luckily our hardware supports switching from ISO to BULK in
order to work around those issues - but bulk transfers have a bad
performance in userspace

Markus
> cheers,
>
> David
>
> --
> usbfs is bugged with >2.6.32.9 and <=2.6.33 (breaks VMWare, Qemu, sane scanners, ...)
> https://bugs.launchpad.net/bugs/544527
> You received this bug notification because you are a direct subscriber
> of the bug.
>

Revision history for this message
David (dvdkhlng) wrote :

I'm not very experienced with kernel development, so didn't try to create a patch for now. It wasn't even immediately obvious were to send patches to. If nobody else wants to work on the issue I'll try to allocate some time for it. Pretty amazing how the bug got in the kernel in the first place. I mean, even the simplest USB testcase could have caught it.

cheers,

David

Revision history for this message
Aleksandr Koltsoff (czr) wrote :

Currently this stops Altera Quartus II Web Edition from working on Lucid and also the scanner driver for Canon P-150 (proprietary sane-backend with open source .so-shim). Lucky me I have and try to use both.

Also, a lot of packages have "Fix committed" in them in this report, but I can't see the fix anywhere?

Revision history for this message
David (dvdkhlng) wrote :

This is not fixed in Ubuntu 10.10 (i.e. 32-bit apps running on 64-bit kernel get incorrectly truncated isochronous transfers). Re-opened a new bug report:

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/672516

Changed in linux (Ubuntu):
status: Fix Committed → Fix Released
Aurelien Jarno (aurel32)
Changed in qemu:
status: Fix Committed → Invalid
Pojar Geo (geoubuntu)
no longer affects: tvtime
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers