Re: [linux-usb-devel] Re: [PATCH 2.6.0] USB pusblp.c

[Paulo Marques]

Greg KH wrote:

On Tue, Dec 30, 2003 at 07:10:51PM +0000, Paulo Marques wrote:

Greg KH wrote:


Hm, this patch is linewrapped or something, and can't be applied.  Can
you try to send it again so that it doesn't get mangled?

Sorry about that.

I'm sending it attached this time to avoid the mess :)

Odd, it still doesn't apply to 2.6.1.  The tabs look ok, and I don't see
any linewrap.  Care to re-diff it again?
Sorry about this.
I'm sorry for replying so late, but i've missed this mail and just noticed it 
now, because it was taking so long for the patch to make it to the kernel (maybe 
it was a busy week) :(

I think the problem was that Oliver Neukum was sending patches at about the same 
time to fix a different problem, so our patches must have colided...

Anyway, here's the patch again this time against 2.6.3-rc1.

The line that IMHO triggers the bug is this:

"writecount += usblp->writeurb->transfer_buffer_length;"

It uses "usblp->writeurb->transfer_buffer_length" before initializing it, 
assuming that it will be zero on the first run. If it is not zero, but instead 
random *negative* garbage from memory, the loop will start printing endless data 
from user-space data.

I hope this time everything works OK :)

--
Paulo Marques
Software Development Department - Restinfor, Lda.
Phone: +351 252 290600, Fax: +351 252 290601
Web: www.grupopie.com
"In a world without walls and fences who needs windows and gates?"
--- drivers/usb/class/usblp.c.orig      2004-02-09 14:46:27.000000000 +0000
+++ drivers/usb/class/usblp.c   2004-02-09 15:03:32.281551096 +0000
@@ -603,7 +603,7 @@ static ssize_t usblp_write(struct file *
 {
        DECLARE_WAITQUEUE(wait, current);
        struct usblp *usblp = file->private_data;
-       int timeout, err = 0;
+       int timeout, err = 0, transfer_length;
        size_t writecount = 0;
 
        while (writecount < count) {
@@ -654,19 +654,13 @@ static ssize_t usblp_write(struct file *
                        continue;
                }
 
-               writecount += usblp->writeurb->transfer_buffer_length;
-               usblp->writeurb->transfer_buffer_length = 0;
+               transfer_length=(count - writecount);
+               if (transfer_length > USBLP_BUF_SIZE)
+                       transfer_length = USBLP_BUF_SIZE;
 
-               if (writecount == count) {
-                       up (&usblp->sem);
-                       break;
-               }
+               usblp->writeurb->transfer_buffer_length = transfer_length;
 
-               usblp->writeurb->transfer_buffer_length = (count - writecount) < 
USBLP_BUF_SIZE ?
-                                                         (count - writecount) : 
USBLP_BUF_SIZE;
-
-               if (copy_from_user(usblp->writeurb->transfer_buffer, buffer + 
writecount,
-                               usblp->writeurb->transfer_buffer_length)) {
+               if (copy_from_user(usblp->writeurb->transfer_buffer, buffer + 
writecount, transfer_length)) {
                        up(&usblp->sem);
                        return writecount ? writecount : -EFAULT;
                }
@@ -683,6 +677,8 @@ static ssize_t usblp_write(struct file *
                        break;
                }
                up (&usblp->sem);
+
+               writecount += transfer_length;
        }
 
        return count;