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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 3 Feb 2008, Matthew Dharm wrote:

> But, the modifications to usb_stor_access_xfer_buf() look good -- no
> request from a sub-driver should be allowed to scribble into memory.  The
> current code does make the implicit assumption that there is enough
> storage, and will walk right off the end of the sg list if there isn't.
> 
> I'm not sure I like the mods to usb_stor_set_xfer_buf().  Any place we set
> a status that we know is going to be thrown away is an invitation for a
> problem later if someone changes the code to preserve that status.  It's a
> jack-in-the-box, waiting to spring open in our face later.  The limit check
> (which mirrors the usb_stor_access_xfer_buf modification) and WARN_ON() are
> probably good.
> 
> In a strictly technical sense, the change to protocol.c are sufficient.
> That is, they will prevent a serious error.  There is a justification tho
> to fix all of the users of usb_stor_access_buf() to not attempt to use more
> SCSI buffer than exists.
> 
> My opinion is this:  Let's make the protocol.c mods (modulo my comments
> about setting useless status bits) now.  Then, let's decide if we're going
> to patch all the other users of the usb_stor_*_xfer_buf() functions as a
> separate discussion.

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 to try and transfer as 
much as they want, knowing that an overrun can't occur.  There won't 
be any need for a WARN_ON or anything else.

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 transfer has reached the end of 
the buffer.  So I'll have to go through and audit all the callers.

Alan Stern

-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux