Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757277AbdLWDDO (ORCPT ); Fri, 22 Dec 2017 22:03:14 -0500 Received: from h2.hallyn.com ([78.46.35.8]:52968 "EHLO h2.hallyn.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757190AbdLWDDN (ORCPT ); Fri, 22 Dec 2017 22:03:13 -0500 Date: Fri, 22 Dec 2017 21:03:11 -0600 From: "Serge E. Hallyn" To: Dongsu Park Cc: linux-kernel@vger.kernel.org, containers@lists.linux-foundation.org, Alban Crequy , "Eric W . Biederman" , Miklos Szeredi , Seth Forshee , Sargun Dhillon , dm-devel@redhat.com, linux-bcache@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mtd@lists.infradead.org, Alexander Viro , Jan Kara , Serge Hallyn Subject: Re: [PATCH 01/11] block_dev: Support checking inode permissions in lookup_bdev() Message-ID: <20171223030311.GA6837@mail.hallyn.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5724 Lines: 152 On Fri, Dec 22, 2017 at 03:32:25PM +0100, Dongsu Park wrote: > From: Seth Forshee > > When looking up a block device by path no permission check is > done to verify that the user has access to the block device inode > at the specified path. In some cases it may be necessary to > check permissions towards the inode, such as allowing > unprivileged users to mount block devices in user namespaces. > > Add an argument to lookup_bdev() to optionally perform this > permission check. A value of 0 skips the permission check and > behaves the same as before. A non-zero value specifies the mask > of access rights required towards the inode at the specified > path. The check is always skipped if the user has CAP_SYS_ADMIN. > > All callers of lookup_bdev() currently pass a mask of 0, so this > patch results in no functional change. Subsequent patches will > add permission checks where appropriate. > > Patch v4 is available: https://patchwork.kernel.org/patch/8943601/ > > Cc: dm-devel@redhat.com > Cc: linux-bcache@vger.kernel.org > Cc: linux-fsdevel@vger.kernel.org > Cc: linux-mtd@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > Cc: Alexander Viro > Cc: Jan Kara > Cc: Serge Hallyn Acked-by: Serge Hallyn > Signed-off-by: Seth Forshee > Signed-off-by: Dongsu Park > --- > drivers/md/bcache/super.c | 2 +- > drivers/md/dm-table.c | 2 +- > drivers/mtd/mtdsuper.c | 2 +- > fs/block_dev.c | 13 ++++++++++--- > fs/quota/quota.c | 2 +- > include/linux/fs.h | 2 +- > 6 files changed, 15 insertions(+), 8 deletions(-) > > diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c > index b4d28928..acc9d56c 100644 > --- a/drivers/md/bcache/super.c > +++ b/drivers/md/bcache/super.c > @@ -1967,7 +1967,7 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr, > sb); > if (IS_ERR(bdev)) { > if (bdev == ERR_PTR(-EBUSY)) { > - bdev = lookup_bdev(strim(path)); > + bdev = lookup_bdev(strim(path), 0); > mutex_lock(&bch_register_lock); > if (!IS_ERR(bdev) && bch_is_open(bdev)) > err = "device already registered"; > diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c > index 88130b5d..bca5eaf4 100644 > --- a/drivers/md/dm-table.c > +++ b/drivers/md/dm-table.c > @@ -410,7 +410,7 @@ dev_t dm_get_dev_t(const char *path) > dev_t dev; > struct block_device *bdev; > > - bdev = lookup_bdev(path); > + bdev = lookup_bdev(path, 0); > if (IS_ERR(bdev)) > dev = name_to_dev_t(path); > else { > diff --git a/drivers/mtd/mtdsuper.c b/drivers/mtd/mtdsuper.c > index e43fea89..4a4d40c0 100644 > --- a/drivers/mtd/mtdsuper.c > +++ b/drivers/mtd/mtdsuper.c > @@ -180,7 +180,7 @@ struct dentry *mount_mtd(struct file_system_type *fs_type, int flags, > /* try the old way - the hack where we allowed users to mount > * /dev/mtdblock$(n) but didn't actually _use_ the blockdev > */ > - bdev = lookup_bdev(dev_name); > + bdev = lookup_bdev(dev_name, 0); > if (IS_ERR(bdev)) { > ret = PTR_ERR(bdev); > pr_debug("MTDSB: lookup_bdev() returned %d\n", ret); > diff --git a/fs/block_dev.c b/fs/block_dev.c > index 4a181fcb..5ca06095 100644 > --- a/fs/block_dev.c > +++ b/fs/block_dev.c > @@ -1662,7 +1662,7 @@ struct block_device *blkdev_get_by_path(const char *path, fmode_t mode, > struct block_device *bdev; > int err; > > - bdev = lookup_bdev(path); > + bdev = lookup_bdev(path, 0); > if (IS_ERR(bdev)) > return bdev; > > @@ -2052,12 +2052,14 @@ EXPORT_SYMBOL(ioctl_by_bdev); > /** > * lookup_bdev - lookup a struct block_device by name > * @pathname: special file representing the block device > + * @mask: rights to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC) > * > * Get a reference to the blockdevice at @pathname in the current > * namespace if possible and return it. Return ERR_PTR(error) > - * otherwise. > + * otherwise. If @mask is non-zero, check for access rights to the > + * inode at @pathname. > */ > -struct block_device *lookup_bdev(const char *pathname) > +struct block_device *lookup_bdev(const char *pathname, int mask) > { > struct block_device *bdev; > struct inode *inode; > @@ -2072,6 +2074,11 @@ struct block_device *lookup_bdev(const char *pathname) > return ERR_PTR(error); > > inode = d_backing_inode(path.dentry); > + if (mask != 0 && !capable(CAP_SYS_ADMIN)) { > + error = __inode_permission(inode, mask); > + if (error) > + goto fail; > + } > error = -ENOTBLK; > if (!S_ISBLK(inode->i_mode)) > goto fail; > diff --git a/fs/quota/quota.c b/fs/quota/quota.c > index 43612e2a..e5d47955 100644 > --- a/fs/quota/quota.c > +++ b/fs/quota/quota.c > @@ -807,7 +807,7 @@ static struct super_block *quotactl_block(const char __user *special, int cmd) > > if (IS_ERR(tmp)) > return ERR_CAST(tmp); > - bdev = lookup_bdev(tmp->name); > + bdev = lookup_bdev(tmp->name, 0); > putname(tmp); > if (IS_ERR(bdev)) > return ERR_CAST(bdev); > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 2995a271..fce19c49 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2551,7 +2551,7 @@ static inline void unregister_chrdev(unsigned int major, const char *name) > #define BLKDEV_MAJOR_MAX 512 > extern const char *__bdevname(dev_t, char *buffer); > extern const char *bdevname(struct block_device *bdev, char *buffer); > -extern struct block_device *lookup_bdev(const char *); > +extern struct block_device *lookup_bdev(const char *, int mask); > extern void blkdev_show(struct seq_file *,off_t); > > #else > -- > 2.13.6