Bug#334961: kernel-image: kernel BUG at return value of cciss_ioctl()

[MAENO Masaki <maeno.masaki@nttcom.co.jp>]

On Fri, 21 Oct 2005 18:53:03 +0900
"Simon Horman [Horms]" <horms@debian.org> wrote:
> On Fri, Oct 21, 2005 at 06:43:14PM +0900, MAENO Masaki wrote:
> > On Fri, 21 Oct 2005 16:06:23 +0900
> > "Simon Horman [Horms]" <horms@verge.net.au> wrote:
> > > 
> > > On Fri, Oct 21, 2005 at 03:39:38PM +0900, MAENO Masaki wrote:
> > > > Package: kernel-image
> > > > Version: 2.6.8-2
> > > > Severity: normal
> > > > 
> > > > "fsync_bdev()" cannot be executed in issuing "ioctl(BLKFLSBUF)" to disk drive using cciss driver.
> > > >   (When return value of "ioctl(BLKFLSBUF)" is only "-EINVAL", "fsync_bdev()" is executed.
> > > >    But "fsync_bdev()" isn't executed bacause its value is "-EBADRQC".)
> > > > 
> > > > I suggest that you correct source as follows:
> > > >   drivers/block/cciss.c:1093
> > > >     - return -EBADRQC;
> > > >     + return -EINVAL;
> > > 
> > > I took a look at the upstream tree, and it seems that the return 
> > > value is now -ENOTTY. Do you think that return value is correct?
> > 
> > I know that the thing to return -EINVAL is an old specification.
> > I think the preferable value is -ENOTTY, but influence on other
> > parts is large.
> > I confirmed that it works good by fix above-mentioned in my
> > environment, tentatively...
> 
> Ok, so you would recommend -EINVAL for 2.6.8?

No, I recommend following patch (in my last mail):
  drivers/block/ioctl.c:197
    -if (ret != -EINVAL)
    +if (ret != -EINVAL && ret != -EBADRQC)
  drivers/block/cciss.c:1093 
     no change

I confirmed that it also works good by fix above-mentioned in my
 another environment, tentatively...


> > > Also, as 2.6.8 is now in the deep-freeze as the kernel for sarge,
> > > can you comment on if this patch is critical enough to warrant inclusion
> > > in a sarge update?
> > 
> > You are correct. So, I suggest that it isn't influence other parts
> > easily to correct as follows(return errno is no change bacause of
> > user application):
> >   drivers/block/ioctl.c:197
> >     -if (ret != -EINVAL)
> >     +if (ret != -EINVAL && ret != -EBADRQC)
> > 
> > I tried to verify whether this patch was safe about the part where -EBADRQC
> > is used by ioctl(BLKFLSBUF).
> > 
> > ======
> > * filename and linenum using BLKFLSBUF searched by grep:
> > drivers/mtd/mtd_blkdevs.c, line 206 -- case BLKFLSBUF:
> >   - no return -EBADRQC.
> > drivers/block/ioctl.c, line 192 -- case BLKFLSBUF:
> >   - patch part.
> > drivers/block/nbd.c, line 111 -- case BLKFLSBUF: return "flush-buffer-cache";
> >   - no return -EBADRQC.
> > drivers/block/rd.c, line 306 -- if (cmd != BLKFLSBUF)
> >   - no return -EBADRQC (-EBUSY only).
> > include/linux/fs.h, line 190 -- #define BLKFLSBUF _IO(0x12,97)
> >   - no problem.
> > include/linux/compat_ioctl.h, line 100 -- COMPATIBLE_IOCTL(BLKFLSBUF)
> >   - no problem.
> > init/do_mounts_initrd.c, line 96 -- error = sys_ioctl(fd, BLKFLSBUF, 0);
> >   - no problem.
> > 
> > == Reference
> > * filename and linenum using EBADRQC searched by grep:
> > drivers/block/cciss.c, line 1093 -- return -EBADRQC;
> > drivers/scsi/ch.c, line 174 -- .errno = EBADRQC,
> > drivers/message/fusion/mptctl.c, line 903 -- return -EBADRQC;
> > fs/afs/vlclient.c, line 74 -- case AFSVL_BADVOLOPER: err = -EBADRQC; break;
> > fs/afs/vlocation.c, line 812 -- case -EBADRQC:
> > fs/cifs/netmisc.c, line 94 -- {ERRsmbcmd, -EBADRQC},
> > fs/ncpfs/ioctl.c, line 116 -- return -EBADRQC;
> > fs/ncpfs/ioctl.c, line 132 -- return -EBADRQC;
> > net/bluetooth/lib.c, line 95 -- return EBADRQC;
> > ======
> > 
> > I think OK, please point it out to me if there is a problem.
> 
> I think that looks fine, though I will have to check the code.

Thanks. Check it, please.

> Do you think this bug is imporatnt enough for inclusion
> in a sarge update?

I hope so.

> Could you describe what breaks?

I cannot judge it.
But, I will look after within the range that I can do if the problem
occurs.

Thanks.

-- 
MAENO, Masaki <maeno.masaki@nttcom.co.jp>