Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-02-08 Thread Alan Stern
On Tue, 5 Feb 2008, Matthew Dharm wrote: We both agree that the code shouldn't run off the end of the s-g list. Incidentally, if people want a simple bugfix patch for 2.6.24.stable, this should do it. Mark, can you confirm that this patch alone fixes the problem? Alan Stern Index:

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-02-08 Thread Mark Glines
On Fri, 8 Feb 2008 11:46:13 -0500 (EST) Alan Stern [EMAIL PROTECTED] wrote: On Tue, 5 Feb 2008, Matthew Dharm wrote: We both agree that the code shouldn't run off the end of the s-g list. Incidentally, if people want a simple bugfix patch for 2.6.24.stable, this should do it. Mark,

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-02-07 Thread Alan Stern
On Wed, 6 Feb 2008, James Bottomley wrote: Did you mean scsi_kmap_atomic_sg()? Yes .. I replied from memory rather than looking in the source. That appears to do only part of what usb_stor_access_xfer_buf() does. In fact, all it does is map a single page. Um, it does everything

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-02-06 Thread Alan Stern
On Tue, 5 Feb 2008, Matthew Dharm wrote: Six of one and a half-dozen of the other. All we're arguing over is the definition of correct behavior here. You want to change the API so that overrun is acceptable and handled; I prefer calling it a Bad Thing(tm). We both agree that the code

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-02-06 Thread James Bottomley
On Wed, 2008-02-06 at 17:18 -0500, Alan Stern wrote: On Wed, 6 Feb 2008, Matthew Dharm wrote: Maybe this is a crazy question, but... Why is this not in the SCSI core? Or even in the block core? It's hardly USB-specific, and I'm willing to bet that there are other HCDs (at

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-02-06 Thread Alan Stern
On Wed, 6 Feb 2008, James Bottomley wrote: What we're talking about is a routine that provides drivers a simple way to access the data in a scatter-gather buffer (which may lie in highmem or otherwise not be easily reachable). The idea is that some commands are emulated by the driver

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-02-06 Thread James Bottomley
On Wed, 2008-02-06 at 18:25 -0500, Alan Stern wrote: On Wed, 6 Feb 2008, James Bottomley wrote: What we're talking about is a routine that provides drivers a simple way to access the data in a scatter-gather buffer (which may lie in highmem or otherwise not be easily reachable). The

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-02-05 Thread Alan Stern
On Tue, 5 Feb 2008, Boaz Harrosh wrote: However the interface to usb_stor_access_xfer_buf() will have to change slightly. Right now if it sees that *sgptr is NULL, it assumes this means it should start at the beginning of the s-g buffer. But with Boaz's change, *sgptr == NULL means the

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-02-05 Thread Boaz Harrosh
On Tue, Feb 05 2008 at 17:42 +0200, Alan Stern [EMAIL PROTECTED] wrote: On Tue, 5 Feb 2008, Boaz Harrosh wrote: However the interface to usb_stor_access_xfer_buf() will have to change slightly. Right now if it sees that *sgptr is NULL, it assumes this means it should start at the beginning

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-02-05 Thread Matthew Dharm
On Mon, Feb 04, 2008 at 03:05:58PM -0500, Alan Stern wrote: On Sun, 3 Feb 2008, Matthew Dharm wrote: I think the correct approach is to modify those routines so that they will never overrun the s-g buffer (like Boaz has done), and _document_ this behavior. Then the callers can feel free

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-02-03 Thread Boaz Harrosh
On Thu, Jan 31 2008 at 22:56 +0200, Alan Stern [EMAIL PROTECTED] wrote: On Thu, 31 Jan 2008, Boaz Harrosh wrote: The code in usb_stor_access_xfer_buf() will now correctly attempt to transfer according to buflen and what ever is available at the passed sg's. Now this can be less or it can

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-02-03 Thread Matthew Dharm
One more comment on the patch... On Sun, Feb 03, 2008 at 06:28:48PM +0200, Boaz Harrosh wrote: + /* Check for overflow */ + if (buflen scsi_bufflen(srb)) { This really should have an unlikely() around it. This is an often-executed code path, and we want to optimize it as much as

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-01-31 Thread Greg KH
On Thu, Jan 31, 2008 at 07:19:57PM +0200, Boaz Harrosh wrote: scsi_scan is issuing a 36-byte INQUIRY request to llds. isd200 would volunteer 96 bytes of INQUIRY. This caused an underflow condition in protocol.c usb_stor_access_xfer_buf(). So first fix is to usb_stor_access_xfer_buf()

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-01-31 Thread Boaz Harrosh
On Thu, Jan 31 2008 at 20:00 +0200, Greg KH [EMAIL PROTECTED] wrote: On Thu, Jan 31, 2008 at 07:19:57PM +0200, Boaz Harrosh wrote: scsi_scan is issuing a 36-byte INQUIRY request to llds. isd200 would volunteer 96 bytes of INQUIRY. This caused an underflow condition in protocol.c

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-01-31 Thread Boaz Harrosh
On Thu, Jan 31 2008 at 21:34 +0200, Alan Stern [EMAIL PROTECTED] wrote: On Thu, 31 Jan 2008, Boaz Harrosh wrote: On Thu, Jan 31 2008 at 19:49 +0200, Alan Stern [EMAIL PROTECTED] wrote: On Thu, 31 Jan 2008, Boaz Harrosh wrote: @@ -228,9 +228,14 @@ void usb_stor_set_xfer_buf(unsigned char

Re: [PATCH] bugfix for an underflow condition in usb storage isd200.c

2008-01-31 Thread Alan Stern
On Thu, 31 Jan 2008, Boaz Harrosh wrote: The code in usb_stor_access_xfer_buf() will now correctly attempt to transfer according to buflen and what ever is available at the passed sg's. Now this can be less or it can be more. SCSI standard defines this as underflow/overflow. When