From: viro@parcelfarce.linux.theplanet.co.uk
raw.c has a refcounting bug (patch attached).
As for the theory... If you have a pathname - use filp_open() or
open_bdev_excl() and be done with that. bdget() et.al. are OK only if you
really have nothing better than device number and that's a situation that
should be avoided unless you really have no choice.
Said that, we have the following primitives:
* lookup_bdev(): takes a pathname, returns a reference to block_device.
* bdget(): takes a number, returns a reference to block_device.
* blkdev_get(): takes a reference to block_device and opens it. If open
fails - drops the reference to block_device passed to it.
* blkdev_put(): takes a reference to block_device and closes it. The
reference is dropped.
* bdput(): drops a reference to block_device.
Note that behaviour of blkdev_get() and blkdev_put() is such that it makes
for minimal cleanup code.
bd_claim()/bd_release() is the exclusion mechanism - that's what mount,
swapon, open with O_EXCL, etc. are using to avoid stepping on each others
toes. bd_claim() claims bdev for given owner; if it's already owned and
not by the same owner you'll get -EBUSY. bd_release() reverts the effect
of bd_claim(). Note that if you claim the thing N times (with the same
owner, obviously), you'll need N bd_release() before it stops being owned.
raw.c grabbed a reference to bdev only after blkdev_get(). If blkdev_get()
failed (e.g. media being absent), you've got an unbalanced bdput().
---
drivers/char/raw.c | 46 ++++++++++++++++++++++++----------------------
1 files changed, 24 insertions(+), 22 deletions(-)
diff -puN drivers/char/raw.c~raw-driver-refcounting-fix drivers/char/raw.c
--- 25/drivers/char/raw.c~raw-driver-refcounting-fix 2004-01-11 22:06:16.000000000 -0800
+++ 25-akpm/drivers/char/raw.c 2004-01-11 22:06:16.000000000 -0800
@@ -57,29 +57,31 @@ static int raw_open(struct inode *inode,
*/
bdev = raw_devices[minor].binding;
err = -ENODEV;
- if (bdev) {
- err = blkdev_get(bdev, filp->f_mode, 0, BDEV_RAW);
- if (err)
- goto out;
- igrab(bdev->bd_inode);
- err = bd_claim(bdev, raw_open);
- if (err) {
- blkdev_put(bdev, BDEV_RAW);
- goto out;
- }
- err = set_blocksize(bdev, bdev_hardsect_size(bdev));
- if (err) {
- bd_release(bdev);
- blkdev_put(bdev, BDEV_RAW);
- goto out;
- }
- filp->f_flags |= O_DIRECT;
- filp->f_mapping = bdev->bd_inode->i_mapping;
- if (++raw_devices[minor].inuse == 1)
- filp->f_dentry->d_inode->i_mapping =
- bdev->bd_inode->i_mapping;
- }
+ if (!bdev)
+ goto out;
+ igrab(bdev->bd_inode);
+ err = blkdev_get(bdev, filp->f_mode, 0, BDEV_RAW);
+ if (err)
+ goto out;
+ err = bd_claim(bdev, raw_open);
+ if (err)
+ goto out1;
+ err = set_blocksize(bdev, bdev_hardsect_size(bdev));
+ if (err)
+ goto out2;
+ filp->f_flags |= O_DIRECT;
+ filp->f_mapping = bdev->bd_inode->i_mapping;
+ if (++raw_devices[minor].inuse == 1)
+ filp->f_dentry->d_inode->i_mapping =
+ bdev->bd_inode->i_mapping;
filp->private_data = bdev;
+ up(&raw_mutex);
+ return 0;
+
+out2:
+ bd_release(bdev);
+out1:
+ blkdev_put(bdev, BDEV_RAW);
out:
up(&raw_mutex);
return err;
_