Al -
Thank you for your feedback. I believe that most of your concerns are
addressed with this set of patches.
On Tue, May 17, 2005 at 05:49:22PM +0100, Al Viro wrote:
> On Tue, May 17, 2005 at 05:09:00PM +0100, Christoph Hellwig wrote:
> > On Tue, May 17, 2005 at 10:25:46AM -0500, Michael Halcrow wrote:
> > > +/**
> > > + * Claim the blockdev to exclude mounters; release on file close.
> > > + */
> > > +static int seclvl_bd_claim(struct file *filp)
> > > {
> > > - int holder;
> > > struct block_device *bdev = NULL;
> > > - dev_t dev = inode->i_rdev;
> > > + dev_t dev = filp->f_dentry->d_inode->i_rdev;
> > > bdev = open_by_devnum(dev, FMODE_WRITE);
> > > if (bdev) {
> > > - if (bd_claim(bdev, &holder)) {
> > > + if (bd_claim(bdev, filp)) {
> > > blkdev_put(bdev);
> > > return -EPERM;
> > > }
> > > - /* claimed, mark it to release on close */
> > > - inode->i_security = current;
> > > + /* Claimed; mark it to release on close */
> > > + filp->f_security = filp;
> > > }
> > > return 0;
> >
> > While we're at it this code is crap before and after your patch.
> > There's absolutely no point at all to use open_by_devnum if you
> > already have an inode or file that you can get the struct
> > block_device from easily.
>
> It's worse than you think. No, they do *not* necessary have
> block_device there. Guess what happens if some clown calls
> e.g. utime("/dev/sda", NULL)? That's right, we go checking if we
> have write permissions on the file in question.
In my tests, utime() does not cause file_permission() to be called.
> Use of current in setting/checking ->i_security is a bad joke.
The patch fixes this:
- /* claimed, mark it to release on close */
- inode->i_security = current;
+ /* Claimed; mark it to release on close */
+ filp->f_security = filp
...
> d) cargo-cult programming: ->f_dentry and ->f_dentry->d_inode are
> *not* NULL, TYVM.
Exactly what code are you refering to here?
> While we are at it... Guys, you do realize that registering an
> object and then deciding to bail out of module_init requires
> unregistering it?
Thanks; this is fixed in the next round of patches.
Mike
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
