[RFC PATCH 0/4] Allow user w/o CAP_SYS_ADMIN to submit ioctl commands into DM

[RFC PATCH 0/4] Allow user w/o CAP_SYS_ADMIN to submit ioctl commands into DM

[okozina]

From: Ondrej Kozina <okozina@redhat.com>

Hi,

I would like to post RFC with a set of changes for device-mapper that would
enable user w/o CAP_SYS_ADMIN capability to create and manage his own devices
in more or less distant future. I`ll try to be as brief as possible, so:

Simple use cases:
^^^^^^^^^^^^^^^^
(1) Non-root user would be able to create dm-crypt device from USB stick
    (assuming the user has permission to access backing device)
(2) You can create pool of storage space for virtual machines. VM managers
    would be able to administrate the pool w/o root permissions. Each VM would
    be able to create it's own device (same premise as before)

Short Overview:
^^^^^^^^^^^^^^
(1) The first checkpoint that decides whether the user has an access right to
    submit ioctl cmds into DM driver are actually the attributes on dentry of
    /dev/mapper/control (through general perm., ACL, LSM checks).

(2) After the DM_DEV_CREATE ioctl, new block device will receive FSUID and
    FSGID from the context on behalf of which we entered into kernel

(3) Whenever user asks to manipulate DM device he must pass the test whether
    he's the owner of DM device or not

(4) During the table creation process we check whether the user has right to
    READ/WRITE the backing device, according to table mode (there's possibly
    major drawback when user submits target device as major:minor couple, more
    on that later). For that purpose I added a new function into target_type
    interface - dm_security_fn.

Note: It's still work in progress. I would jusk like to ask you humbly for any
comments, if you may miss a few minutes.

Another note: I'm prety sure that there are many bugs and many mistakes in my
concept (locking, maybe even reference leaks,...) but what I'm looking for
right now, is rather a proof of concept than patchset ready for upstream.

With regards
O.

Ondrej Kozina (4):
  Adds support for user-submitted ioctl commands
  Adds support for the dentry lookup from path or major:minor couple
    during target security check
  Adds support for security checks in the linear target
  Adds support for security checks in the crypt target

 drivers/md/dm-crypt.c         |   37 +++++++++++-
 drivers/md/dm-ioctl.c         |  138 +++++++++++++++++++++++++++++++++++------
 drivers/md/dm-linear.c        |   49 ++++++++++++++-
 drivers/md/dm-sysfs.c         |   30 +++++++++
 drivers/md/dm-table.c         |   16 +++++
 drivers/md/dm-target.c        |  126 +++++++++++++++++++++++++++++++++++++
 drivers/md/dm.c               |   86 +++++++++++++++++++++++++
 drivers/md/dm.h               |   14 ++++
 include/linux/device-mapper.h |    3 +
 9 files changed, 477 insertions(+), 22 deletions(-)

-- 
1.7.8.5

[RFC PATCH 1/4] Adds support for user-submitted ioctl commands

[okozina]

From: Ondrej Kozina <okozina@redhat.com>

The core of changes in DM ioctl operations

---
 drivers/md/dm-ioctl.c         |  138 +++++++++++++++++++++++++++++++++++------
 drivers/md/dm-sysfs.c         |   30 +++++++++
 drivers/md/dm-table.c         |   16 +++++
 drivers/md/dm.c               |   86 +++++++++++++++++++++++++
 drivers/md/dm.h               |   14 ++++
 include/linux/device-mapper.h |    3 +
 6 files changed, 267 insertions(+), 20 deletions(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index a1a3e6d..f3de6fc 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -351,6 +351,7 @@ static char *__change_cell_name(struct hash_cell *hc, char *new_name)
 static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 					    const char *new)
 {
+	int r;
 	char *new_data, *old_name = NULL;
 	struct hash_cell *hc;
 	struct dm_table *table;
@@ -397,6 +398,14 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 		return ERR_PTR(-ENXIO);
 	}
 
+	r = dm_check_dev_permission(hc->md, 1, NULL);
+	if (r) {
+		dm_put(hc->md);
+		up_write(&_hash_lock);
+		kfree(new_data);
+		return ERR_PTR(-EACCES);
+	}
+
 	/*
 	 * Does this device already have a uuid?
 	 */
@@ -434,6 +443,41 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 	return md;
 }
 
+/*
+ * non-root submited ioctl helper function
+ */
+static int initial_permission_check(unsigned int cmd)
+{
+	if (capable(CAP_SYS_ADMIN))
+		return 0;
+
+	/* FIXME: This is only an example */
+	switch(_IOC_NR(cmd)) {
+	case DM_REMOVE_ALL_CMD:
+	case DM_DEV_SET_GEOMETRY_CMD:
+		DMDEBUG("non-root user cant't call this ioctl: 0x%x", cmd);
+		return 1;
+	}
+	return 0;
+}
+
+static int deny_create(void)
+{
+	if (capable(CAP_SYS_ADMIN))
+		return 0;
+
+	/*
+	 * FIXME: Add some function to limit user to create
+	 * only reasonable number of DM_DEVS
+	 *
+	 * sugestions:
+	 * a) only limited number of devices per second
+	 * b) limited number in total per specific user
+	 * c) limited number in total per non-root user
+	 */
+	return 0;
+}
+
 /*-----------------------------------------------------------------
  * Implementation of the ioctl commands
  *---------------------------------------------------------------*/
@@ -492,9 +536,12 @@ static int list_devices(struct dm_ioctl *param, size_t param_size)
 	 */
 	for (i = 0; i < NUM_BUCKETS; i++) {
 		list_for_each_entry (hc, _name_buckets + i, name_list) {
-			needed += sizeof(struct dm_name_list);
-			needed += strlen(hc->name) + 1;
-			needed += ALIGN_MASK;
+			BUG_ON(!hc->md);
+			if (!dm_check_dev_permission(hc->md, 0, NULL)) {
+				needed += sizeof(struct dm_name_list);
+				needed += strlen(hc->name) + 1;
+				needed += ALIGN_MASK;
+			}
 		}
 	}
 
@@ -515,16 +562,19 @@ static int list_devices(struct dm_ioctl *param, size_t param_size)
 	 */
 	for (i = 0; i < NUM_BUCKETS; i++) {
 		list_for_each_entry (hc, _name_buckets + i, name_list) {
-			if (old_nl)
-				old_nl->next = (uint32_t) ((void *) nl -
-							   (void *) old_nl);
-			disk = dm_disk(hc->md);
-			nl->dev = huge_encode_dev(disk_devt(disk));
-			nl->next = 0;
-			strcpy(nl->name, hc->name);
-
-			old_nl = nl;
-			nl = align_ptr(((void *) ++nl) + strlen(hc->name) + 1);
+			BUG_ON(!hc->md);
+			if (!dm_check_dev_permission(hc->md, 0, NULL)) {
+				if (old_nl)
+					old_nl->next = (uint32_t) ((void *) nl -
+							(void *) old_nl);
+				disk = dm_disk(hc->md);
+				nl->dev = huge_encode_dev(disk_devt(disk));
+				nl->next = 0;
+				strcpy(nl->name, hc->name);
+
+				old_nl = nl;
+				nl = align_ptr(((void *) ++nl) + strlen(hc->name) + 1);
+			}
 		}
 	}
 
@@ -704,6 +754,9 @@ static int dev_create(struct dm_ioctl *param, size_t param_size)
 	int r, m = DM_ANY_MINOR;
 	struct mapped_device *md;
 
+	if (deny_create())
+		return -EACCES;
+
 	r = check_name(param->name);
 	if (r)
 		return r;
@@ -808,6 +861,12 @@ static int dev_remove(struct dm_ioctl *param, size_t param_size)
 
 	md = hc->md;
 
+	if (dm_check_dev_permission(md, 1, NULL)) {
+		up_write(&_hash_lock);
+		dm_put(md);
+		return -EACCES;
+	}
+
 	/*
 	 * Ensure the device is not open and nothing further can open it.
 	 */
@@ -930,6 +989,11 @@ static int do_suspend(struct dm_ioctl *param)
 	if (!md)
 		return -ENXIO;
 
+	if (dm_check_dev_permission(md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	if (param->flags & DM_SKIP_LOCKFS_FLAG)
 		suspend_flags &= ~DM_SUSPEND_LOCKFS_FLAG;
 	if (param->flags & DM_NOFLUSH_FLAG)
@@ -968,6 +1032,11 @@ static int do_resume(struct dm_ioctl *param)
 
 	md = hc->md;
 
+	if (dm_check_dev_permission(md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	new_map = hc->new_map;
 	hc->new_map = NULL;
 	param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
@@ -1008,7 +1077,7 @@ static int do_resume(struct dm_ioctl *param)
 
 	if (!r)
 		__dev_status(md, param);
-
+out:
 	dm_put(md);
 	return r;
 }
@@ -1123,6 +1192,11 @@ static int dev_wait(struct dm_ioctl *param, size_t param_size)
 	if (!md)
 		return -ENXIO;
 
+	if (dm_check_dev_permission(md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	/*
 	 * Wait for a notification event
 	 */
@@ -1222,6 +1296,11 @@ static int table_load(struct dm_ioctl *param, size_t param_size)
 	if (!md)
 		return -ENXIO;
 
+	if (dm_check_dev_permission(md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	r = dm_table_create(&t, get_mode(param), param->target_count, md);
 	if (r)
 		goto out;
@@ -1292,6 +1371,7 @@ out:
 
 static int table_clear(struct dm_ioctl *param, size_t param_size)
 {
+	int r = 0;
 	struct hash_cell *hc;
 	struct mapped_device *md;
 
@@ -1304,6 +1384,11 @@ static int table_clear(struct dm_ioctl *param, size_t param_size)
 		return -ENXIO;
 	}
 
+	if (dm_check_dev_permission(hc->md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	if (hc->new_map) {
 		dm_table_destroy(hc->new_map);
 		hc->new_map = NULL;
@@ -1313,10 +1398,12 @@ static int table_clear(struct dm_ioctl *param, size_t param_size)
 
 	__dev_status(hc->md, param);
 	md = hc->md;
+
+out:
 	up_write(&_hash_lock);
 	dm_put(md);
 
-	return 0;
+	return r;
 }
 
 /*
@@ -1387,6 +1474,7 @@ static int table_deps(struct dm_ioctl *param, size_t param_size)
  */
 static int table_status(struct dm_ioctl *param, size_t param_size)
 {
+	int r = 0;
 	struct mapped_device *md;
 	struct dm_table *table;
 
@@ -1394,6 +1482,11 @@ static int table_status(struct dm_ioctl *param, size_t param_size)
 	if (!md)
 		return -ENXIO;
 
+	if (dm_check_dev_permission(md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	__dev_status(md, param);
 
 	table = dm_get_live_or_inactive_table(md, param);
@@ -1402,9 +1495,10 @@ static int table_status(struct dm_ioctl *param, size_t param_size)
 		dm_table_put(table);
 	}
 
+out:
 	dm_put(md);
 
-	return 0;
+	return r;
 }
 
 /*
@@ -1423,6 +1517,11 @@ static int target_message(struct dm_ioctl *param, size_t param_size)
 	if (!md)
 		return -ENXIO;
 
+	if (dm_check_dev_permission(md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	if (tmsg < (struct dm_target_msg *) param->data ||
 	    invalid_str(tmsg->message, (void *) param + param_size)) {
 		DMWARN("Invalid target message parameters.");
@@ -1616,13 +1715,12 @@ static int ctl_ioctl(uint command, struct dm_ioctl __user *user)
 	ioctl_fn fn = NULL;
 	size_t input_param_size;
 
-	/* only root can play with this */
-	if (!capable(CAP_SYS_ADMIN))
-		return -EACCES;
-
 	if (_IOC_TYPE(command) != DM_IOCTL)
 		return -ENOTTY;
 
+	if (initial_permission_check(command))
+		return -EACCES;
+
 	cmd = _IOC_NR(command);
 
 	/*
diff --git a/drivers/md/dm-sysfs.c b/drivers/md/dm-sysfs.c
index 84d2b91..a0bde93 100644
--- a/drivers/md/dm-sysfs.c
+++ b/drivers/md/dm-sysfs.c
@@ -64,14 +64,44 @@ static ssize_t dm_attr_suspended_show(struct mapped_device *md, char *buf)
 	return strlen(buf);
 }
 
+static ssize_t dm_attr_owner_uid_show(struct mapped_device *md, char *buf)
+{
+	struct block_device *bdev = bdget_disk(dm_disk(md), 0);
+
+	if (bdev) {
+		sprintf(buf, "%d\n", bdev->bd_inode->i_uid);
+		bdput(bdev);
+		return strlen(buf);
+	}
+
+	return -EIO;
+}
+
+static ssize_t dm_attr_owner_gid_show(struct mapped_device *md, char *buf)
+{
+	struct block_device *bdev = bdget_disk(dm_disk(md), 0);
+
+	if (bdev) {
+		sprintf(buf, "%d\n", bdev->bd_inode->i_gid);
+		bdput(bdev);
+		return strlen(buf);
+	}
+
+	return -EIO;
+}
+
 static DM_ATTR_RO(name);
 static DM_ATTR_RO(uuid);
 static DM_ATTR_RO(suspended);
+static DM_ATTR_RO(owner_uid);
+static DM_ATTR_RO(owner_gid);
 
 static struct attribute *dm_attrs[] = {
 	&dm_attr_name.attr,
 	&dm_attr_uuid.attr,
 	&dm_attr_suspended.attr,
+	&dm_attr_owner_uid.attr,
+	&dm_attr_owner_gid.attr,
 	NULL,
 };
 
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 2e227fb..1b41e33 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -816,6 +816,20 @@ int dm_table_add_target(struct dm_table *t, const char *type,
 		goto bad;
 	}
 
+	/* THIS IS WIP */
+	DMDEBUG("taget-type: %s, security-check %s defined", tgt->type->name, tgt->type->security ? "" : "not");
+	if (tgt->type->security) {
+		r = tgt->type->security(tgt, argc, argv);
+		if (r)
+			goto sec_bad;
+	} else {
+		if (!capable(CAP_SYS_ADMIN)) {
+			tgt->error = "target does not support security checks!";
+			r = -ENOTTY;
+			goto sec_bad;
+		}
+	}
+
 	r = tgt->type->ctr(tgt, argc, argv);
 	kfree(argv);
 	if (r)
@@ -829,6 +843,8 @@ int dm_table_add_target(struct dm_table *t, const char *type,
 
 	return 0;
 
+ sec_bad:
+	kfree(argv);
  bad:
 	DMERR("%s: %s: %s", dm_device_name(t->md), type, tgt->error);
 	dm_put_target_type(tgt->type);
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index e24143c..db6e547 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -333,6 +333,78 @@ static void __exit dm_exit(void)
 }
 
 /*
+ * okozina: permissions
+ */
+
+/* FIXME: just simple test */
+/*
+ * Supposed to call during device initialization.
+ * Otherwise inode counter should be incremented.
+ */
+static int set_bdev_owner(struct block_device *bdev)
+{
+	int r = 0;
+	struct inode *inode;
+
+	/*
+	 * ATTR_FORCE is compulsory. otherwise perm dennied for
+	 * user w/o CAP_FOWNER.
+	 *
+	 * Note that user that has no right to create dm device
+	 * needs to be stopped before ioct() syscall on control
+	 * nod.
+	 */
+	struct iattr attr = {
+		.ia_valid = ATTR_UID | ATTR_GID | ATTR_FORCE,
+		.ia_uid = current_fsuid(),
+		.ia_gid = current_fsgid()
+	};
+
+	inode = bdev->bd_inode;
+
+	mutex_lock(&inode->i_mutex);
+
+	/* for info only */
+	BUG_ON(!inode->i_sb);
+	BUG_ON(!inode->i_sb->s_root);
+
+	r = inode_change_ok(inode, &attr);
+	if (r)
+		goto out;
+
+	setattr_copy(inode, &attr);
+	mark_inode_dirty(inode);
+
+out:
+	mutex_unlock(&inode->i_mutex);
+	return r;
+}
+
+/*
+ * This is only suggestion how to define ownership
+ * of block device in kernel
+ */
+int dm_check_dev_permission(struct mapped_device *md, int warn, void *attr __attribute__((unused)))
+{
+	int r;
+
+	if (!md)
+		return -EINVAL;
+
+	BUG_ON(!md->bdev);
+	BUG_ON(!md->bdev->bd_inode);
+
+	r = !inode_owner_or_capable(md->bdev->bd_inode);
+
+	if (r && warn)
+		DMWARN("uid: %d, git: %d is not owner (or capable) of the "
+			"device %s", current_uid(), current_gid(),
+			dm_device_name(md));
+
+	return r;
+}
+
+/*
  * Block device functions
  */
 int dm_deleting_md(struct mapped_device *md)
@@ -421,6 +493,11 @@ static int dm_blk_ioctl(struct block_device *bdev, fmode_t mode,
 	if (!map || !dm_table_get_size(map))
 		goto out;
 
+	if (!capable(CAP_SYS_ADMIN)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	/* We only support devices that have a single target */
 	if (dm_table_get_num_targets(map) != 1)
 		goto out;
@@ -1884,6 +1961,15 @@ static struct mapped_device *alloc_dev(int minor)
 	if (!md->bdev)
 		goto bad_bdev;
 
+	/* It's just a test. This should be
+	 * in block layer */
+	if (set_bdev_owner(md->bdev)) {
+		DMDEBUG("set device ownership failed");
+		goto bad_bdev;
+	}
+	DMDEBUG("Device %s has got owner uid: %d, gid %d",
+		md->name, md->bdev->bd_inode->i_uid, md->bdev->bd_inode->i_gid);
+
 	bio_init(&md->flush_bio);
 	md->flush_bio.bi_bdev = md->bdev;
 	md->flush_bio.bi_rw = WRITE_FLUSH;
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index b7dacd5..93d8869 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -15,6 +15,8 @@
 #include <linux/list.h>
 #include <linux/blkdev.h>
 #include <linux/hdreg.h>
+#include <linux/namei.h>
+#include <linux/mount.h>
 
 /*
  * Suspend feature flags
@@ -29,6 +31,8 @@
 #define DM_TYPE_BIO_BASED	1
 #define DM_TYPE_REQUEST_BASED	2
 
+#define DEV_PATH "/dev/"
+
 /*
  * List of devices that a metadevice uses and should open/close.
  */
@@ -156,4 +160,14 @@ void dm_kcopyd_exit(void);
 struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity);
 void dm_free_md_mempools(struct dm_md_mempools *pools);
 
+/*
+ * Check device permission
+ */
+int dm_check_dev_permission(struct mapped_device *md, int warn, void *attr);
+
+/*
+ * Lookup device node according to major:minor or path
+ */
+struct dentry* dm_lookup_bdev_dentry(char *device_param);
+
 #endif
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 98f34b8..a9c1d87 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -95,6 +95,8 @@ typedef int (*dm_iterate_devices_fn) (struct dm_target *ti,
 typedef void (*dm_io_hints_fn) (struct dm_target *ti,
 				struct queue_limits *limits);
 
+typedef int (*dm_security_fn) (struct dm_target *ti, unsigned int argc, char **argv);
+
 /*
  * Returns:
  *    0: The target can handle the next I/O immediately.
@@ -151,6 +153,7 @@ struct target_type {
 	dm_busy_fn busy;
 	dm_iterate_devices_fn iterate_devices;
 	dm_io_hints_fn io_hints;
+	dm_security_fn security;
 
 	/* For internal device-mapper use. */
 	struct list_head list;
-- 
1.7.8.5

[RFC PATCH 2/4] Adds support for the dentry lookup from path or major:minor couple during target security check

[okozina]

From: Ondrej Kozina <okozina@redhat.com>

Translation of the major:minor couple into dentry with inode. AFAIK there's no
proper way to do it because of the fact that you can have posibly many dentries
asociated with one major:minor couple. I lean to propose a patch for the block
layer that would allow to define ownership of block device. Right now I use
inode in block_device structure for that purpose, but it's simple inode w/o
support for extended attributes. Moreover the inode in bdev pseudo-fs is not
accessible from userspace. If we got support for block device ownership, it
would easier to know whether the user is owner or not. I'm aware that this
solution is not very credible. I would most welcome any comments on that
particular problem.

---
 drivers/md/dm-target.c |  126 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 126 insertions(+), 0 deletions(-)

diff --git a/drivers/md/dm-target.c b/drivers/md/dm-target.c
index 8da366c..fb97233 100644
--- a/drivers/md/dm-target.c
+++ b/drivers/md/dm-target.c
@@ -150,5 +150,131 @@ void dm_target_exit(void)
 	dm_unregister_target(&error_target);
 }
 
+static struct block_device* dmt_get_bdev(char *device_param, char **path_string)
+{
+	dev_t dev;
+	struct block_device *bdev;
+	unsigned int major, minor;
+
+	if (sscanf(device_param, "%u:%u", &major, &minor) == 2) {
+		/* Extract the major/minor numbers */
+		dev = MKDEV(major, minor);
+		if (MAJOR(dev) != major || MINOR(dev) != minor)
+			return ERR_PTR(-EOVERFLOW);
+
+		bdev = bdget(dev);
+	} else {
+		bdev = lookup_bdev(device_param);
+		*path_string = device_param;
+	}
+
+	return bdev;
+}
+
+/*
+ * Tries to lookup dentry associated with device id passed in device_param.
+ * In case path is stored in device_param, the lookup is trivial.
+ *
+ * In case major:minor couple is stored in device_param the situation is
+ * much worse. major:minor is unique id for device, but not for a dentry
+ * node linked with block device. Perhaps, we should move security checks against
+ * device into block layer. And there's another problem - bdev inodes for block
+ * devices are not accessible from userspace in any way:(
+ *
+ * @device_param: major:minor couple or path (i.e. /dev/sda)
+ */
+struct dentry* dm_lookup_bdev_dentry(char *device_param)
+{
+	char *path_str = NULL;
+	int r = 0, partno;
+	struct block_device *bdev;
+	struct gendisk *disk = NULL;
+	struct dentry *dentry = NULL;
+	struct inode *inode;
+	struct path path;
+
+	/*
+	 * if _device_param_ contains the path, its reference is stored
+	 * in _path_
+	 */
+	bdev = dmt_get_bdev(device_param, &path_str);
+	if (!bdev)
+		return ERR_PTR(-EINVAL);
+	if (IS_ERR(bdev))
+		return ERR_PTR(PTR_ERR(bdev));
+
+	/*
+	 * This is the ugly part. I presume that the device node for major/minor
+	 * is in /dev/<device_nam>.
+	 */
+	if (!path_str) {
+		BUG_ON(!bdev->bd_dev);
+
+		disk = get_gendisk(bdev->bd_dev, &partno);
+		if (!disk) {
+			r = -EINVAL;
+			goto fail_bdev;
+		}
+
+		BUG_ON(!disk->disk_name);
+
+		path_str = kzalloc(strlen(disk->disk_name) * sizeof(char)
+				   + strlen(DEV_PATH) * sizeof(char),
+				   GFP_KERNEL);
+		if (!path_str) {
+			put_disk(disk);
+			r = -ENOMEM;
+			goto fail_bdev;
+		}
+
+		sprintf(path_str, "%s", DEV_PATH);
+		memcpy(path_str + strlen(DEV_PATH), disk->disk_name,
+					strlen(disk->disk_name));
+		put_disk(disk);
+	}
+	DMDEBUG("lookup_bdev_dentry(): path_str for dentry lookup: %s", path_str);
+
+	r = kern_path(path_str, LOOKUP_FOLLOW, &path);
+	if (r)
+		goto fail_bdev;
+
+	dentry = dget(path.dentry);
+	if (!dentry) {
+		r = -EINVAL;
+		goto fail_path;
+	}
+
+	inode = dentry->d_inode;
+
+	if (!S_ISBLK(inode->i_mode)) {
+		r = -ENOTBLK;
+		goto fail_dentry;
+	}
+
+	/*
+	 * That check is supposed to be elsewhere maybe, but
+	 * it's the esential test!
+	 */
+	if (path.mnt->mnt_flags & MNT_NODEV)
+		r = -EACCES;
+
+fail_dentry:
+	if (r)
+		dput(dentry);
+fail_path:
+	path_put(&path);
+fail_bdev:
+	if (path_str && (device_param != path_str))
+		kfree(path_str);
+
+	bdput(bdev);
+
+	if (r)
+		return ERR_PTR(r);
+	else
+		return dentry;
+}
+
+EXPORT_SYMBOL(dm_lookup_bdev_dentry);
 EXPORT_SYMBOL(dm_register_target);
 EXPORT_SYMBOL(dm_unregister_target);
-- 
1.7.8.5

[RFC PATCH 3/4] Adds support for security checks in the linear target

[okozina]

From: Ondrej Kozina <okozina@redhat.com>

Just a security check implementation for the linear target

---
 drivers/md/dm-linear.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 48 insertions(+), 1 deletions(-)

diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
index 3639eea..68345ad 100644
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -153,9 +153,55 @@ static int linear_iterate_devices(struct dm_target *ti,
 	return fn(ti, lc->dev, lc->start, ti->len, data);
 }
 
+/*
+ * This function checks wheter user has right to create the target with passed
+ * parameters.
+ *
+ * NOTE: Right now it checks access rights against dentry for device node.
+ * 	 There's problem with translation from major:minor to dentry (which one
+ * 	 of possibly many dentries?)
+ *
+ * 	 In the end, it should do checks against block device itself! But
+ * 	 we can't set security context for bdev->bd_inode because bdev
+ * 	 inodes do not support extended attributes.
+ */
+static int linear_security(struct dm_target *ti, unsigned int argc, char **argv)
+{
+	int perm = 0, r;
+	struct dentry *bdev_dentry;
+
+	if (argc < 1) {
+		ti->error = "dm-linear: Security check: invalid number of parameters";
+		return -EINVAL;
+	}
+
+	bdev_dentry = dm_lookup_bdev_dentry(argv[0]);
+
+	if (IS_ERR(bdev_dentry)) {
+		ti->error = "dm-linear: security_check: Couldn't get dentry for bdev";
+		r = PTR_ERR(bdev_dentry);
+		goto err;
+	}
+
+	/* FIXME: MAY_ACCESS, MAY_OPEN ?*/
+	if (dm_table_get_mode(ti->table) & FMODE_READ)
+		perm |= MAY_READ;
+	if (dm_table_get_mode(ti->table) & FMODE_WRITE)
+		perm |= MAY_WRITE;
+
+	r = inode_permission(bdev_dentry->d_inode, perm);
+
+	if (r)
+		ti->error = "dm-linear: Security check failed";
+
+	dput(bdev_dentry);
+err:
+	return r;
+}
+
 static struct target_type linear_target = {
 	.name   = "linear",
-	.version = {1, 1, 0},
+	.version = {1, 2, 0},
 	.module = THIS_MODULE,
 	.ctr    = linear_ctr,
 	.dtr    = linear_dtr,
@@ -164,6 +210,7 @@ static struct target_type linear_target = {
 	.ioctl  = linear_ioctl,
 	.merge  = linear_merge,
 	.iterate_devices = linear_iterate_devices,
+	.security = linear_security
 };
 
 int __init dm_linear_init(void)
-- 
1.7.8.5

[RFC PATCH 4/4] Adds support for security checks in the crypt target

[okozina]

From: Ondrej Kozina <okozina@redhat.com>

Just a security check implementation for the crypt target

---
 drivers/md/dm-crypt.c |   37 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 3f06df5..06e2018 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -28,6 +28,7 @@
 #include <crypto/algapi.h>
 
 #include <linux/device-mapper.h>
+#include "dm.h"
 
 #define DM_MSG_PREFIX "crypt"
 
@@ -1867,9 +1868,42 @@ static int crypt_iterate_devices(struct dm_target *ti,
 	return fn(ti, cc->dev, cc->start, ti->len, data);
 }
 
+static int crypt_security(struct dm_target *ti, unsigned int argc, char **argv)
+{
+	int perm = 0, r;
+	struct dentry *bdev_dentry;
+
+	if (argc < 4) {
+		ti->error = "dm-crypt: Security check: invalid number of parameters";
+		return -EINVAL;
+	}
+	bdev_dentry = dm_lookup_bdev_dentry(argv[3]);
+
+	if (IS_ERR(bdev_dentry)) {
+		ti->error = "dm-crypt: security_check: Couldn't get dentry for bdev";
+		r = PTR_ERR(bdev_dentry);
+		goto err;
+	}
+
+	/* FIXME: MAY_ACCESS, MAY_OPEN ?*/
+	if (dm_table_get_mode(ti->table) & FMODE_READ)
+		perm |= MAY_READ;
+	if (dm_table_get_mode(ti->table) & FMODE_WRITE)
+		perm |= MAY_WRITE;
+
+	r = inode_permission(bdev_dentry->d_inode, perm);
+
+	if (r)
+		ti->error = "dm-crypt: Security check failed";
+
+	dput(bdev_dentry);
+err:
+	return r;
+}
+
 static struct target_type crypt_target = {
 	.name   = "crypt",
-	.version = {1, 11, 0},
+	.version = {1, 12, 0},
 	.module = THIS_MODULE,
 	.ctr    = crypt_ctr,
 	.dtr    = crypt_dtr,
@@ -1881,6 +1915,7 @@ static struct target_type crypt_target = {
 	.message = crypt_message,
 	.merge  = crypt_merge,
 	.iterate_devices = crypt_iterate_devices,
+	.security = crypt_security
 };
 
 static int __init dm_crypt_init(void)
-- 
1.7.8.5

[RFC PATCH v2 0/3] Allow user w/o CAP_SYS_ADMIN to submit ioctl commands into DM

[Ondrej Kozina]

Hi,

I would like to post new version of my previous RFC with a set of changes for
device-mapper that would enable user w/o CAP_SYS_ADMIN capability to create 
and manage his own devices.

Changes in version No. 2:
^^^^^^^^^^^^^^^^^^^^^^^^

(*) No major:minor translation to dentry (inode) in kernel space. Supposing
    userspace should resolve that.

(*) Load time parameter to enable security enhancements

(*) Supposing security is enabled, userspace should handle open file descriptors
    and kernel checks whether the user is authorised to access the device
    linked to the device node.

The rest of message is the same as in original RFC

Simple use cases:
^^^^^^^^^^^^^^^^
(1) Non-root user would be able to create dm-crypt device from USB stick
    (assuming the user has permission to access backing device)
(2) You can create pool of storage space for virtual machines. VM managers
    would be able to administrate the pool w/o root permissions:

    .--------------. .----------------.
    |_owner: VM 01_| |__owner: VM 02__|
           |                |
           |                |
    .---------------------------------.
    |____owner: VM manager____________|

Short Overview:
^^^^^^^^^^^^^^
(1) The first checkpoint that decides whether the user has an access right to
    submit ioctl cmds into DM driver are actually the attributes on dentry of
    /dev/mapper/control (through general perm., ACL, LSM checks).

(2) After the DM_DEV_CREATE ioctl, new block device will receive FSUID and
    from the context on behalf of which we entered into kernel

(3) Whenever user asks to manipulate DM device he must pass the test whether
    he's the owner of DM device or he has CAP_FOWNER capability.

(4) During the table creation process we check whether the user has right to
    READ/WRITE the backing device, according to table mode. For that purpose
    I added a new function into target_type interface - dm_security_fn.

With regards
O.

Ondrej Kozina (3):
  Adds support for user-submitted ioctl commands
  Adds support for security checks in the linear target
  Adds support for security checks in the crypt target

 drivers/md/Makefile           |    2 +-
 drivers/md/dm-crypt.c         |   18 +++++-
 drivers/md/dm-ioctl.c         |  141 +++++++++++++++++++++++++++++++++++------
 drivers/md/dm-linear.c        |   19 +++++-
 drivers/md/dm-security.c      |   95 +++++++++++++++++++++++++++
 drivers/md/dm-sysfs.c         |   15 +++++
 drivers/md/dm-table.c         |   23 +++++++-
 drivers/md/dm.c               |   87 +++++++++++++++++++++++++
 drivers/md/dm.h               |   12 ++++
 include/linux/device-mapper.h |    5 ++
 10 files changed, 393 insertions(+), 24 deletions(-)
 create mode 100644 drivers/md/dm-security.c

-- 
1.7.8.6

[RFC PATCH v2 1/3] Adds support for user-submitted ioctl commands

[Ondrej Kozina]

The core of changes in DM ioctl operations

Changes in version No. 2:
^^^^^^^^^^^^^^^^^^^^^^^^

(*) With security enabled, open file descriptor for backing device has to be 
    passed in target parameters (e.g. "fd:9" instead of "8:2")

(*) Load time parameter for dm-mod "enable_security"

(*) Removed owner_gid from sysfs attributes

---
 drivers/md/Makefile           |    2 +-
 drivers/md/dm-ioctl.c         |  141 +++++++++++++++++++++++++++++++++++------
 drivers/md/dm-security.c      |   95 +++++++++++++++++++++++++++
 drivers/md/dm-sysfs.c         |   15 +++++
 drivers/md/dm-table.c         |   23 +++++++-
 drivers/md/dm.c               |   87 +++++++++++++++++++++++++
 drivers/md/dm.h               |   12 ++++
 include/linux/device-mapper.h |    5 ++
 8 files changed, 358 insertions(+), 22 deletions(-)
 create mode 100644 drivers/md/dm-security.c

diff --git a/drivers/md/Makefile b/drivers/md/Makefile
index 8b2e0df..28d0808 100644
--- a/drivers/md/Makefile
+++ b/drivers/md/Makefile
@@ -3,7 +3,7 @@
 #
 
 dm-mod-y	+= dm.o dm-table.o dm-target.o dm-linear.o dm-stripe.o \
-		   dm-ioctl.o dm-io.o dm-kcopyd.o dm-sysfs.o
+		   dm-ioctl.o dm-io.o dm-kcopyd.o dm-sysfs.o dm-security.o
 dm-multipath-y	+= dm-path-selector.o dm-mpath.o
 dm-snapshot-y	+= dm-snap.o dm-exception-store.o dm-snap-transient.o \
 		    dm-snap-persistent.o
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index a1a3e6d..e94a883 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -351,6 +351,7 @@ static char *__change_cell_name(struct hash_cell *hc, char *new_name)
 static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 					    const char *new)
 {
+	int r;
 	char *new_data, *old_name = NULL;
 	struct hash_cell *hc;
 	struct dm_table *table;
@@ -397,6 +398,14 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 		return ERR_PTR(-ENXIO);
 	}
 
+	r = dm_check_dm_dev_permission(hc->md, 1, NULL);
+	if (r) {
+		dm_put(hc->md);
+		up_write(&_hash_lock);
+		kfree(new_data);
+		return ERR_PTR(-EACCES);
+	}
+
 	/*
 	 * Does this device already have a uuid?
 	 */
@@ -434,6 +443,44 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 	return md;
 }
 
+/*
+ * non-root submited ioctl helper function
+ */
+static int initial_permission_check(unsigned int cmd)
+{
+	if (capable(CAP_SYS_ADMIN))
+		return 0;
+
+	if (!sec_enabled)
+		return 1;
+
+	/* FIXME: This is only an example */
+	switch(_IOC_NR(cmd)) {
+	case DM_REMOVE_ALL_CMD:
+	case DM_DEV_SET_GEOMETRY_CMD:
+		DMDEBUG("non-root user cant't call this ioctl: 0x%x", cmd);
+		return 1;
+	}
+	return 0;
+}
+
+static int deny_create(void)
+{
+	if (capable(CAP_SYS_ADMIN))
+		return 0;
+
+	/*
+	 * FIXME: Add some function to limit user to create
+	 * only reasonable number of DM_DEVS
+	 *
+	 * sugestions:
+	 * a) only limited number of devices per second
+	 * b) limited number in total per specific user
+	 * c) limited number in total per non-root user
+	 */
+	return 0;
+}
+
 /*-----------------------------------------------------------------
  * Implementation of the ioctl commands
  *---------------------------------------------------------------*/
@@ -492,9 +539,12 @@ static int list_devices(struct dm_ioctl *param, size_t param_size)
 	 */
 	for (i = 0; i < NUM_BUCKETS; i++) {
 		list_for_each_entry (hc, _name_buckets + i, name_list) {
-			needed += sizeof(struct dm_name_list);
-			needed += strlen(hc->name) + 1;
-			needed += ALIGN_MASK;
+			BUG_ON(!hc->md);
+			if (!dm_check_dm_dev_permission(hc->md, 0, NULL)) {
+				needed += sizeof(struct dm_name_list);
+				needed += strlen(hc->name) + 1;
+				needed += ALIGN_MASK;
+			}
 		}
 	}
 
@@ -515,16 +565,19 @@ static int list_devices(struct dm_ioctl *param, size_t param_size)
 	 */
 	for (i = 0; i < NUM_BUCKETS; i++) {
 		list_for_each_entry (hc, _name_buckets + i, name_list) {
-			if (old_nl)
-				old_nl->next = (uint32_t) ((void *) nl -
-							   (void *) old_nl);
-			disk = dm_disk(hc->md);
-			nl->dev = huge_encode_dev(disk_devt(disk));
-			nl->next = 0;
-			strcpy(nl->name, hc->name);
-
-			old_nl = nl;
-			nl = align_ptr(((void *) ++nl) + strlen(hc->name) + 1);
+			BUG_ON(!hc->md);
+			if (!dm_check_dm_dev_permission(hc->md, 0, NULL)) {
+				if (old_nl)
+					old_nl->next = (uint32_t) ((void *) nl -
+							(void *) old_nl);
+				disk = dm_disk(hc->md);
+				nl->dev = huge_encode_dev(disk_devt(disk));
+				nl->next = 0;
+				strcpy(nl->name, hc->name);
+
+				old_nl = nl;
+				nl = align_ptr(((void *) ++nl) + strlen(hc->name) + 1);
+			}
 		}
 	}
 
@@ -704,6 +757,9 @@ static int dev_create(struct dm_ioctl *param, size_t param_size)
 	int r, m = DM_ANY_MINOR;
 	struct mapped_device *md;
 
+	if (deny_create())
+		return -EACCES;
+
 	r = check_name(param->name);
 	if (r)
 		return r;
@@ -808,6 +864,12 @@ static int dev_remove(struct dm_ioctl *param, size_t param_size)
 
 	md = hc->md;
 
+	if (dm_check_dm_dev_permission(md, 1, NULL)) {
+		up_write(&_hash_lock);
+		dm_put(md);
+		return -EACCES;
+	}
+
 	/*
 	 * Ensure the device is not open and nothing further can open it.
 	 */
@@ -930,6 +992,11 @@ static int do_suspend(struct dm_ioctl *param)
 	if (!md)
 		return -ENXIO;
 
+	if (dm_check_dm_dev_permission(md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	if (param->flags & DM_SKIP_LOCKFS_FLAG)
 		suspend_flags &= ~DM_SUSPEND_LOCKFS_FLAG;
 	if (param->flags & DM_NOFLUSH_FLAG)
@@ -968,6 +1035,11 @@ static int do_resume(struct dm_ioctl *param)
 
 	md = hc->md;
 
+	if (dm_check_dm_dev_permission(md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	new_map = hc->new_map;
 	hc->new_map = NULL;
 	param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
@@ -1008,7 +1080,7 @@ static int do_resume(struct dm_ioctl *param)
 
 	if (!r)
 		__dev_status(md, param);
-
+out:
 	dm_put(md);
 	return r;
 }
@@ -1123,6 +1195,11 @@ static int dev_wait(struct dm_ioctl *param, size_t param_size)
 	if (!md)
 		return -ENXIO;
 
+	if (dm_check_dm_dev_permission(md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	/*
 	 * Wait for a notification event
 	 */
@@ -1222,6 +1299,11 @@ static int table_load(struct dm_ioctl *param, size_t param_size)
 	if (!md)
 		return -ENXIO;
 
+	if (dm_check_dm_dev_permission(md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	r = dm_table_create(&t, get_mode(param), param->target_count, md);
 	if (r)
 		goto out;
@@ -1292,6 +1374,7 @@ out:
 
 static int table_clear(struct dm_ioctl *param, size_t param_size)
 {
+	int r = 0;
 	struct hash_cell *hc;
 	struct mapped_device *md;
 
@@ -1304,6 +1387,11 @@ static int table_clear(struct dm_ioctl *param, size_t param_size)
 		return -ENXIO;
 	}
 
+	if (dm_check_dm_dev_permission(hc->md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	if (hc->new_map) {
 		dm_table_destroy(hc->new_map);
 		hc->new_map = NULL;
@@ -1313,10 +1401,12 @@ static int table_clear(struct dm_ioctl *param, size_t param_size)
 
 	__dev_status(hc->md, param);
 	md = hc->md;
+
+out:
 	up_write(&_hash_lock);
 	dm_put(md);
 
-	return 0;
+	return r;
 }
 
 /*
@@ -1387,6 +1477,7 @@ static int table_deps(struct dm_ioctl *param, size_t param_size)
  */
 static int table_status(struct dm_ioctl *param, size_t param_size)
 {
+	int r = 0;
 	struct mapped_device *md;
 	struct dm_table *table;
 
@@ -1394,6 +1485,11 @@ static int table_status(struct dm_ioctl *param, size_t param_size)
 	if (!md)
 		return -ENXIO;
 
+	if (dm_check_dm_dev_permission(md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	__dev_status(md, param);
 
 	table = dm_get_live_or_inactive_table(md, param);
@@ -1402,9 +1498,10 @@ static int table_status(struct dm_ioctl *param, size_t param_size)
 		dm_table_put(table);
 	}
 
+out:
 	dm_put(md);
 
-	return 0;
+	return r;
 }
 
 /*
@@ -1423,6 +1520,11 @@ static int target_message(struct dm_ioctl *param, size_t param_size)
 	if (!md)
 		return -ENXIO;
 
+	if (dm_check_dm_dev_permission(md, 1, NULL)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	if (tmsg < (struct dm_target_msg *) param->data ||
 	    invalid_str(tmsg->message, (void *) param + param_size)) {
 		DMWARN("Invalid target message parameters.");
@@ -1616,13 +1718,12 @@ static int ctl_ioctl(uint command, struct dm_ioctl __user *user)
 	ioctl_fn fn = NULL;
 	size_t input_param_size;
 
-	/* only root can play with this */
-	if (!capable(CAP_SYS_ADMIN))
-		return -EACCES;
-
 	if (_IOC_TYPE(command) != DM_IOCTL)
 		return -ENOTTY;
 
+	if (initial_permission_check(command))
+		return -EACCES;
+
 	cmd = _IOC_NR(command);
 
 	/*
diff --git a/drivers/md/dm-security.c b/drivers/md/dm-security.c
new file mode 100644
index 0000000..5ce80cf
--- /dev/null
+++ b/drivers/md/dm-security.c
@@ -0,0 +1,95 @@
+#include "dm.h"
+#include <linux/fs.h>
+#include <linux/file.h>
+#include <linux/mount.h>
+
+static struct file *get_device_file(unsigned int fd)
+{
+	int r;
+	struct file *file;
+
+	file = fget(fd);
+	if (!file)
+		return file;
+
+	r = -ENOTBLK;
+	if (!S_ISBLK(file->f_dentry->d_inode->i_mode))
+		goto err;
+
+	r = -EACCES;
+	if (file->f_vfsmnt->mnt_flags & MNT_NODEV)
+		goto err;
+
+	return file;
+err:
+	fput(file);
+	return ERR_PTR(r);
+}
+
+
+static struct inode *get_bdev_inode(const char *fd_string)
+{
+	char dummy;
+	struct file *dev_file;
+	struct inode *inode;
+	unsigned int fd;
+
+	if (sscanf(fd_string, "fd:%u%c", &fd, &dummy) != 1)
+		return ERR_PTR(-EINVAL);
+
+	dev_file = get_device_file(fd);
+	if (!dev_file)
+		return ERR_PTR(-EINVAL);
+	if (IS_ERR(dev_file))
+		return (void *)dev_file;
+
+	/*
+	 * fd is open by process under it's context
+	 * we're in kernel space
+	 */
+	ihold(dev_file->f_dentry->d_inode);
+
+	inode = dev_file->f_dentry->d_inode;
+	fput(dev_file);
+	return inode;
+}
+
+int dm_check_backing_dev_permission(struct dm_target *ti, const char *fd_string)
+{
+	int perm = 0, r;
+	struct inode *inode;
+
+	inode = get_bdev_inode(fd_string);
+	if (!inode)
+		return -EINVAL;
+	if (IS_ERR(inode))
+		return PTR_ERR(inode);
+
+	if (dm_table_get_mode(ti->table) & FMODE_READ)
+		perm |= MAY_READ;
+	if (dm_table_get_mode(ti->table) & FMODE_WRITE)
+		perm |= MAY_WRITE;
+
+	r = inode_permission(inode, perm);
+
+	iput(inode);
+
+	return r;
+}
+
+/* fget_light would be better */
+int dm_get_device_from_fd(unsigned int fd, dev_t *devid)
+{
+	struct file *file;
+
+	file = get_device_file(fd);
+	if (!file)
+		return -EINVAL;
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	*devid = file->f_dentry->d_inode->i_rdev;
+
+	fput(file);
+	return 0;
+}
diff --git a/drivers/md/dm-sysfs.c b/drivers/md/dm-sysfs.c
index 84d2b91..0efdb21 100644
--- a/drivers/md/dm-sysfs.c
+++ b/drivers/md/dm-sysfs.c
@@ -64,14 +64,29 @@ static ssize_t dm_attr_suspended_show(struct mapped_device *md, char *buf)
 	return strlen(buf);
 }
 
+static ssize_t dm_attr_owner_uid_show(struct mapped_device *md, char *buf)
+{
+	struct block_device *bdev = bdget_disk(dm_disk(md), 0);
+
+	if (bdev) {
+		sprintf(buf, "%d\n", bdev->bd_inode->i_uid);
+		bdput(bdev);
+		return strlen(buf);
+	}
+
+	return -EIO;
+}
+
 static DM_ATTR_RO(name);
 static DM_ATTR_RO(uuid);
 static DM_ATTR_RO(suspended);
+static DM_ATTR_RO(owner_uid);
 
 static struct attribute *dm_attrs[] = {
 	&dm_attr_name.attr,
 	&dm_attr_uuid.attr,
 	&dm_attr_suspended.attr,
+	&dm_attr_owner_uid.attr,
 	NULL,
 };
 
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 2e227fb..f29cb19 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -461,7 +461,7 @@ int dm_get_device(struct dm_target *ti, const char *path, fmode_t mode,
 	int r;
 	dev_t uninitialized_var(dev);
 	struct dm_dev_internal *dd;
-	unsigned int major, minor;
+	unsigned int fd, major, minor;
 	struct dm_table *t = ti->table;
 	char dummy;
 
@@ -472,6 +472,10 @@ int dm_get_device(struct dm_target *ti, const char *path, fmode_t mode,
 		dev = MKDEV(major, minor);
 		if (MAJOR(dev) != major || MINOR(dev) != minor)
 			return -EOVERFLOW;
+	} else if (sscanf(path, "fd:%u%c", &fd, &dummy) == 1) {
+		r = dm_get_device_from_fd(fd, &dev);
+		if (r)
+			return r;
 	} else {
 		/* convert the path to a device */
 		struct block_device *bdev = lookup_bdev(path);
@@ -816,6 +820,21 @@ int dm_table_add_target(struct dm_table *t, const char *type,
 		goto bad;
 	}
 
+	DMDEBUG("taget-type: %s, security-check %s defined", tgt->type->name, tgt->type->security ? "" : "not");
+
+	if (sec_enabled) {
+		if (tgt->type->security) {
+			r = tgt->type->security(tgt, argc, argv);
+			if (r)
+				goto sec_bad;
+		} else {
+			tgt->error = "target does not support security checks!";
+			/* NOTE: think about suitable error number */
+			r = -ENOTTY;
+			goto sec_bad;
+		}
+	}
+
 	r = tgt->type->ctr(tgt, argc, argv);
 	kfree(argv);
 	if (r)
@@ -829,6 +848,8 @@ int dm_table_add_target(struct dm_table *t, const char *type,
 
 	return 0;
 
+ sec_bad:
+	kfree(argv);
  bad:
 	DMERR("%s: %s: %s", dm_device_name(t->md), type, tgt->error);
 	dm_put_target_type(tgt->type);
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index e24143c..a333ea7 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -46,6 +46,8 @@ static const char *_name = DM_NAME;
 static unsigned int major = 0;
 static unsigned int _major = 0;
 
+unsigned short sec_enabled = 0;
+
 static DEFINE_IDR(_minor_idr);
 
 static DEFINE_SPINLOCK(_minor_lock);
@@ -333,6 +335,76 @@ static void __exit dm_exit(void)
 }
 
 /*
+ * okozina: permissions
+ */
+
+/* FIXME: just simple test */
+/*
+ * Supposed to call during device initialization.
+ * Otherwise inode counter should be incremented.
+ */
+static int set_bdev_owner(struct block_device *bdev)
+{
+	int r = 0;
+	struct inode *inode;
+
+	/*
+	 * ATTR_FORCE is compulsory. otherwise perm dennied for
+	 * user w/o CAP_FOWNER.
+	 *
+	 * Note that user that has no right to create dm device
+	 * needs to be stopped before ioct() syscall on control
+	 * nod.
+	 */
+	struct iattr attr = {
+		.ia_valid = ATTR_UID | ATTR_FORCE,
+		.ia_uid = current_fsuid()
+	};
+
+	inode = bdev->bd_inode;
+
+	mutex_lock(&inode->i_mutex);
+
+	/* for info only */
+	BUG_ON(!inode->i_sb);
+	BUG_ON(!inode->i_sb->s_root);
+
+	r = inode_change_ok(inode, &attr);
+	if (r)
+		goto out;
+
+	setattr_copy(inode, &attr);
+	mark_inode_dirty(inode);
+
+out:
+	mutex_unlock(&inode->i_mutex);
+	return r;
+}
+
+/*
+ * This is only suggestion how to define ownership
+ * of block device in kernel
+ */
+int dm_check_dm_dev_permission(struct mapped_device *md, int warn, void *attr __attribute__((unused)))
+{
+	int r;
+
+	if (!md)
+		return -EINVAL;
+
+	BUG_ON(!md->bdev);
+	BUG_ON(!md->bdev->bd_inode);
+
+	r = !inode_owner_or_capable(md->bdev->bd_inode);
+
+	if (r && warn)
+		DMWARN("uid: %d is not owner (or capable) of the "
+			"device %s", current_uid(), dm_device_name(md));
+
+	return r;
+}
+
+/*
  * Block device functions
  */
 int dm_deleting_md(struct mapped_device *md)
@@ -421,6 +493,11 @@ static int dm_blk_ioctl(struct block_device *bdev, fmode_t mode,
 	if (!map || !dm_table_get_size(map))
 		goto out;
 
+	if (!capable(CAP_SYS_ADMIN)) {
+		r = -EACCES;
+		goto out;
+	}
+
 	/* We only support devices that have a single target */
 	if (dm_table_get_num_targets(map) != 1)
 		goto out;
@@ -1884,6 +1961,15 @@ static struct mapped_device *alloc_dev(int minor)
 	if (!md->bdev)
 		goto bad_bdev;
 
+	/* It's just a test. This should be
+	 * in block layer */
+	if (set_bdev_owner(md->bdev)) {
+		DMDEBUG("set device ownership failed");
+		goto bad_bdev;
+	}
+	DMDEBUG("Device %s has got owner uid: %d",
+		md->name, md->bdev->bd_inode->i_uid);
+
 	bio_init(&md->flush_bio);
 	md->flush_bio.bi_bdev = md->bdev;
 	md->flush_bio.bi_rw = WRITE_FLUSH;
@@ -2774,6 +2860,7 @@ module_init(dm_init);
 module_exit(dm_exit);
 
 module_param(major, uint, 0);
+module_param_named(enable_security, sec_enabled, ushort, 0444);
 MODULE_PARM_DESC(major, "The major number of the device mapper");
 MODULE_DESCRIPTION(DM_NAME " driver");
 MODULE_AUTHOR("Joe Thornber <dm-devel@redhat.com>");
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index b7dacd5..82e98bd 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -156,4 +156,16 @@ void dm_kcopyd_exit(void);
 struct dm_md_mempools *dm_alloc_md_mempools(unsigned type, unsigned integrity);
 void dm_free_md_mempools(struct dm_md_mempools *pools);
 
+/*
+ * Check device permission
+ */
+int dm_check_dm_dev_permission(struct mapped_device *md, int warn, void *attr);
+
+/*
+ *  Security related functions
+ */
+
+int dm_get_device_from_fd(unsigned int fd, dev_t *devid);
+int dm_check_backing_dev_permission(struct dm_target *ti, const char *fd_string);
+
 #endif
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 98f34b8..7ddca63 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -12,6 +12,8 @@
 #include <linux/blkdev.h>
 #include <linux/ratelimit.h>
 
+extern unsigned short sec_enabled;
+
 struct dm_dev;
 struct dm_target;
 struct dm_table;
@@ -95,6 +97,8 @@ typedef int (*dm_iterate_devices_fn) (struct dm_target *ti,
 typedef void (*dm_io_hints_fn) (struct dm_target *ti,
 				struct queue_limits *limits);
 
+typedef int (*dm_security_fn) (struct dm_target *ti, unsigned int argc, char **argv);
+
 /*
  * Returns:
  *    0: The target can handle the next I/O immediately.
@@ -151,6 +155,7 @@ struct target_type {
 	dm_busy_fn busy;
 	dm_iterate_devices_fn iterate_devices;
 	dm_io_hints_fn io_hints;
+	dm_security_fn security;
 
 	/* For internal device-mapper use. */
 	struct list_head list;
-- 
1.7.8.6

[RFC PATCH v2 2/3] Adds support for security checks in the linear target

[Ondrej Kozina]

Just a security check implementation for linear target

---
 drivers/md/dm-linear.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
index 3639eea..b04aa00 100644
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -153,9 +153,25 @@ static int linear_iterate_devices(struct dm_target *ti,
 	return fn(ti, lc->dev, lc->start, ti->len, data);
 }
 
+static int linear_security(struct dm_target *ti, unsigned int argc, char **argv)
+{
+	int r;
+
+	if (argc < 1) {
+		ti->error = "dm-linear: Security check: invalid number of parameters";
+		return -EINVAL;
+	}
+
+	r = dm_check_backing_dev_permission(ti, argv[0]);
+	if (r)
+		ti->error = "dm-linear: Security check failed";
+
+	return r;
+}
+
 static struct target_type linear_target = {
 	.name   = "linear",
-	.version = {1, 1, 0},
+	.version = {1, 2, 0},
 	.module = THIS_MODULE,
 	.ctr    = linear_ctr,
 	.dtr    = linear_dtr,
@@ -164,6 +180,7 @@ static struct target_type linear_target = {
 	.ioctl  = linear_ioctl,
 	.merge  = linear_merge,
 	.iterate_devices = linear_iterate_devices,
+	.security = linear_security
 };
 
 int __init dm_linear_init(void)
-- 
1.7.8.6

[RFC PATCH v2 3/3] Adds support for security checks in the crypt target

[Ondrej Kozina]

Just a security check implementation for the crypt target

---
 drivers/md/dm-crypt.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 3f06df5..af6f319 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1867,9 +1867,24 @@ static int crypt_iterate_devices(struct dm_target *ti,
 	return fn(ti, cc->dev, cc->start, ti->len, data);
 }
 
+static int crypt_security(struct dm_target *ti, unsigned int argc, char **argv)
+{
+	int r;
+
+	if (argc < 4) {
+		ti->error = "dm-crypt: Security check: invalid number of parameters";
+		return -EINVAL;
+	}
+	r = dm_check_backing_dev_permission(ti, argv[3]);
+	if (r)
+		ti->error = "dm-crypt: Security check failed";
+
+	return r;
+}
+
 static struct target_type crypt_target = {
 	.name   = "crypt",
-	.version = {1, 11, 0},
+	.version = {1, 12, 0},
 	.module = THIS_MODULE,
 	.ctr    = crypt_ctr,
 	.dtr    = crypt_dtr,
@@ -1881,6 +1896,7 @@ static struct target_type crypt_target = {
 	.message = crypt_message,
 	.merge  = crypt_merge,
 	.iterate_devices = crypt_iterate_devices,
+	.security = crypt_security
 };
 
 static int __init dm_crypt_init(void)
-- 
1.7.8.6