gmane.linux.kernel.device-mapper.devel

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

Ondrej Kozina — 2012-05-25T13:22:27

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
< at >< at > -153,9 +153,25 < at >< at > 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,
< at >< at > -164,6 +180,7 < at >< at > 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)

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

Ondrej Kozina — 2012-05-25T13:22:26

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
< at >< at > -3,7 +3,7 < at >< at >
 #
 
 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
< at >< at > -351,6 +351,7 < at >< at > 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;
< at >< at > -397,6 +398,14 < at >< at > 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?
  */
< at >< at > -434,6 +443,44 < at >< at > 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
  *---------------------------------------------------------------*/
< at >< at > -492,9 +539,12 < at >< at > 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;
+}
 }
 }
 
< at >< at > -515,16 +565,19 < at >< at > 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);
+}
 }
 }
 
< at >< at > -704,6 +757,9 < at >< at > 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;
< at >< at > -808,6 +864,12 < at >< at > 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.
  */
< at >< at > -930,6 +992,11 < at >< at > 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)
< at >< at > -968,6 +1035,11 < at >< at > 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;
< at >< at > -1008,7 +1080,7 < at >< at > static int do_resume(struct dm_ioctl *param)
 
 if (!r)
 __dev_status(md, param);
-
+out:
 dm_put(md);
 return r;
 }
< at >< at > -1123,6 +1195,11 < at >< at > 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
  */
< at >< at > -1222,6 +1299,11 < at >< at > 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;
< at >< at > -1292,6 +1374,7 < at >< at > out:
 
 static int table_clear(struct dm_ioctl *param, size_t param_size)
 {
+int r = 0;
 struct hash_cell *hc;
 struct mapped_device *md;
 
< at >< at > -1304,6 +1387,11 < at >< at > 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;
< at >< at > -1313,10 +1401,12 < at >< at > 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;
 }
 
 /*
< at >< at > -1387,6 +1477,7 < at >< at > 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;
 
< at >< at > -1394,6 +1485,11 < at >< at > 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);
< at >< at > -1402,9 +1498,10 < at >< at > static int table_status(struct dm_ioctl *param, size_t param_size)
 dm_table_put(table);
 }
 
+out:
 dm_put(md);
 
-return 0;
+return r;
 }
 
 /*
< at >< at > -1423,6 +1520,11 < at >< at > 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.");
< at >< at > -1616,13 +1718,12 < at >< at > 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
< at >< at > -0,0 +1,95 < at >< at >
+#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
< at >< at > -64,14 +64,29 < at >< at > 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
< at >< at > -461,7 +461,7 < at >< at > 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;
 
< at >< at > -472,6 +472,10 < at >< at > 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);
< at >< at > -816,6 +820,21 < at >< at > 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)
< at >< at > -829,6 +848,8 < at >< at > 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
< at >< at > -46,6 +46,8 < at >< at > 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);
< at >< at > -333,6 +335,76 < at >< at > 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)
< at >< at > -421,6 +493,11 < at >< at > 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;
< at >< at > -1884,6 +1961,15 < at >< at > 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;
< at >< at > -2774,6 +2860,7 < at >< at > 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< at >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
< at >< at > -156,4 +156,16 < at >< at > 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
< at >< at > -12,6 +12,8 < at >< at >
 #include <linux/blkdev.h>
 #include <linux/ratelimit.h>
 
+extern unsigned short sec_enabled;
+
 struct dm_dev;
 struct dm_target;
 struct dm_table;
< at >< at > -95,6 +97,8 < at >< at > 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.
< at >< at > -151,6 +155,7 < at >< at > 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;

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

Ondrej Kozina — 2012-05-25T13:22:25

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

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

Ondrej Kozina — 2012-05-25T13:22:28

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
< at >< at > -1867,9 +1867,24 < at >< at > 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,
< at >< at > -1881,6 +1896,7 < at >< at > 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)

Re: [PATCH 0/2] multipath: compile cleanup

Christophe Varoqui — 2012-05-25T05:20:53

Merged.

Thanks.

[PATCH 0/2] multipath: compile cleanup

Benjamin Marzinski — 2012-05-25T04:57:41

When the multipath code is compiled with the standard redhat rpm cflags,
there are a number of warnings that get issued, some about real problems.

These patches switch multipath to use those cflags, and then clean up
warnings. If people want upstream to continue using the existing cflags,
I don't particularly mind.  I really included the first patch to make sure
people can see the warnings that the second patch corrects.

Also, some of the warnings are for actual errors, and how to fix them is
obvious.  Others won't actually cause problems, and if people object to
the changes I made to make them go away, I'm happy to fix them differently. 

Benjamin Marzinski (2):
  multipath: Build with standard rpm cflags
  multipath: Fix warnings from stricter compile options.

 Makefile.inc                     |    6 +++++-
 libmpathpersist/mpath_persist.c  |   10 +++++++---
 libmpathpersist/mpath_pr_ioctl.c |    9 +++++++++
 libmultipath/alias.c             |    8 ++++++--
 mpathpersist/main.c              |    2 --
 multipathd/main.c                |   24 +++++++++++++++++++-----
 6 files changed, 46 insertions(+), 13 deletions(-)

[PATCH 2/2] multipath: Fix warnings from strictercompile options.

Benjamin Marzinski — 2012-05-25T04:57:43

With stricter compilation options, multipath printed number of
warnings during compilation. Some of them were actual bugs. Others
couldn't cause any problems.  This patch cleans up all the new
warnings.

Signed-off-by: Benjamin Marzinski <bmarzins< at >redhat.com>
---
 libmpathpersist/mpath_persist.c  |   10 +++++++---
 libmpathpersist/mpath_pr_ioctl.c |    9 +++++++++
 libmultipath/alias.c             |    8 ++++++--
 mpathpersist/main.c              |    2 --
 multipathd/main.c                |   24 +++++++++++++++++++-----
 5 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index d99c0da..3041089 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
< at >< at > -177,8 +177,10 < at >< at > int mpath_persistent_reserve_in (int fd, int rq_servact, struct prin_resp *resp,
 goto out;
 }
 
-if (path_discovery(pathvec, conf, DI_SYSFS | DI_CHECKER))
+if (path_discovery(pathvec, conf, DI_SYSFS | DI_CHECKER)) {
+ret = MPATH_PR_DMMP_ERROR;
 goto out1;
+}
 
 /* get info of all paths from the dm device*/
 if (get_mpvec (curmp, pathvec, alias)){
< at >< at > -265,8 +267,10 < at >< at > int mpath_persistent_reserve_out ( int fd, int rq_servact, int rq_scope,
                 goto out;
         }
 
-if (path_discovery(pathvec, conf, DI_SYSFS | DI_CHECKER))
+if (path_discovery(pathvec, conf, DI_SYSFS | DI_CHECKER)) {
+ret = MPATH_PR_DMMP_ERROR;
 goto out1;
+}
 
 /* get info of all paths from the dm device     */
 if (get_mpvec(curmp, pathvec, alias)){
< at >< at > -408,7 +412,7 < at >< at > int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope,
 int rc;
 int count=0;
 int status = MPATH_PR_SUCCESS;
-uint64_t sa_key;
+uint64_t sa_key = 0;
 
 if (!mpp)
 return MPATH_PR_DMMP_ERROR; 
diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c
index 2d4d968..de3292e 100644
--- a/libmpathpersist/mpath_pr_ioctl.c
+++ b/libmpathpersist/mpath_pr_ioctl.c
< at >< at > -314,6 +314,11 < at >< at > int prin_do_scsi_ioctl(char * dev, int rq_servact, struct prin_resp * resp, int
 else
 mx_resp_len = get_prin_length(rq_servact);
 
+if (mx_resp_len == 0) {
+status = MPATH_PR_SYNTAX_ERROR;
+goto out;
+}
+
 cdb[1] = (unsigned char)(rq_servact & 0x1f);
 cdb[7] = (unsigned char)((mx_resp_len >> 8) & 0xff);
 cdb[8] = (unsigned char)(mx_resp_len & 0xff);
< at >< at > -569,6 +574,10 < at >< at > int get_prin_length(int rq_servact)
                 case MPATH_PRIN_RFSTAT_SA:
                         mx_resp_len = sizeof(struct print_fulldescr_list) + sizeof(struct prin_fulldescr *)*32;
                         break;
+default:
+condlog(0, "invalid service action, %d", rq_servact);
+mx_resp_len = 0;
+break;
         }
         return mx_resp_len;
 }
diff --git a/libmultipath/alias.c b/libmultipath/alias.c
index 4159ec6..ec3a225 100644
--- a/libmultipath/alias.c
+++ b/libmultipath/alias.c
< at >< at > -165,7 +165,9 < at >< at > open_bindings_file(char *file, int *can_write)
 "Cannot write header to bindings file : %s",
 strerror(errno));
 /* cleanup partially written header */
