Commit a15092bd authored by Andrew Morton's avatar Andrew Morton Committed by Linus Torvalds

[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().
parent 5524f26f
......@@ -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;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment