Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751377Ab0B0F0Y (ORCPT ); Sat, 27 Feb 2010 00:26:24 -0500 Received: from kroah.org ([198.145.64.141]:42389 "EHLO coco.kroah.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751265Ab0B0F0X (ORCPT ); Sat, 27 Feb 2010 00:26:23 -0500 Date: Fri, 26 Feb 2010 21:26:48 -0800 From: Greg KH To: Markus Rechberger Cc: Linus Torvalds , linux-usb@vger.kernel.org, werner@guyane.dyn-o-saur.com, Marcus Meissner , linux-kernel@vger.kernel.org Subject: Re: 2.6.33 bugs (USBFS, Intel graphic) Message-ID: <20100227052648.GA31418@kroah.com> References: <20100227035639.GA11680@kroah.com> <20100227041815.GA12956@kroah.com> <20100227051737.GA14976@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100227051737.GA14976@kroah.com> User-Agent: Mutt/1.5.17 (2007-11-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3369 Lines: 93 On Fri, Feb 26, 2010 at 09:17:37PM -0800, Greg KH wrote: > On Sat, Feb 27, 2010 at 05:34:27AM +0100, Markus Rechberger wrote: > > On Sat, Feb 27, 2010 at 5:29 AM, Linus Torvalds > > wrote: > > > > > > > > > On Fri, 26 Feb 2010, Greg KH wrote: > > >> > > >> Yes, and that patch didn't touch the iso frames. ?That happens later on > > >> in the functions that were modified. ?The patch should not have had any > > >> affect on iso transfers. ?Unless I'm missing something? > > > > > > Hmm. What seems to happen is that for an isochronous transfer, the buffer > > > is split for each microframe. No? > > > > > > > exactly. and each microframe has its own buffer length identifier. > > > > the current behaviour breaks VMware, QEMU and virtualbox .. probably > > other things too. > > > > > > > So the total length may be in 'urb->actual_length', but the actual data in > > > the buffer may not be contiguous, because it's created from multiple > > > smaller frames, some of which might not be full length? > > > > > > > yes, it's only contiguous for BULK. > > > > > I dunno. That would explain the problem - actual_length is correct, but > > > the 'copy_to_user()' still doesn't copy all the data, because it's > > > fragmented. > > > > > > > no you got it, but your patch does not work. The best way would be to > > revert it if someone wants to speed up BULK it should go down another > > path, leaving the old working implementation untouched. > > Hm, so it's back to the original idea of just doing a kzalloc of the > initial buffer, that should solve the problem that Marcus found. > > I'll go dig that back up and if you could test it, that would be most > appreciated. Here, can you try this on top of everything? thanks, greg k-h diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index a678186..252d3b4 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -1168,7 +1168,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, return -ENOMEM; } if (uurb->buffer_length > 0) { - as->urb->transfer_buffer = kmalloc(uurb->buffer_length, + as->urb->transfer_buffer = kzalloc(uurb->buffer_length, GFP_KERNEL); if (!as->urb->transfer_buffer) { kfree(isopkt); @@ -1312,9 +1312,9 @@ static int processcompl(struct async *as, void __user * __user *arg) void __user *addr = as->userurb; unsigned int i; - if (as->userbuffer && urb->actual_length) + if (as->userbuffer) if (copy_to_user(as->userbuffer, urb->transfer_buffer, - urb->actual_length)) + urb->transfer_buffer_length)) goto err_out; if (put_user(as->status, &userurb->status)) goto err_out; @@ -1480,9 +1480,9 @@ static int processcompl_compat(struct async *as, void __user * __user *arg) void __user *addr = as->userurb; unsigned int i; - if (as->userbuffer && urb->actual_length) + if (as->userbuffer) if (copy_to_user(as->userbuffer, urb->transfer_buffer, - urb->actual_length)) + urb->transfer_buffer_length)) return -EFAULT; if (put_user(as->status, &userurb->status)) return -EFAULT; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/