-ftruncate(fd, 0);
+if (ftruncate(fd, 0))
+condlog(0, "Cannot truncate the header : %s",
+strerror(errno));
 goto fail;
 }
 fsync(fd);
< at >< at > -337,7 +339,9 < at >< at > allocate_binding(int fd, char *wwid, int id, char *prefix)
 condlog(0, "Cannot write binding to bindings file : %s",
 strerror(errno));
 /* clear partial write */
-ftruncate(fd, offset);
+if (ftruncate(fd, offset))
+condlog(0, "Cannot truncate the header : %s",
+strerror(errno));
 return NULL;
 }
 c = strchr(buf, ' ');
diff --git a/mpathpersist/main.c b/mpathpersist/main.c
index 33dad90..465fcb1 100644
--- a/mpathpersist/main.c
+++ b/mpathpersist/main.c
< at >< at > -736,7 +736,6 < at >< at > mpath_print_transport_id(struct prin_fulldescr *fdesc)
 int
 construct_transportid(const char * lcp, struct transportid transid[], int num_transportids)
 {
-unsigned char * tidp;
 int k = 0;
 int j, n, b, c, len, alen;
 const char * ecp;
< at >< at > -792,7 +791,6 < at >< at > construct_transportid(const char * lcp, struct transportid transid[], int num_tr
 if (ecp && (isip > ecp))
 isip = NULL;
 len = ecp ? (ecp - lcp) : (int)strlen(lcp);
-memset(&tidp, 0, 24);
 transid[num_transportids].format_code = (isip ? MPATH_WWUI_PORT_IDENTIFIER:MPATH_WWUI_DEVICE_NAME);
 transid[num_transportids].protocol_id = MPATH_PROTOCOL_ID_ISCSI;
 alen = len + 1; /* at least one trailing null */
diff --git a/multipathd/main.c b/multipathd/main.c
index 69cef3c..5dea995 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
< at >< at > -664,7 +664,7 < at >< at > fail:
 static int
 uev_update_path (struct uevent *uev, struct vectors * vecs)
 {
-int retval, ro;
+int ro, retval = 0;
 
 ro = uevent_get_disk_ro(uev);
 
< at >< at > -1755,11 +1755,23 < at >< at > daemonize(void)
 }
 
 close(STDIN_FILENO);
-dup(dev_null_fd);
+if (dup(dev_null_fd) < 0) {
+fprintf(stderr, "cannot dup /dev/null to stdin : %s\n",
+strerror(errno));
+_exit(0);
+}
 close(STDOUT_FILENO);
-dup(dev_null_fd);
+if (dup(dev_null_fd) < 0) {
+fprintf(stderr, "cannot dup /dev/null to stdout : %s\n",
+strerror(errno));
+_exit(0);
+}
 close(STDERR_FILENO);
-dup(dev_null_fd);
+if (dup(dev_null_fd) < 0) {
+fprintf(stderr, "cannot dup /dev/null to stderr : %s\n",
+strerror(errno));
+_exit(0);
+}
 close(dev_null_fd);
 daemon_pid = getpid();
 return 0;
< at >< at > -1783,7 +1795,9 < at >< at > main (int argc, char *argv[])
 }
 
 /* make sure we don't lock any path */
-chdir("/");
+if (chdir("/") < 0)
+fprintf(stderr, "can't chdir to root directory : %s\n",
+strerror(errno));
 umask(umask(077) | 022);
 
 conf = alloc_config();

[PATCH 1/2] multipath: Build with standard rpm cflags

Benjamin Marzinski — 2012-05-25T04:57:42

This patch makes multipath build with the standard redhat rpm cflags, which
can help catch some code errors.

Signed-off-by: Benjamin Marzinski <bmarzins< at >redhat.com>
---
 Makefile.inc |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/Makefile.inc b/Makefile.inc
index 02aef4f..b0c68f4 100644
--- a/Makefile.inc
+++ b/Makefile.inc
< at >< at > -38,7 +38,11 < at >< at > mpathpersistdir = $(TOPDIR)/libmpathpersist
 GZIP        = /bin/gzip -9 -c
 INSTALL_PROGRAM = install
 
-OPTFLAGS     = -pipe -g -Wall -Wunused -Wstrict-prototypes
+ifndef RPM_OPT_FLAGS
+RPM_OPT_FLAGS = -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4
+endif
+
+OPTFLAGS     = $(RPM_OPT_FLAGS) -Wunused -Wstrict-prototypes
 CFLAGS     = $(OPTFLAGS) -fPIC -DLIB_STRING=\"${LIB}\"
 SHARED_FLAGS = -shared

Re: [PATCH v2 08/14] block: Kill bi_destructor

Vivek Goyal — 2012-05-24T19:58:10

Just noticed bio_kmalloc_destructor() will take care of freeing bio_kmalloc()
bios and it has been dropped in this patch. So memory leak point is moot.
bio_reset() question still remains though.

Thanks
Vivek

Re: [PATCH v2 07/14] pktcdvd: Switch to bio_kmalloc()

Kent Overstreet — 2012-05-24T19:55:49

bio_kmalloc() sets up bi_destructor, so there's no need to kfree() the
bio - bio_put() frees it just like normal.

bcache performance test problem

Qinling — 2012-05-24T01:14:01

Hi Kent,

I have tested bcache on my platform, but the result is not good. I think maybe the wrong config caused it. Would you pls help me to check the config, or give me some advice for bcache performance testing?

My SSD card is  a PCIe SSD card (about 120,000 IOPS), and I used 10GB.

I have download the code from git://evilpiepirate.org/~kent/linux-bcache.git.
The commands creating bcache is below:
make-bcache -B /dev/sdb8
make-bcache -C -w4k -b1M -writeback /dev/hioa5

I use FIO to test the seq./random IO performance, and the result is below:
write             read                    randwrite    randread
bcache               10809 KB/s    164502.5 KB/s     2713 KB/s    29640 KB/s
cache_hit_rate  80                  79                         94                92


And the basic test result is:
Write                   read                  randwrite             randread
HDD      81211.6 KB/s       172242 KB/s     2606 KB/s             1855 KB/s
SSD       250948.2 KB/s     813855 KB/s     238892.7 KB/s      657319.5 KB/s

[PATCH v2 04/14] block: Add bio_clone_kmalloc()

Kent Overstreet — 2012-05-24T00:02:41

Signed-off-by: Kent Overstreet <koverstreet< at >google.com>
---
 drivers/block/osdblk.c |    3 +--
 drivers/block/rbd.c    |    8 ++------
 fs/bio.c               |   13 +++++++++++++
 fs/exofs/ore.c         |    5 ++---
 include/linux/bio.h    |    1 +
 5 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/block/osdblk.c b/drivers/block/osdblk.c
index 87311eb..1bbc681 100644
--- a/drivers/block/osdblk.c
+++ b/drivers/block/osdblk.c
< at >< at > -266,11 +266,10 < at >< at > static struct bio *bio_chain_clone(struct bio *old_chain, gfp_t gfpmask)
 struct bio *tmp, *new_chain = NULL, *tail = NULL;
 
 while (old_chain) {
-tmp = bio_kmalloc(gfpmask, old_chain->bi_max_vecs);
+tmp = bio_clone_kmalloc(old_chain, gfpmask);
 if (!tmp)
 goto err_out;
 
-__bio_clone(tmp, old_chain);
 tmp->bi_bdev = NULL;
 gfpmask &= ~__GFP_WAIT;
 tmp->bi_next = NULL;
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 013c7a5..5a953c6 100644
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
< at >< at > -729,7 +729,7 < at >< at > static struct bio *bio_chain_clone(struct bio **old, struct bio **next,
 }
 
 while (old_chain && (total < len)) {
-tmp = bio_kmalloc(gfpmask, old_chain->bi_max_vecs);
+tmp = bio_clone_kmalloc(old_chain, gfpmask);
 if (!tmp)
 goto err_out;
 
< at >< at > -751,13 +751,9 < at >< at > static struct bio *bio_chain_clone(struct bio **old, struct bio **next,
 if (!bp)
 goto err_out;
 
-__bio_clone(tmp, &bp->bio1);
-
 *next = &bp->bio2;
-} else {
-__bio_clone(tmp, old_chain);
+} else
 *next = old_chain->bi_next;
-}
 
 tmp->bi_bdev = NULL;
 gfpmask &= ~__GFP_WAIT;
diff --git a/fs/bio.c b/fs/bio.c
index 4cb621f..e2c0970 100644
--- a/fs/bio.c
+++ b/fs/bio.c
< at >< at > -491,6 +491,19 < at >< at > struct bio *bio_clone(struct bio *bio, gfp_t gfp_mask)
 }
 EXPORT_SYMBOL(bio_clone);
 
+struct bio *bio_clone_kmalloc(struct bio *bio, gfp_t gfp_mask)
+{
+struct bio *b = bio_kmalloc(gfp_mask, bio->bi_max_vecs);
+
+if (!b)
+return NULL;
+
+__bio_clone(b, bio);
+
+return b;
+}
+EXPORT_SYMBOL(bio_clone_kmalloc);
+
 /**
  *bio_get_nr_vecs- return approx number of vecs
  *< at >bdev:  I/O target
diff --git a/fs/exofs/ore.c b/fs/exofs/ore.c
index 49cf230..95c7264 100644
--- a/fs/exofs/ore.c
+++ b/fs/exofs/ore.c
< at >< at > -820,8 +820,8 < at >< at > static int _write_mirror(struct ore_io_state *ios, int cur_comp)
 struct bio *bio;
 
 if (per_dev != master_dev) {
-bio = bio_kmalloc(GFP_KERNEL,
-  master_dev->bio->bi_max_vecs);
+bio = bio_clone_kmalloc(master_dev->bio,
+GFP_KERNEL);
 if (unlikely(!bio)) {
 ORE_DBGMSG(
       "Failed to allocate BIO size=%u\n",
< at >< at > -830,7 +830,6 < at >< at > static int _write_mirror(struct ore_io_state *ios, int cur_comp)
 goto out;
 }
 
-__bio_clone(bio, master_dev->bio);
 bio->bi_bdev = NULL;
 bio->bi_next = NULL;
 per_dev->offset = master_dev->offset;
diff --git a/include/linux/bio.h b/include/linux/bio.h
index e068a68..b27f16b 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
< at >< at > -225,6 +225,7 < at >< at > extern int bio_phys_segments(struct request_queue *, struct bio *);
 extern void __bio_clone(struct bio *, struct bio *);
 extern struct bio *bio_clone_bioset(struct bio *, gfp_t, struct bio_set *bs);
 extern struct bio *bio_clone(struct bio *, gfp_t);
+struct bio *bio_clone_kmalloc(struct bio *, gfp_t);
 
 extern void bio_init(struct bio *);

[PATCH v2 03/14] block: Add bio_clone_bioset()

Kent Overstreet — 2012-05-24T00:02:40

This consolidates some code, and will help in a later patch changing how
bio cloning works.

Signed-off-by: Kent Overstreet <koverstreet< at >google.com>
Change-Id: If6cfd840db9cf90a798c6a5862e3314465c83000
---
 block/blk-core.c    |    8 +-------
 drivers/md/dm.c     |   27 ++++++++++-----------------
 drivers/md/md.c     |   20 +-------------------
 fs/bio.c            |   16 ++++++++++++----
 include/linux/bio.h |    1 +
 5 files changed, 25 insertions(+), 47 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 1f61b74..91617eb 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
< at >< at > -2660,16 +2660,10 < at >< at > int blk_rq_prep_clone(struct request *rq, struct request *rq_src,
 blk_rq_init(NULL, rq);
 
 __rq_for_each_bio(bio_src, rq_src) {
-bio = bio_alloc_bioset(gfp_mask, bio_src->bi_max_vecs, bs);
+bio = bio_clone_bioset(bio_src, gfp_mask, bs);
 if (!bio)
 goto free_and_out;
 
-__bio_clone(bio, bio_src);
-
-if (bio_integrity(bio_src) &&
-    bio_integrity_clone(bio, bio_src, gfp_mask, bs))
-goto free_and_out;
-
 if (bio_ctr && bio_ctr(bio, bio_src, data))
 goto free_and_out;
 
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index e6e7b19..e5076f2 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
< at >< at > -1062,26 +1062,19 < at >< at > static struct bio *split_bvec(struct bio *bio, sector_t sector,
  * Creates a bio that consists of range of complete bvecs.
  */
 static struct bio *clone_bio(struct bio *bio, sector_t sector,
-     unsigned short idx, unsigned short bv_count,
+     unsigned short bv_count,
      unsigned int len, struct bio_set *bs)
 {
 struct bio *clone;
 
-clone = bio_alloc_bioset(GFP_NOIO, bio->bi_max_vecs, bs);
-__bio_clone(clone, bio);
+clone = bio_clone_bioset(bio, GFP_NOIO, bs);
 clone->bi_sector = sector;
-clone->bi_idx = idx;
-clone->bi_vcnt = idx + bv_count;
+clone->bi_vcnt = bv_count;
 clone->bi_size = to_bytes(len);
-clone->bi_flags &= ~(1 << BIO_SEG_VALID);
 
-if (bio_integrity(bio)) {
-bio_integrity_clone(clone, bio, GFP_NOIO, bs);
-
-if (idx != bio->bi_idx || clone->bi_size < bio->bi_size)
-bio_integrity_trim(clone,
-   bio_sector_offset(bio, idx, 0), len);
-}
+if (bio_integrity(bio) &&
+    clone->bi_size < bio->bi_size)
+bio_integrity_trim(clone, 0, len);
 
 return clone;
 }
< at >< at > -1111,8 +1104,8 < at >< at > static void __issue_target_request(struct clone_info *ci, struct dm_target *ti,
  * ci->bio->bi_max_vecs is BIO_INLINE_VECS anyway, for both flush
  * and discard, so no need for concern about wasted bvec allocations.
  */
-clone = bio_alloc_bioset(GFP_NOIO, ci->bio->bi_max_vecs, ci->md->bs);
-__bio_clone(clone, ci->bio);
+clone = bio_clone_bioset(ci->bio, GFP_NOIO, ci->md->bs);
+
 if (len) {
 clone->bi_sector = ci->sector;
 clone->bi_size = to_bytes(len);
< at >< at > -1151,7 +1144,7 < at >< at > static void __clone_and_map_simple(struct clone_info *ci, struct dm_target *ti)
 struct dm_target_io *tio;
 
 tio = alloc_tio(ci, ti);
-clone = clone_bio(bio, ci->sector, ci->idx,
+clone = clone_bio(bio, ci->sector,
   bio->bi_vcnt - ci->idx, ci->sector_count,
   ci->md->bs);
 __map_bio(ti, clone, tio);
< at >< at > -1230,7 +1223,7 < at >< at > static int __clone_and_map(struct clone_info *ci)
 }
 
 tio = alloc_tio(ci, ti);
-clone = clone_bio(bio, ci->sector, ci->idx, i - ci->idx, len,
+clone = clone_bio(bio, ci->sector, i - ci->idx, len,
   ci->md->bs);
 __map_bio(ti, clone, tio);
 
diff --git a/drivers/md/md.c b/drivers/md/md.c
index a5a524e..508ae81 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
< at >< at > -173,28 +173,10 < at >< at > EXPORT_SYMBOL_GPL(bio_alloc_mddev);
 struct bio *bio_clone_mddev(struct bio *bio, gfp_t gfp_mask,
     struct mddev *mddev)
 {
-struct bio *b;
-
 if (!mddev || !mddev->bio_set)
 return bio_clone(bio, gfp_mask);
 
-b = bio_alloc_bioset(gfp_mask, bio->bi_max_vecs, mddev->bio_set);
-if (!b)
-return NULL;
-
-__bio_clone(b, bio);
-if (bio_integrity(bio)) {
-int ret;
-
-ret = bio_integrity_clone(b, bio, gfp_mask, mddev->bio_set);
-
-if (ret < 0) {
-bio_put(b);
-return NULL;
-}
-}
-
-return b;
+return bio_clone_bioset(bio, gfp_mask, mddev->bio_set);
 }
 EXPORT_SYMBOL_GPL(bio_clone_mddev);
 
diff --git a/fs/bio.c b/fs/bio.c
index 3667cef..4cb621f 100644
--- a/fs/bio.c
+++ b/fs/bio.c
< at >< at > -453,15 +453,17 < at >< at > void __bio_clone(struct bio *bio, struct bio *bio_src)
 EXPORT_SYMBOL(__bio_clone);
 
 /**
- *bio_clone-clone a bio
+ *bio_clone_bioset -clone a bio
  *< at >bio: bio to clone
  *< at >gfp_mask: allocation priority
+ *< at >bs: bio_set to allocate from
  *
  * Like __bio_clone, only also allocates the returned bio
  */
-struct bio *bio_clone(struct bio *bio, gfp_t gfp_mask)
+struct bio *bio_clone_bioset(struct bio *bio, gfp_t gfp_mask,
+     struct bio_set *bs)
 {
-struct bio *b = bio_alloc(gfp_mask, bio->bi_max_vecs);
+struct bio *b = bio_alloc_bioset(gfp_mask, bio->bi_max_vecs, bs);
 
 if (!b)
 return NULL;
< at >< at > -471,7 +473,7 < at >< at > struct bio *bio_clone(struct bio *bio, gfp_t gfp_mask)
 if (bio_integrity(bio)) {
 int ret;
 
-ret = bio_integrity_clone(b, bio, gfp_mask, fs_bio_set);
+ret = bio_integrity_clone(b, bio, gfp_mask, bs);
 
 if (ret < 0) {
 bio_put(b);
< at >< at > -481,6 +483,12 < at >< at > struct bio *bio_clone(struct bio *bio, gfp_t gfp_mask)
 
 return b;
 }
+EXPORT_SYMBOL(bio_clone_bioset);
+
+struct bio *bio_clone(struct bio *bio, gfp_t gfp_mask)
+{
+return bio_clone_bioset(bio, gfp_mask, fs_bio_set);
+}
 EXPORT_SYMBOL(bio_clone);
 
 /**
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 4d94eb8..e068a68 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
< at >< at > -223,6 +223,7 < at >< at > struct request_queue;
 extern int bio_phys_segments(struct request_queue *, struct bio *);
 
 extern void __bio_clone(struct bio *, struct bio *);
+extern struct bio *bio_clone_bioset(struct bio *, gfp_t, struct bio_set *bs);
 extern struct bio *bio_clone(struct bio *, gfp_t);
 
 extern void bio_init(struct bio *);

[PATCH v2 07/14] pktcdvd: Switch to bio_kmalloc()

Kent Overstreet — 2012-05-24T00:02:44

This is prep work for killing bi_destructor - previously, pktcdvd had
its own pkt_bio_alloc which was basically duplication bio_kmalloc(),
necessitating its own bi_destructor implementation.

Signed-off-by: Kent Overstreet <koverstreet< at >google.com>
Change-Id: I5604293e07f695c8f0106ae819e306f1def89a67
---
 drivers/block/pktcdvd.c |  115 ++++++++++++++++-------------------------------
 1 file changed, 39 insertions(+), 76 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index ba66e44..6fe693a 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
< at >< at > -522,36 +522,38 < at >< at > static void pkt_bio_finished(struct pktcdvd_device *pd)
 }
 }
 
-static void pkt_bio_destructor(struct bio *bio)
+static void pkt_end_io_read(struct bio *bio, int err)
 {
-kfree(bio->bi_io_vec);
-kfree(bio);
-}
+struct packet_data *pkt = bio->bi_private;
+struct pktcdvd_device *pd = pkt->pd;
+BUG_ON(!pd);
 
-static struct bio *pkt_bio_alloc(int nr_iovecs)
-{
-struct bio_vec *bvl = NULL;
-struct bio *bio;
+VPRINTK("pkt_end_io_read: bio=%p sec0=%llx sec=%llx err=%d\n", bio,
+(unsigned long long)pkt->sector, (unsigned long long)bio->bi_sector, err);
 
-bio = kmalloc(sizeof(struct bio), GFP_KERNEL);
-if (!bio)
-goto no_bio;
-bio_init(bio);
+if (err)
+atomic_inc(&pkt->io_errors);
+if (atomic_dec_and_test(&pkt->io_wait)) {
+atomic_inc(&pkt->run_sm);
+wake_up(&pd->wqueue);
+}
+pkt_bio_finished(pd);
+}
 
-bvl = kcalloc(nr_iovecs, sizeof(struct bio_vec), GFP_KERNEL);
-if (!bvl)
-goto no_bvl;
+static void pkt_end_io_packet_write(struct bio *bio, int err)
+{
+struct packet_data *pkt = bio->bi_private;
+struct pktcdvd_device *pd = pkt->pd;
+BUG_ON(!pd);
 
-bio->bi_max_vecs = nr_iovecs;
-bio->bi_io_vec = bvl;
-bio->bi_destructor = pkt_bio_destructor;
+VPRINTK("pkt_end_io_packet_write: id=%d, err=%d\n", pkt->id, err);
 
-return bio;
+pd->stats.pkt_ended++;
 
- no_bvl:
-kfree(bio);
- no_bio:
-return NULL;
+pkt_bio_finished(pd);
+atomic_dec(&pkt->io_wait);
+atomic_inc(&pkt->run_sm);
+wake_up(&pd->wqueue);
 }
 
 /*
< at >< at > -567,10 +569,13 < at >< at > static struct packet_data *pkt_alloc_packet_data(int frames)
 goto no_pkt;
 
 pkt->frames = frames;
-pkt->w_bio = pkt_bio_alloc(frames);
+pkt->w_bio = bio_kmalloc(GFP_KERNEL, frames);
 if (!pkt->w_bio)
 goto no_bio;
 
+pkt->w_bio->bi_end_io = pkt_end_io_packet_write;
+pkt->w_bio->bi_private = pkt;
+
 for (i = 0; i < frames / FRAMES_PER_PAGE; i++) {
 pkt->pages[i] = alloc_page(GFP_KERNEL|__GFP_ZERO);
 if (!pkt->pages[i])
< at >< at > -581,9 +586,12 < at >< at > static struct packet_data *pkt_alloc_packet_data(int frames)
 bio_list_init(&pkt->orig_bios);
 
 for (i = 0; i < frames; i++) {
-struct bio *bio = pkt_bio_alloc(1);
+struct bio *bio = bio_kmalloc(GFP_KERNEL, 1);
 if (!bio)
 goto no_rd_bio;
+
+bio->bi_end_io = pkt_end_io_read;
+bio->bi_private = pkt;
 pkt->r_bios[i] = bio;
 }
 
< at >< at > -1036,40 +1044,6 < at >< at > static void pkt_make_local_copy(struct packet_data *pkt, struct bio_vec *bvec)
 }
 }
 
-static void pkt_end_io_read(struct bio *bio, int err)
-{
-struct packet_data *pkt = bio->bi_private;
-struct pktcdvd_device *pd = pkt->pd;
-BUG_ON(!pd);
-
-VPRINTK("pkt_end_io_read: bio=%p sec0=%llx sec=%llx err=%d\n", bio,
-(unsigned long long)pkt->sector, (unsigned long long)bio->bi_sector, err);
-
-if (err)
-atomic_inc(&pkt->io_errors);
-if (atomic_dec_and_test(&pkt->io_wait)) {
-atomic_inc(&pkt->run_sm);
-wake_up(&pd->wqueue);
-}
-pkt_bio_finished(pd);
-}
-
-static void pkt_end_io_packet_write(struct bio *bio, int err)
-{
-struct packet_data *pkt = bio->bi_private;
-struct pktcdvd_device *pd = pkt->pd;
-BUG_ON(!pd);
-
-VPRINTK("pkt_end_io_packet_write: id=%d, err=%d\n", pkt->id, err);
-
-pd->stats.pkt_ended++;
-
-pkt_bio_finished(pd);
-atomic_dec(&pkt->io_wait);
-atomic_inc(&pkt->run_sm);
-wake_up(&pd->wqueue);
-}
-
 /*
  * Schedule reads for the holes in a packet
  */
< at >< at > -1111,21 +1085,15 < at >< at > static void pkt_gather_data(struct pktcdvd_device *pd, struct packet_data *pkt)
  * Schedule reads for missing parts of the packet.
  */
 for (f = 0; f < pkt->frames; f++) {
-struct bio_vec *vec;
-
 int p, offset;
+
 if (written[f])
 continue;
+
 bio = pkt->r_bios[f];
-vec = bio->bi_io_vec;
-bio_init(bio);
-bio->bi_max_vecs = 1;
-bio->bi_sector = pkt->sector + f * (CD_FRAMESIZE >> 9);
-bio->bi_bdev = pd->bdev;
-bio->bi_end_io = pkt_end_io_read;
-bio->bi_private = pkt;
-bio->bi_io_vec = vec;
-bio->bi_destructor = pkt_bio_destructor;
+bio_reset(bio);
+bio->bi_sector= pkt->sector + f * (CD_FRAMESIZE >> 9);
+bio->bi_bdev= pd->bdev;
 
 p = (f * CD_FRAMESIZE) / PAGE_SIZE;
 offset = (f * CD_FRAMESIZE) % PAGE_SIZE;
< at >< at > -1418,14 +1386,9 < at >< at > static void pkt_start_write(struct pktcdvd_device *pd, struct packet_data *pkt)
 }
 
 /* Start the write request */
-bio_init(pkt->w_bio);
-pkt->w_bio->bi_max_vecs = PACKET_MAX_SIZE;
+bio_reset(pkt->w_bio);
 pkt->w_bio->bi_sector = pkt->sector;
 pkt->w_bio->bi_bdev = pd->bdev;
-pkt->w_bio->bi_end_io = pkt_end_io_packet_write;
-pkt->w_bio->bi_private = pkt;
-pkt->w_bio->bi_io_vec = bvec;
-pkt->w_bio->bi_destructor = pkt_bio_destructor;
 for (f = 0; f < pkt->frames; f++)
 if (!bio_add_page(pkt->w_bio, bvec[f].bv_page, CD_FRAMESIZE, bvec[f].bv_offset))
 BUG();

[PATCH] multipath: Some device configuration changes forNetAppLUNs

Benjamin Marzinski — 2012-05-23T21:42:20

NetApp has asked for these configuration changes.

Signed-off-by: Benjamin Marzinski <bmarzins< at >redhat.com>
---
 libmultipath/hwtable.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: multipath-tools-120518/libmultipath/hwtable.c
===================================================================
--- multipath-tools-120518.orig/libmultipath/hwtable.c
+++ multipath-tools-120518/libmultipath/hwtable.c
< at >< at > -912,11 +912,12 < at >< at > static struct hwentry default_hw[] = {
 {
 .vendor        = "NETAPP",
 .product       = "LUN.*",
-.features      = "1 queue_if_no_path",
+.features      = "3 queue_if_no_path pg_init_retries 50",
 .hwhandler     = DEFAULT_HWHANDLER,
 .selector      = DEFAULT_SELECTOR,
 .pgpolicy      = GROUP_BY_PRIO,
 .pgfailback    = -FAILBACK_IMMEDIATE,
+.flush_on_last_del = FLUSH_ENABLED,
 .rr_weight     = RR_WEIGHT_NONE,
 .no_path_retry = NO_PATH_RETRY_UNDEF,
 .minio         = 128,

[PATCH][RESEND] multipath: fix scsi async tur checkercorruption

Benjamin Marzinski — 2012-05-23T21:05:20

Since the tur checker runs asynchronously in its own thread, there is nothing
that keeps a path from being orphaned or deleted before the tur thread has
finished. When this happenes the checker struct gets deleted.  However, the tur
thread might still we writing to that memory.  This can lead to memory
corruption.  This patch adds all of the necessary data to the checker context,
and makes the tur thread only use that. This way, if the checker is deleted
while the thread is still using the context, the thread will clean up the
context itself.

Since the context can only be freed when both the thread and the paths checker
structure have stopped needing it, and these can get get finished with the
context asychronously with respect to each other, the context has a holders
counter, protected by a spinlock, to keep track of the users.  When the
counter drops to zero, the context gets freed.

Signed-off-by: Benjamin Marzinski <bmarzins< at >redhat.com>
---
 libmultipath/checkers/tur.c |   85 +++++++++++++++++++++++++++++++++-----------
 libmultipath/discovery.c    |    1 
 2 files changed, 65 insertions(+), 21 deletions(-)

Index: multipath-tools-120518/libmultipath/checkers/tur.c
===================================================================
--- multipath-tools-120518.orig/libmultipath/checkers/tur.c
+++ multipath-tools-120518/libmultipath/checkers/tur.c
< at >< at > -35,10 +35,15 < at >< at > struct tur_checker_context {
 dev_t devt;
 int state;
 int running;
-time_t timeout;
+int fd;
+unsigned int timeout;
+time_t time;
 pthread_t thread;
 pthread_mutex_t lock;
 pthread_cond_t active;
+pthread_spinlock_t hldr_lock;
+int holders;
+char message[CHECKER_MSG_LEN];
 };
 
 #define TUR_DEVT(c) major((c)->devt), minor((c)->devt)
< at >< at > -53,28 +58,49 < at >< at > int libcheck_init (struct checker * c)
 memset(ct, 0, sizeof(struct tur_checker_context));
 
 ct->state = PATH_UNCHECKED;
+ct->fd = -1;
+ct->holders = 1;
 pthread_cond_init(&ct->active, NULL);
 pthread_mutex_init(&ct->lock, NULL);
+pthread_spin_init(&ct->hldr_lock, PTHREAD_PROCESS_PRIVATE);
 c->context = ct;
 
 return 0;
 }
 
+void cleanup_context(struct tur_checker_context *ct)
+{
+pthread_mutex_destroy(&ct->lock);
+pthread_cond_destroy(&ct->active);
+pthread_spin_destroy(&ct->hldr_lock);
+free(ct);
+}
+
 void libcheck_free (struct checker * c)
 {
 if (c->context) {
 struct tur_checker_context *ct = c->context;
+int holders;
+pthread_t thread;
 
-pthread_mutex_destroy(&ct->lock);
-pthread_cond_destroy(&ct->active);
-free(c->context);
+pthread_spin_lock(&ct->hldr_lock);
+ct->holders--;
+holders = ct->holders;
+thread = ct->thread;
+pthread_spin_unlock(&ct->hldr_lock);
+if (holders)
+pthread_cancel(thread);
+else
+cleanup_context(ct);
 c->context = NULL;
 }
 return;
 }
 
+#define TUR_MSG(msg, fmt, args...) snprintf(msg, CHECKER_MSG_LEN, fmt, ##args);
+
 int
-tur_check (struct checker * c)
+tur_check(int fd, unsigned int timeout, char *msg)
 {
 struct sg_io_hdr io_hdr;
 unsigned char turCmdBlk[TUR_CMD_LEN] = { 0x00, 0, 0, 0, 0, 0 };
< at >< at > -90,10 +116,10 < at >< at > tur_check (struct checker * c)
 io_hdr.dxfer_direction = SG_DXFER_NONE;
 io_hdr.cmdp = turCmdBlk;
 io_hdr.sbp = sense_buffer;
-io_hdr.timeout = c->timeout;
+io_hdr.timeout = timeout;
 io_hdr.pack_id = 0;
-if (ioctl(c->fd, SG_IO, &io_hdr) < 0) {
-MSG(c, MSG_TUR_DOWN);
+if (ioctl(fd, SG_IO, &io_hdr) < 0) {
+TUR_MSG(msg, MSG_TUR_DOWN);
 return PATH_DOWN;
 }
 if ((io_hdr.status & 0x7e) == 0x18) {
< at >< at > -101,7 +127,7 < at >< at > tur_check (struct checker * c)
  * SCSI-3 arrays might return
  * reservation conflict on TUR
  */
-MSG(c, MSG_TUR_UP);
+TUR_MSG(msg, MSG_TUR_UP);
 return PATH_UP;
 }
 if (io_hdr.info & SG_INFO_OK_MASK) {
< at >< at > -146,14 +172,14 < at >< at > tur_check (struct checker * c)
  * LOGICAL UNIT NOT ACCESSIBLE,
  * TARGET PORT IN STANDBY STATE
  */
-MSG(c, MSG_TUR_GHOST);
+TUR_MSG(msg, MSG_TUR_GHOST);
 return PATH_GHOST;
 }
 }
-MSG(c, MSG_TUR_DOWN);
+TUR_MSG(msg, MSG_TUR_DOWN);
 return PATH_DOWN;
 }
-MSG(c, MSG_TUR_UP);
+TUR_MSG(msg, MSG_TUR_UP);
 return PATH_UP;
 }
 
< at >< at > -162,18 +188,25 < at >< at > tur_check (struct checker * c)
 
 void cleanup_func(void *data)
 {
+int holders;
 struct tur_checker_context *ct = data;
+pthread_spin_lock(&ct->hldr_lock);
+ct->holders--;
+holders = ct->holders;
 ct->thread = 0;
+pthread_spin_unlock(&ct->hldr_lock);
+if (!holders)
+cleanup_context(ct);
 }
 
 void *tur_thread(void *ctx)
 {
-struct checker *c = ctx;
-struct tur_checker_context *ct = c->context;
+struct tur_checker_context *ct = ctx;
 int state;
 
 condlog(3, "%d:%d: tur checker starting up", TUR_DEVT(ct));
 
+ct->message[0] = '\0';
 /* This thread can be canceled, so setup clean up */
 tur_thread_cleanup_push(ct)
 
< at >< at > -182,7 +215,7 < at >< at > void *tur_thread(void *ctx)
 ct->state = PATH_PENDING;
 pthread_mutex_unlock(&ct->lock);
 
-state = tur_check(c);
+state = tur_check(ct->fd, ct->timeout, ct->message);
 
 /* TUR checker done */
 pthread_mutex_lock(&ct->lock);
< at >< at > -213,7 +246,7 < at >< at > void tur_set_async_timeout(struct checke
 struct timeval now;
 
 gettimeofday(&now, NULL);
-ct->timeout = now.tv_sec + c->timeout;
+ct->time = now.tv_sec + c->timeout;
 }
 
 int tur_check_async_timeout(struct checker *c)
< at >< at > -222,7 +255,7 < at >< at > int tur_check_async_timeout(struct check
 struct timeval now;
 
 gettimeofday(&now, NULL);
-return (now.tv_sec > ct->timeout);
+return (now.tv_sec > ct->time);
 }
 
 extern int
< at >< at > -242,7 +275,7 < at >< at > libcheck_check (struct checker * c)
 ct->devt = sb.st_rdev;
 
 if (c->sync)
-return tur_check(c);
+return tur_check(c->fd, c->timeout, c->message);
 
 /*
  * Async mode
< at >< at > -276,6 +309,8 < at >< at > libcheck_check (struct checker * c)
 /* TUR checker done */
 ct->running = 0;
 tur_status = ct->state;
+strncpy(c->message, ct->message, CHECKER_MSG_LEN);
+c->message[CHECKER_MSG_LEN - 1] = '\0';
 }
 pthread_mutex_unlock(&ct->lock);
 } else {
< at >< at > -284,24 +319,32 < at >< at > libcheck_check (struct checker * c)
 pthread_mutex_unlock(&ct->lock);
 condlog(3, "%d:%d: tur thread not responding, "
 "using sync mode", TUR_DEVT(ct));
-return tur_check(c);
+return tur_check(c->fd, c->timeout, c->message);
 }
 /* Start new TUR checker */
 ct->state = PATH_UNCHECKED;
+ct->fd = c->fd;
+ct->timeout = c->timeout;
+pthread_spin_lock(&ct->hldr_lock);
+ct->holders++;
+pthread_spin_unlock(&ct->hldr_lock);
 tur_set_async_timeout(c);
 setup_thread_attr(&attr, 32 * 1024, 1);
-r = pthread_create(&ct->thread, &attr, tur_thread, c);
+r = pthread_create(&ct->thread, &attr, tur_thread, ct);
 if (r) {
 pthread_mutex_unlock(&ct->lock);
 ct->thread = 0;
+ct->holders--;
 condlog(3, "%d:%d: failed to start tur thread, using"
 " sync mode", TUR_DEVT(ct));
-return tur_check(c);
+return tur_check(c->fd, c->timeout, c->message);
 }
 pthread_attr_destroy(&attr);
 tur_timeout(&tsp);
 r = pthread_cond_timedwait(&ct->active, &ct->lock, &tsp);
 tur_status = ct->state;
+strncpy(c->message, ct->message,CHECKER_MSG_LEN);
+c->message[CHECKER_MSG_LEN -1] = '\0';
 pthread_mutex_unlock(&ct->lock);
 if (ct->thread &&
     (tur_status == PATH_PENDING || tur_status == PATH_UNCHECKED)) {
Index: multipath-tools-120518/libmultipath/discovery.c
===================================================================
--- multipath-tools-120518.orig/libmultipath/discovery.c
+++ multipath-tools-120518/libmultipath/discovery.c
< at >< at > -747,6 +747,7 < at >< at > get_state (struct path * pp, int daemon)
 }
 checker_set_fd(c, pp->fd);
 if (checker_init(c, pp->mpp?&pp->mpp->mpcontext:NULL)) {
+memset(c, 0x0, sizeof(struct checker));
 condlog(3, "%s: checker init failed", pp->dev);
 return PATH_UNCHECKED;
 }

Re: [PATCH] multipath: Make sure we store all the hwentry attributes.

Christophe Varoqui — 2012-05-23T20:37:23

Applied.

[PATCH] multipath: Allow user_friendly_names in moreconfigsections

Benjamin Marzinski — 2012-05-23T20:36:30

This patch adds support for setting user_friendly_names in the devices and
multipaths config sections.

Signed-off-by: Benjamin Marzinski <bmarzins< at >redhat.com>
---
 libmultipath/config.c  |    2 +
 libmultipath/config.h  |    2 +
 libmultipath/dict.c    |   94 +++++++++++++++++++++++++++++++++++++++++++++----
 libmultipath/propsel.c |   14 ++++++-
 libmultipath/structs.h |    6 +++
 5 files changed, 110 insertions(+), 8 deletions(-)

Index: multipath-tools-120518/libmultipath/config.c
===================================================================
--- multipath-tools-120518.orig/libmultipath/config.c
+++ multipath-tools-120518/libmultipath/config.c
< at >< at > -325,6 +325,7 < at >< at > merge_hwe (struct hwentry * dst, struct
 merge_num(flush_on_last_del);
 merge_num(fast_io_fail);
 merge_num(dev_loss);
+merge_num(user_friendly_names);
 
 return 0;
 }
< at >< at > -383,6 +384,7 < at >< at > store_hwe (vector hwtable, struct hwentr
 hwe->flush_on_last_del = dhwe->flush_on_last_del;
 hwe->fast_io_fail = dhwe->fast_io_fail;
 hwe->dev_loss = dhwe->dev_loss;
+hwe->user_friendly_names = dhwe->user_friendly_names;
 
 if (dhwe->bl_product && !(hwe->bl_product = set_param_str(dhwe->bl_product)))
 goto out;
Index: multipath-tools-120518/libmultipath/config.h
===================================================================
--- multipath-tools-120518.orig/libmultipath/config.h
+++ multipath-tools-120518/libmultipath/config.h
< at >< at > -44,6 +44,7 < at >< at > struct hwentry {
 int flush_on_last_del;
 int fast_io_fail;
 unsigned int dev_loss;
+int user_friendly_names;
 char * bl_product;
 };
 
< at >< at > -66,6 +67,7 < at >< at > struct mpentry {
 int pg_timeout;
 int flush_on_last_del;
 int attribute_flags;
+int user_friendly_names;
 uid_t uid;
 gid_t gid;
 mode_t mode;
Index: multipath-tools-120518/libmultipath/dict.c
===================================================================
--- multipath-tools-120518.orig/libmultipath/dict.c
+++ multipath-tools-120518/libmultipath/dict.c
< at >< at > -583,7 +583,7 < at >< at > def_reservation_key_handler(vector strve
 }
 
 static int
-names_handler(vector strvec)
+def_names_handler(vector strvec)
 {
 char * buff;
 
< at >< at > -594,10 +594,12 < at >< at > names_handler(vector strvec)
 
 if ((strlen(buff) == 2 && !strcmp(buff, "no")) ||
     (strlen(buff) == 1 && !strcmp(buff, "0")))
-conf->user_friendly_names = 0;
+conf->user_friendly_names = USER_FRIENDLY_NAMES_OFF;
 else if ((strlen(buff) == 3 && !strcmp(buff, "yes")) ||
  (strlen(buff) == 1 && !strcmp(buff, "1")))
-conf->user_friendly_names = 1;
+conf->user_friendly_names = USER_FRIENDLY_NAMES_ON;
+else
+conf->user_friendly_names = USER_FRIENDLY_NAMES_UNDEF;
 
 FREE(buff);
 return 0;
< at >< at > -1207,6 +1209,32 < at >< at > hw_flush_on_last_del_handler(vector strv
 return 0;
 }
 
+static int
+hw_names_handler(vector strvec)
+{
+struct hwentry *hwe = VECTOR_LAST_SLOT(conf->hwtable);
+char * buff;
+
+if (!hwe)
+return 1;
+
+buff = set_value(strvec);
+if (!buff)
+return 1;
+
+if ((strlen(buff) == 2 && strcmp(buff, "no") == 0) ||
+    (strlen(buff) == 1 && strcmp(buff, "0") == 0))
+hwe->user_friendly_names = USER_FRIENDLY_NAMES_OFF;
+else if ((strlen(buff) == 3 && strcmp(buff, "yes") == 0) ||
+ (strlen(buff) == 1 && strcmp(buff, "1") == 0))
+hwe->user_friendly_names = USER_FRIENDLY_NAMES_ON;
+else
+hwe->user_friendly_names = USER_FRIENDLY_NAMES_UNDEF;
+
+FREE(buff);
+return 0;
+}
+
 /*
  * multipaths block handlers
  */
< at >< at > -1654,6 +1682,31 < at >< at > mp_reservation_key_handler (vector strve
 return 0;
 }
 
+static int
+mp_names_handler(vector strvec)
+{
+struct mpentry *mpe = VECTOR_LAST_SLOT(conf->mptable);
+char * buff;
+
+if (!mpe)
+return 1;
+
+buff = set_value(strvec);
+if (!buff)
+return 1;
+
+if ((strlen(buff) == 2 && strcmp(buff, "no") == 0) ||
+    (strlen(buff) == 1 && strcmp(buff, "0") == 0))
+mpe->user_friendly_names = USER_FRIENDLY_NAMES_OFF;
+else if ((strlen(buff) == 3 && strcmp(buff, "yes") == 0) ||
+ (strlen(buff) == 1 && strcmp(buff, "1") == 0))
+mpe->user_friendly_names = USER_FRIENDLY_NAMES_ON;
+else
+mpe->user_friendly_names = USER_FRIENDLY_NAMES_UNDEF;
+
+FREE(buff);
+return 0;
+}
 
 /*
  * config file keywords printing
< at >< at > -1884,6 +1937,18 < at >< at > snprint_mp_reservation_key (char * buff,
 return snprintf(buff, len, "%s" , mpe->reservation_key);
 }
 
+static int
+snprint_mp_user_friendly_names (char * buff, int len, void * data)
+{
+struct mpentry * mpe = (struct mpentry *)data;
+
+if (mpe->user_friendly_names == USER_FRIENDLY_NAMES_UNDEF)
+return 0;
+else if (mpe->user_friendly_names == USER_FRIENDLY_NAMES_OFF)
+return snprintf(buff, len, "no");
+else
+return snprintf(buff, len, "yes");
+}
 
 static int
 snprint_hw_fast_io_fail(char * buff, int len, void * data)
< at >< at > -2174,6 +2239,19 < at >< at > snprint_hw_path_checker (char * buff, in
 return snprintf(buff, len, "%s", hwe->checker_name);
 }
 
+static int
+snprint_hw_user_friendly_names (char * buff, int len, void * data)
+{
+struct hwentry * hwe = (struct hwentry *)data;
+
+if (hwe->user_friendly_names == USER_FRIENDLY_NAMES_UNDEF)
+return 0;
+else if (hwe->user_friendly_names == USER_FRIENDLY_NAMES_OFF)
+return snprintf(buff, len, "no");
+else
+return snprintf(buff, len, "yes");
+}
+
 static int
 snprint_def_polling_interval (char * buff, int len, void * data)
 {
< at >< at > -2461,10 +2539,10 < at >< at > snprint_def_log_checker_err (char * buff
 static int
 snprint_def_user_friendly_names (char * buff, int len, void * data)
 {
-if (!conf->user_friendly_names)
+if (conf->user_friendly_names  == USER_FRIENDLY_NAMES_ON)
+return snprintf(buff, len, "yes");
+else
 return snprintf(buff, len, "no");
-
-return snprintf(buff, len, "yes");
 }
 
 static int
< at >< at > -2547,7 +2625,7 < at >< at > init_keywords(void)
 install_keyword("checker_timeout", &def_checker_timeout_handler, &snprint_def_checker_timeout);
 install_keyword("pg_timeout", &def_pg_timeout_handler, &snprint_def_pg_timeout);
 install_keyword("flush_on_last_del", &def_flush_on_last_del_handler, &snprint_def_flush_on_last_del);
-install_keyword("user_friendly_names", &names_handler, &snprint_def_user_friendly_names);
+install_keyword("user_friendly_names", &def_names_handler, &snprint_def_user_friendly_names);
 install_keyword("mode", &def_mode_handler, &snprint_def_mode);
 install_keyword("uid", &def_uid_handler, &snprint_def_uid);
 install_keyword("gid", &def_gid_handler, &snprint_def_gid);
< at >< at > -2616,6 +2694,7 < at >< at > init_keywords(void)
 install_keyword("flush_on_last_del", &hw_flush_on_last_del_handler, &snprint_hw_flush_on_last_del);
 install_keyword("fast_io_fail_tmo", &hw_fast_io_fail_handler, &snprint_hw_fast_io_fail);
 install_keyword("dev_loss_tmo", &hw_dev_loss_handler, &snprint_hw_dev_loss);
+install_keyword("user_friendly_names", &hw_names_handler, &snprint_hw_user_friendly_names);
 install_sublevel_end();
 
 install_keyword_root("multipaths", &multipaths_handler);
< at >< at > -2639,5 +2718,6 < at >< at > init_keywords(void)
 install_keyword("uid", &mp_uid_handler, &snprint_mp_uid);
 install_keyword("gid", &mp_gid_handler, &snprint_mp_gid);
 install_keyword("reservation_key", &mp_reservation_key_handler, &snprint_mp_reservation_key);
+install_keyword("user_friendly_names", &mp_names_handler, &snprint_mp_user_friendly_names);
 install_sublevel_end();
 }
Index: multipath-tools-120518/libmultipath/propsel.c
===================================================================
--- multipath-tools-120518.orig/libmultipath/propsel.c
+++ multipath-tools-120518/libmultipath/propsel.c
< at >< at > -237,6 +237,18 < at >< at > select_alias_prefix (struct multipath *
 mp->wwid, mp->alias_prefix);
 }
 
+static int
+want_user_friendly_names(struct multipath * mp)
+{
+if (mp->mpe &&
+    mp->mpe->user_friendly_names != USER_FRIENDLY_NAMES_UNDEF)
+return (mp->mpe->user_friendly_names == USER_FRIENDLY_NAMES_ON);
+if (mp->hwe &&
+    mp->hwe->user_friendly_names != USER_FRIENDLY_NAMES_UNDEF)
+return (mp->hwe->user_friendly_names == USER_FRIENDLY_NAMES_ON);
+return (conf->user_friendly_names  == USER_FRIENDLY_NAMES_ON);
+}
+
 extern int
 select_alias (struct multipath * mp)
 {
< at >< at > -244,7 +256,7 < at >< at > select_alias (struct multipath * mp)
 mp->alias = STRDUP(mp->mpe->alias);
 else {
 mp->alias = NULL;
-if (conf->user_friendly_names) {
+if (want_user_friendly_names(mp)) {
 select_alias_prefix(mp);
 mp->alias = get_user_friendly_alias(mp->wwid,
 conf->bindings_file, mp->alias_prefix, conf->bindings_read_only);
Index: multipath-tools-120518/libmultipath/structs.h
===================================================================
--- multipath-tools-120518.orig/libmultipath/structs.h
+++ multipath-tools-120518/libmultipath/structs.h
< at >< at > -92,6 +92,12 < at >< at > enum log_checker_err_states {
 LOG_CHKR_ERR_ONCE,
 };
 
+enum user_friendly_names_states {
+USER_FRIENDLY_NAMES_UNDEF,
+USER_FRIENDLY_NAMES_OFF,
+USER_FRIENDLY_NAMES_ON,
+};
+
 struct scsi_idlun {
 int dev_id;
 int host_unique_id;

[PATCH] multipath: Make sure we store all the hwentryattributes.

Benjamin Marzinski — 2012-05-23T20:29:05

Not all of the attributes from the hardware table entries were getting stored
when the built-in devices configurations were being setup.

Signed-off-by: Benjamin Marzinski <bmarzins< at >redhat.com>
---
 libmultipath/config.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: multipath-tools-120518/libmultipath/config.c
===================================================================
--- multipath-tools-120518.orig/libmultipath/config.c
+++ multipath-tools-120518/libmultipath/config.c
< at >< at > -379,6 +379,10 < at >< at > store_hwe (vector hwtable, struct hwentr
 hwe->no_path_retry = dhwe->no_path_retry;
 hwe->minio = dhwe->minio;
 hwe->minio_rq = dhwe->minio_rq;
+hwe->pg_timeout = dhwe->pg_timeout;
+hwe->flush_on_last_del = dhwe->flush_on_last_del;
+hwe->fast_io_fail = dhwe->fast_io_fail;
+hwe->dev_loss = dhwe->dev_loss;
 
 if (dhwe->bl_product && !(hwe->bl_product = set_param_str(dhwe->bl_product)))
 goto out;

Re: [PATCH]] multipath-tools : Minor typo in multipath.conf.annotated

Christophe Varoqui — 2012-05-23T20:26:43

Applied,

Thanks.

does file system page cache work after device mapping?

zhiming shen — 2012-05-22T19:18:42

Hi everyone, I have some questions regarding to the mechanism of device
mapper: After the device mapper layer gets the mapping information, it will
issue IO request to the real device. How does it perform IO operation at
this stage? For example, if the device is a block device, will it go to the
page cache and check whether the desired block is in cache? If the block is
not in cache and it read from the physical device, will the block be cached
in memory?

Thanks very much!


Zhiming