[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LFD.2.00.1002261953450.4513@localhost.localdomain>
Date: Fri, 26 Feb 2010 20:11:00 -0800 (PST)
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Markus Rechberger <mrechberger@...il.com>
cc: werner@...ane.dyn-o-saur.com, Greg KH <greg@...ah.com>,
Marcus Meissner <meissner@...e.de>,
linux-kernel@...r.kernel.org
Subject: Re: 2.6.33 bugs (USBFS, Intel graphic)
On Sat, 27 Feb 2010, Markus Rechberger wrote:
>
> commit d4a4683ca054ed9917dfc9e3ff0f7ecf74ad90d6 upstream.
>
> this patch breaks isochronous USBFS support, please revert that patch!
>
> http://sundtek.de/images/tvtime-bildfehler.jpg
>
> with the patch reverted:
> http://sundtek.de/images/tvtime-working.png
Hmm. That would seem to mean that either the app (tvtime) depended in some
_really_ interesting way on some random data that was never even
transferred from the device, or 'urb->actual_length' isn't actually
reliable in some cases.
Does this patch (_instead_ of reverting things) change any behavior? Do
you get that warning? It will zero-fill the remainder of the buffer.
(UNTESTED! It compiled for me, and looks ok, but whatever..)
Linus
---
drivers/usb/core/devio.c | 53 +++++++++++++++++++++++++++++++++++++++-------
1 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index a678186..8afee02 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1305,6 +1305,47 @@ static int proc_unlinkurb(struct dev_state *ps, void __user *arg)
return 0;
}
+/*
+ * Fixme! We don't have a good memclear_user(), so we do it with a
+ * stupid copy_to_user() loop from a zero buffer.
+ */
+static int memclear_user(void __user *dst, long len)
+{
+ while (len > 0) {
+ static const char zeroes[128];
+ unsigned long n = len;
+
+ if (n > sizeof(zeroes))
+ n = sizeof(zeroes);
+ if (copy_to_user(dst, zeroes, n))
+ return -EFAULT;
+ dst += n;
+ len -= n;
+ }
+ return 0;
+}
+
+static int copy_buffer_to_user(struct async *as, struct urb *urb)
+{
+ void __user *dst = as->userbuffer;
+ void *src;
+ unsigned long len, full;
+
+ if (!dst)
+ return 0;
+
+ len = urb->actual_length;
+ full = urb->transfer_buffer_length;
+ if (WARN_ONCE(len > full, "actual_length (%lu) > transfer_buffer_length (%lu)", len, full))
+ len = full;
+
+ src = urb->transfer_buffer;
+ if (copy_to_user(dst, src, len))
+ return -EFAULT;
+
+ return memclear_user(dst + len, full - len);
+}
+
static int processcompl(struct async *as, void __user * __user *arg)
{
struct urb *urb = as->urb;
@@ -1312,10 +1353,8 @@ 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 (copy_to_user(as->userbuffer, urb->transfer_buffer,
- urb->actual_length))
- goto err_out;
+ if (copy_buffer_to_user(as, urb))
+ goto err_out;
if (put_user(as->status, &userurb->status))
goto err_out;
if (put_user(urb->actual_length, &userurb->actual_length))
@@ -1480,10 +1519,8 @@ 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 (copy_to_user(as->userbuffer, urb->transfer_buffer,
- urb->actual_length))
- return -EFAULT;
+ if (copy_buffer_to_user(as, urb))
+ return -EFAULT;
if (put_user(as->status, &userurb->status))
return -EFAULT;
if (put_user(urb->actual_length, &userurb->actual_length))
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists