[sane-devel] Problem with libusb and 64 bits 2.6.25 kernel

[Dennis Lou]

The way I see it, Sam's buffer overflow concern is predicated on a misbehaving device.  I haven't witnessed it but it is possible given that we're reverse engineering things rather than working from a formal spec.  Nicolas' buffer overflow concern is predicated on a misbehaving usb stack.  It's also possible, but probably less likely than Sam's concern.  Which to implement depends on how paranoid you are about the behavior of the device and libusb.

-Dennis

----- Original Message ----
From: Sam Varshavchik <mrsam at courier-mta.com>
To: Nicolas <nicolas.martin at freesurf.fr>
Cc: Dennis Lou <dlou99 at yahoo.com>; mrsam-guest <mrsam-guest at alioth.debian.org>; sane-devel <sane-devel at lists.alioth.debian.org>
Sent: Tuesday, June 24, 2008 3:21:08 PM
Subject: Re: [sane-devel] Problem with libusb and 64 bits 2.6.25 kernel

Nicolas writes:

> Ok, then I propose to commit this patch to CVS, but I agree, libusb64
> behavior is here pretty wierd...
> 
> Concerning potential buffer overflow risk in this function, raised by
> Sam, could be possible to add a couple of lines in here, to reject too
> many bytes read:
> 
>       error = pixma_read (s->io, data, chunksize);
>       if (error < 0)
>         return count;
>       count += error;
>       data += error;
> +      chunksize = error;
> +      if (chunksize > size)
> +        return PIXMA_EIO;
>       size -= error;
>     }
>   return count;
> }
> 
> Sam, worth a try with these lines ? 
> (I attached the patch from CVS to this post)

I don't think this is necessary any more, because with my patch this 
function no longer asks for more bytes than it expects. By changing the 
requested chunk size not to exceed the expected # of bytes, you'll no longer 
need to deal with this possibility.

Additionally, I do agree that it's rather bizarre that my problem only 
happens on x86_64. On i386, the same version of libusb works perfectly. 
After some further pondering, it seems more likely that the difference lies 
deeper, in the kernel. My 64 bit motherboard, of course, has different 
onboard USB hardware than my 32 bit laptop, so the real answer probably lies 
with the driver differences in the kernel. That's probably what the real 
answer is, but that's mostly educational now. I've done all kinds of scans, 
at various resolutions, with my patch applied, and it's been flawless every 
time. Just one more confirmation that MF-4270 is working "good".





> 
> Nicolas
> 
> 
> So unless I'm wrong, 
> 
> Le lundi 23 juin 2008 à 21:53 -0700, Dennis Lou a écrit :
>> Right now my build system is acting a bit screwy.  I started working 
>> on another USB device and now xsane isn't working, so I think I messed 
>> something up and am not currently set up to do sane development at the moment.
>> 
>> In any case, I took a look at the patch.  Basically, it's a one-liner that 
>> changes the minimum block size from 512 to the remainder.  The reason I chose a 
>> 512 byte minimum block size is that the Windows driver does it that way, so I imitated.  
>> If the patch works in 64bit libusb, it looks to me that it would likely work in 32bit as well.  
>> However, it may take me a few days to rebuild my sane development environment and confirm.
>> 
>> My recollection of the USB spec is a bit hazy, but 64bit libusb behavior doesn't seem correct to me.
>> 
>> -Dennis
>> 
>> ----- Original Message ----
>> From: Nicolas <nicolas.martin at freesurf.fr>
>> To: Dennis Lou <dlou99 at yahoo.com>
>> Cc: mrsam-guest at alioth.debian.org; sane-devel <sane-devel at lists.alioth.debian.org>
>> Sent: Saturday, June 21, 2008 9:07:56 AM
>> Subject: Re: [sane-devel] Problem with libusb and 64 bits 2.6.25 kernel
>> 
>> Hi Dennis, 
>> 
>> A bug was opened a while back by Sam Varshavchik, concerning the pixma
>> backend for Canon ImageClass MF-4270, when compiled and used on a 64
>> bits platform (no issue so far on 32 bits), details are given here:
>> 
>> https://alioth.debian.org/tracker/?group_id=30186&atid=410366&func=detail&aid=310861
>> 
>> We shared with Sam some info to try to locate the origin, which looked
>> to be related to the 64 bits libusb. 
>> 
>> Sam has performed investigations since then, his conclusions spot on
>> that 32 and 64 bits libusb behave slightly differently when reading data
>> from USB, more precisely, the 64 bits libusb seems to expect an exact
>> count of bytes to read (but not 32 bits libusb), and fails (timeout) if
>> such count is not satisfied.
>> 
>> Sam, in his last post on the bug report page, proposes a patch, which
>> looks to me fine for both 32 and 64 bits, and impacts only the
>> ImageClass part of the backend. 
>> 
>> Dennis, If you don't mind, could you also have a look and give a try, so
>> that we can commit into CVS.
>> 
>> Nicolas
>> 
>> 
>>