From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Bottomley Subject: Re: [PATCH] SCSI: fix the return type of the remove() method in sgiwd93.c Date: Wed, 03 Dec 2008 15:33:45 -0600 Message-ID: <1228340025.5551.87.camel@localhost.localdomain> References: <1227140357-29921-1-git-send-email-dmitri.vorobiev@movial.fi> <1228327306.5551.36.camel@localhost.localdomain> <35647.88.114.226.209.1228329736.squirrel@webmail.movial.fi> <1228330800.5551.58.camel@localhost.localdomain> <1228337529.5551.72.camel@localhost.localdomain> <1228338142.5551.77.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Return-path: Received: from accolon.hansenpartnership.com ([76.243.235.52]:34392 "EHLO accolon.hansenpartnership.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754803AbYLCVdq (ORCPT ); Wed, 3 Dec 2008 16:33:46 -0500 In-Reply-To: Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: Kay Sievers Cc: Vorobiev Dmitri , linux-scsi@vger.kernel.org, linux-mips@linux-mips.org, Greg KH On Wed, 2008-12-03 at 22:28 +0100, Kay Sievers wrote: > On Wed, Dec 3, 2008 at 22:02, James Bottomley > wrote: > > On Wed, 2008-12-03 at 21:59 +0100, Kay Sievers wrote: > >> On Wed, Dec 3, 2008 at 21:52, James Bottomley > >> wrote: > >> > On Wed, 2008-12-03 at 21:29 +0100, Kay Sievers wrote: > >> >> On Wed, Dec 3, 2008 at 20:00, James Bottomley > >> >> 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