Re: [PATCH] SCSI: fix the return type of the remove() method in sgiwd93.c

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Wed, 2008-12-03 at 22:28 +0100, Kay Sievers wrote:
> On Wed, Dec 3, 2008 at 22:02, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Wed, 2008-12-03 at 21:59 +0100, Kay Sievers wrote:
> >> On Wed, Dec 3, 2008 at 21:52, James Bottomley
> >> <James.Bottomley@hansenpartnership.com> wrote:
> >> > On Wed, 2008-12-03 at 21:29 +0100, Kay Sievers wrote:
> >> >> On Wed, Dec 3, 2008 at 20:00, James Bottomley
> >> >> <James.Bottomley@hansenpartnership.com> wrote:
> >> >> >> We are already in the middle of a ~400 files "struct device" bus_id
> >> >> >> conversion, and only very few maintainers respond to these patches. We
> >> >> >> also never got any reply to the SCSI bus_id patch we sent weeks ago.
> >> >> >> :)
> >> >> >
> >> >> > When did you send it?  Searching the scsi archives on bus_id produces no
> >> >> > results, what was the subject line?
> >> >>
> >> >> http://git.kernel.org/?p=linux/kernel/git/gregkh/patches.git;a=blob;f=driver-core/bus_id-scsi.patch;hb=HEAD
> >> >
> >> > Hmm, OK ... if you want a review, over the SCSI list is best.
> >> >
> >> > Things like this:
> >> >
> >> >
> >> >> --- a/drivers/scsi/scsi_ioctl.c
> >> >> 182 +++ b/drivers/scsi/scsi_ioctl.c
> >> >> 183 @@ -170,7 +170,8 @@ static int scsi_ioctl_get_pci(struct scs
> >> >> 184
> >> >> 185          if (!dev)
> >> >> 186                 return -ENXIO;
> >> >> 187
> >> >> -        return copy_to_user(arg, dev->bus_id, sizeof(dev->bus_id))? -EFAULT: 0;
> >> >> 188 +        return copy_to_user(arg,
> >> >> 189
> >> >> +                           dev_name(dev), strlen(dev_name(dev)))? -EFAULT: 0;
> >> >> 190  }
> >> >
> >> > Give cause for concern:  in the original, we know we scribble over 20
> >> > bytes of user space.  With the new one we scribble over an unknown
> >> > number (which could potentially be much greater than 20).  That's an
> >> > accident waiting to happen in userspace.
> >>
> >> Yeah, but the name will have no real limit. What should we do here?
> >> Just Truncate at 20, because we "know" it's not longer?
> >
> > Well, the problem is the stupid ioctl which gives nowhere to say how
> > many bytes the buffer is.  For safety's sake, yes, I think you have to
> > limit it to 20 bytes.  Otherwise, on the day we introduce long names
> > some random application using this ioctl will die with data corruption
> > and that will be extremely hard to debug.
> 
> Do you want to take over the patch to the scsi tree, and we will work
> from there. It's through Greg's tree in -next since a while.

Sure ... can you send a copy of it rather than me having to pull it out
of your git tree.

Thanks,

James