2009-03-27 10:31:53

by Dan Carpenter

[permalink] [raw]
Subject: [patch] char/raw.c locking on error

There is an unlock_kernel missing.

This bug was found by smatch (http://repo.or.cz/w/smatch.git/). Compile
tested only, sorry.

regards,
dan carpenter

Signed-off-by: Dan Carpenter <[email protected]>

--- orig/drivers/char/raw.c 2009-03-26 22:40:27.000000000 +0300
+++ devel/drivers/char/raw.c 2009-03-26 22:41:20.000000000 +0300
@@ -90,6 +90,7 @@
blkdev_put(bdev, filp->f_mode);
out:
mutex_unlock(&raw_mutex);
+ unlock_kernel();
return err;
}


2009-03-27 15:48:26

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [patch] char/raw.c locking on error

On Fri, 27 Mar 2009 13:28:48 +0300 (EAT)
Dan Carpenter <[email protected]> wrote:

> This bug was found by smatch (http://repo.or.cz/w/smatch.git/). Compile
> tested only, sorry.

But clearly correct. This is probably my fault, duh. I've grabbed the
fix into the bkl-removal tree in case it doesn't get in via some faster
path.

(I bet raw doesn't need BKL at all; I'll see if I can't find some time
to have a look later on).

Thanks,

jon

2009-03-27 21:36:23

by Michael Tokarev

[permalink] [raw]
Subject: Re: [patch] char/raw.c locking on error

Jonathan Corbet wrote:
> (I bet raw doesn't need BKL at all; I'll see if I can't find some time
> to have a look later on).

Is it still in use - the raw driver, that is? Nowadays,
every block device can be opened in direct mode by using
O_DIRECT - this is what raw does. I mean, is it worth
spending even more time on it?

/mjt

2009-03-27 21:44:20

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [patch] char/raw.c locking on error

On Sat, 28 Mar 2009 00:36:08 +0300
Michael Tokarev <[email protected]> wrote:

> Is it still in use - the raw driver, that is? Nowadays,
> every block device can be opened in direct mode by using
> O_DIRECT - this is what raw does. I mean, is it worth
> spending even more time on it?

This was discussed a while back. My understanding is that there are
people using it. It is a user-space ABI of sorts which can't just be
yanked arbitrarily. The right thing to do is probably to replace it
with a shell implementation that just opens the device using O_DIRECT,
but nobody has actually done that...

jon

2009-03-27 23:54:06

by Bodo Eggert

[permalink] [raw]
Subject: Re: [patch] char/raw.c locking on error

Jonathan Corbet <[email protected]> wrote:
> Michael Tokarev <[email protected]> wrote:

>> Is it still in use - the raw driver, that is? Nowadays,
>> every block device can be opened in direct mode by using
>> O_DIRECT - this is what raw does. I mean, is it worth
>> spending even more time on it?

As long as you don't fix all the legacy applications to use O_DIRECT.

> This was discussed a while back. My understanding is that there are
> people using it. It is a user-space ABI of sorts which can't just be
> yanked arbitrarily. The right thing to do is probably to replace it
> with a shell implementation that just opens the device using O_DIRECT,
> but nobody has actually done that...

It had long been done when before it was discussed the last time.

2009-03-28 00:23:16

by Alan

[permalink] [raw]
Subject: Re: [patch] char/raw.c locking on error

On Sat, 28 Mar 2009 00:36:08 +0300
Michael Tokarev <[email protected]> wrote:

> Jonathan Corbet wrote:
> > (I bet raw doesn't need BKL at all; I'll see if I can't find some time
> > to have a look later on).
>
> Is it still in use - the raw driver, that is? Nowadays,
> every block device can be opened in direct mode by using
> O_DIRECT - this is what raw does. I mean, is it worth
> spending even more time on it?

Certainly when I worked for a vendor there were some big proprietary
applications using the raw interface both because it was an established
API and also because it mirrorred various Unixen and relatives they also
ran the apps on with the same basic code.

Attempts to kill it were met with pretty forceful objections.