| Message ID | 20200904091005.7537-1-idryomov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rbd: require global CAP_SYS_ADMIN for mapping and unmapping | expand |
On Fri, 2020-09-04 at 11:10 +0200, Ilya Dryomov wrote: > It turns out that currently we rely only on sysfs attribute > permissions: > > $ ll /sys/bus/rbd/{add*,remove*} > --w------- 1 root root 4096 Sep 3 20:37 /sys/bus/rbd/add > --w------- 1 root root 4096 Sep 3 20:37 /sys/bus/rbd/add_single_major > --w------- 1 root root 4096 Sep 3 20:37 /sys/bus/rbd/remove > --w------- 1 root root 4096 Sep 3 20:38 /sys/bus/rbd/remove_single_major > > This means that images can be mapped and unmapped (i.e. block devices > can be created and deleted) by a UID 0 process even after it drops all > privileges or by any process with CAP_DAC_OVERRIDE in its user namespace > as long as UID 0 is mapped into that user namespace. > > Be consistent with other virtual block devices (loop, nbd, dm, md, etc) > and require CAP_SYS_ADMIN in the initial user namespace for mapping and > unmapping, and also for dumping the configuration string and refreshing > the image header. > > Cc: stable@vger.kernel.org > Signed-off-by: Ilya Dryomov <idryomov@gmail.com> > --- > drivers/block/rbd.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > index fa252e8ed276..180587ce606c 100644 > --- a/drivers/block/rbd.c > +++ b/drivers/block/rbd.c > @@ -5120,6 +5120,9 @@ static ssize_t rbd_config_info_show(struct device *dev, > { > struct rbd_device *rbd_dev = dev_to_rbd_dev(dev); > > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > return sprintf(buf, "%s\n", rbd_dev->config_info); > } > > @@ -5231,6 +5234,9 @@ static ssize_t rbd_image_refresh(struct device *dev, > struct rbd_device *rbd_dev = dev_to_rbd_dev(dev); > int ret; > > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > ret = rbd_dev_refresh(rbd_dev); > if (ret) > return ret; > @@ -7059,6 +7065,9 @@ static ssize_t do_rbd_add(struct bus_type *bus, > struct rbd_client *rbdc; > int rc; > > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > if (!try_module_get(THIS_MODULE)) > return -ENODEV; > > @@ -7209,6 +7218,9 @@ static ssize_t do_rbd_remove(struct bus_type *bus, > bool force = false; > int ret; > > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > dev_id = -1; > opt_buf[0] = '\0'; > sscanf(buf, "%d %5s", &dev_id, opt_buf); Good catch! Reviewed-by: Jeff Layton <jlayton@kernel.org>
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index fa252e8ed276..180587ce606c 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -5120,6 +5120,9 @@ static ssize_t rbd_config_info_show(struct device *dev, { struct rbd_device *rbd_dev = dev_to_rbd_dev(dev); + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + return sprintf(buf, "%s\n", rbd_dev->config_info); } @@ -5231,6 +5234,9 @@ static ssize_t rbd_image_refresh(struct device *dev, struct rbd_device *rbd_dev = dev_to_rbd_dev(dev); int ret; + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + ret = rbd_dev_refresh(rbd_dev); if (ret) return ret; @@ -7059,6 +7065,9 @@ static ssize_t do_rbd_add(struct bus_type *bus, struct rbd_client *rbdc; int rc; + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + if (!try_module_get(THIS_MODULE)) return -ENODEV; @@ -7209,6 +7218,9 @@ static ssize_t do_rbd_remove(struct bus_type *bus, bool force = false; int ret; + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + dev_id = -1; opt_buf[0] = '\0'; sscanf(buf, "%d %5s", &dev_id, opt_buf);
It turns out that currently we rely only on sysfs attribute permissions: $ ll /sys/bus/rbd/{add*,remove*} --w------- 1 root root 4096 Sep 3 20:37 /sys/bus/rbd/add --w------- 1 root root 4096 Sep 3 20:37 /sys/bus/rbd/add_single_major --w------- 1 root root 4096 Sep 3 20:37 /sys/bus/rbd/remove --w------- 1 root root 4096 Sep 3 20:38 /sys/bus/rbd/remove_single_major This means that images can be mapped and unmapped (i.e. block devices can be created and deleted) by a UID 0 process even after it drops all privileges or by any process with CAP_DAC_OVERRIDE in its user namespace as long as UID 0 is mapped into that user namespace. Be consistent with other virtual block devices (loop, nbd, dm, md, etc) and require CAP_SYS_ADMIN in the initial user namespace for mapping and unmapping, and also for dumping the configuration string and refreshing the image header. Cc: stable@vger.kernel.org Signed-off-by: Ilya Dryomov <idryomov@gmail.com> --- drivers/block/rbd.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)