lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 30 Apr 2021 10:14:09 +0800 From: Xiubo Li <xiubli@...hat.com> To: Prasanna Kumar Kalever <prasanna.kalever@...hat.com>, linux-kernel@...r.kernel.org Cc: linux-block@...r.kernel.org, nbd@...er.debian.org, josef@...icpanda.com, axboe@...nel.dk, idryomov@...hat.com Subject: Re: [PATCH] nbd: provide a way for userspace processes to identify device backends On 2021/4/29 18:28, Prasanna Kumar Kalever wrote: > Problem: > On reconfigure of device, there is no way to defend if the backend > storage is matching with the initial backend storage. > > Say, if an initial connect request for backend "pool1/image1" got > mapped to /dev/nbd0 and the userspace process is terminated. A next > reconfigure request within NBD_ATTR_DEAD_CONN_TIMEOUT is allowed to > use /dev/nbd0 for a different backend "pool1/image2" > > For example, an operation like below could be dangerous: > > $ sudo rbd-nbd map --try-netlink rbd-pool/ext4-image > /dev/nbd0 > $ sudo blkid /dev/nbd0 > /dev/nbd0: UUID="bfc444b4-64b1-418f-8b36-6e0d170cfc04" TYPE="ext4" > $ sudo pkill -9 rbd-nbd > $ sudo rbd-nbd attach --try-netlink --device /dev/nbd0 rbd-pool/xfs-image > /dev/nbd0 > $ sudo blkid /dev/nbd0 > /dev/nbd0: UUID="d29bf343-6570-4069-a9ea-2fa156ced908" TYPE="xfs" > > Solution: > Provide a way for userspace processes to keep some metadata to identify > between the device and the backend, so that when a reconfigure request is > made, we can compare and avoid such dangerous operations. > > With this solution, as part of the initial connect request, backend > path can be stored in the sysfs per device config, so that on a reconfigure > request it's easy to check if the backend path matches with the initial > connect backend path. > > Please note, ioctl interface to nbd will not have these changes, as there > won't be any reconfigure. > > Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@...hat.com> > --- > drivers/block/nbd.c | 60 +++++++++++++++++++++++++++++++- > include/uapi/linux/nbd-netlink.h | 1 + > 2 files changed, 60 insertions(+), 1 deletion(-) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 4ff71b579cfc..b5022187ad9c 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -79,6 +79,7 @@ struct link_dead_args { > #define NBD_RT_HAS_CONFIG_REF 4 > #define NBD_RT_BOUND 5 > #define NBD_RT_DISCONNECT_ON_CLOSE 6 > +#define NBD_RT_HAS_BACKEND_FILE 7 > > #define NBD_DESTROY_ON_DISCONNECT 0 > #define NBD_DISCONNECT_REQUESTED 1 > @@ -119,6 +120,8 @@ struct nbd_device { > > struct completion *destroy_complete; > unsigned long flags; > + > + char *backend; > }; > > #define NBD_CMD_REQUEUED 1 > @@ -216,6 +219,20 @@ static const struct device_attribute pid_attr = { > .show = pid_show, > }; > > +static ssize_t backend_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct gendisk *disk = dev_to_disk(dev); > + struct nbd_device *nbd = (struct nbd_device *)disk->private_data; > + > + return sprintf(buf, "%s\n", nbd->backend ?: ""); > +} > + > +static const struct device_attribute backend_attr = { > + .attr = { .name = "backend", .mode = 0444}, > + .show = backend_show, > +}; > + > static void nbd_dev_remove(struct nbd_device *nbd) > { > struct gendisk *disk = nbd->disk; > @@ -1215,6 +1232,12 @@ static void nbd_config_put(struct nbd_device *nbd) > &config->runtime_flags)) > device_remove_file(disk_to_dev(nbd->disk), &pid_attr); > nbd->task_recv = NULL; > + if (test_and_clear_bit(NBD_RT_HAS_BACKEND_FILE, > + &config->runtime_flags)) { > + device_remove_file(disk_to_dev(nbd->disk), &backend_attr); > + kfree(nbd->backend); > + nbd->backend = NULL; > + } > nbd_clear_sock(nbd); > if (config->num_connections) { > int i; > @@ -1274,7 +1297,7 @@ static int nbd_start_device(struct nbd_device *nbd) > > error = device_create_file(disk_to_dev(nbd->disk), &pid_attr); > if (error) { > - dev_err(disk_to_dev(nbd->disk), "device_create_file failed!\n"); > + dev_err(disk_to_dev(nbd->disk), "device_create_file failed for pid!\n"); > return error; > } > set_bit(NBD_RT_HAS_PID_FILE, &config->runtime_flags); > @@ -1681,6 +1704,7 @@ static int nbd_dev_add(int index) > BLK_MQ_F_BLOCKING; > nbd->tag_set.driver_data = nbd; > nbd->destroy_complete = NULL; > + nbd->backend = NULL; > > err = blk_mq_alloc_tag_set(&nbd->tag_set); > if (err) > @@ -1754,6 +1778,7 @@ static const struct nla_policy nbd_attr_policy[NBD_ATTR_MAX + 1] = { > [NBD_ATTR_SOCKETS] = { .type = NLA_NESTED}, > [NBD_ATTR_DEAD_CONN_TIMEOUT] = { .type = NLA_U64 }, > [NBD_ATTR_DEVICE_LIST] = { .type = NLA_NESTED}, > + [NBD_ATTR_BACKEND_IDENTIFIER] = { .type = NLA_STRING}, > }; > > static const struct nla_policy nbd_sock_policy[NBD_SOCK_MAX + 1] = { > @@ -1956,6 +1981,23 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) > } > } > ret = nbd_start_device(nbd); > + if (ret) > + goto out; > + if (info->attrs[NBD_ATTR_BACKEND_IDENTIFIER]) { > + nbd->backend = nla_strdup(info->attrs[NBD_ATTR_BACKEND_IDENTIFIER], > + GFP_KERNEL); > + if (!nbd->backend) { > + ret = -ENOMEM; > + goto out; > + } > + } > + ret = device_create_file(disk_to_dev(nbd->disk), &backend_attr); > + if (ret) { > + dev_err(disk_to_dev(nbd->disk), > + "device_create_file failed for backend!\n"); > + goto out; > + } > + set_bit(NBD_RT_HAS_BACKEND_FILE, &config->runtime_flags); > out: > mutex_unlock(&nbd->config_lock); > if (!ret) { > @@ -2048,6 +2090,22 @@ static int nbd_genl_reconfigure(struct sk_buff *skb, struct genl_info *info) > index); > return -EINVAL; > } > + if (nbd->backend) { > + if (info->attrs[NBD_ATTR_BACKEND_IDENTIFIER]) { > + if (nla_strcmp(info->attrs[NBD_ATTR_BACKEND_IDENTIFIER], > + nbd->backend)) { > + mutex_unlock(&nbd_index_mutex); > + dev_err(nbd_to_dev(nbd), > + "backend image doesn't match with %s\n", > + nbd->backend); > + return -EINVAL; > + } > + } else { > + mutex_unlock(&nbd_index_mutex); > + dev_err(nbd_to_dev(nbd), "must specify backend\n"); > + return -EINVAL; > + } > + } > if (!refcount_inc_not_zero(&nbd->refs)) { > mutex_unlock(&nbd_index_mutex); > printk(KERN_ERR "nbd: device at index %d is going down\n", > diff --git a/include/uapi/linux/nbd-netlink.h b/include/uapi/linux/nbd-netlink.h > index c5d0ef7aa7d5..2d0b90964227 100644 > --- a/include/uapi/linux/nbd-netlink.h > +++ b/include/uapi/linux/nbd-netlink.h > @@ -35,6 +35,7 @@ enum { > NBD_ATTR_SOCKETS, > NBD_ATTR_DEAD_CONN_TIMEOUT, > NBD_ATTR_DEVICE_LIST, > + NBD_ATTR_BACKEND_IDENTIFIER, > __NBD_ATTR_MAX, > }; > #define NBD_ATTR_MAX (__NBD_ATTR_MAX - 1) Reviewed-by: Xiubo Li <xiubli@...hat.com>
Powered by blists - more mailing lists