[PATCH] raw.c refcounting fix

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().

diff --git a/drivers/char/raw.c b/drivers/char/raw.c
index 352ac13a0d42c82b57929754d6f89c17f7770c03..d6c4d7ca8a38afd9e03d9301ea006e9e91e616b2 100644 (file)
--- a/drivers/char/raw.c
+++ b/drivers/char/raw.c
@@ -57,29 +57,31 @@ static int raw_open(struct inode *inode, struct file *filp)
         */
        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;