* [PATCH] rbd: require global CAP_SYS_ADMIN for mapping and unmapping
@ 2020-09-04 9:10 Ilya Dryomov
2020-09-04 12:27 ` Jeff Layton
0 siblings, 1 reply; 2+ messages in thread
From: Ilya Dryomov @ 2020-09-04 9:10 UTC (permalink / raw)
To: ceph-devel
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);
--
2.19.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] rbd: require global CAP_SYS_ADMIN for mapping and unmapping
2020-09-04 9:10 [PATCH] rbd: require global CAP_SYS_ADMIN for mapping and unmapping Ilya Dryomov
@ 2020-09-04 12:27 ` Jeff Layton
0 siblings, 0 replies; 2+ messages in thread
From: Jeff Layton @ 2020-09-04 12:27 UTC (permalink / raw)
To: Ilya Dryomov, ceph-devel
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>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-09-04 12:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04 9:10 [PATCH] rbd: require global CAP_SYS_ADMIN for mapping and unmapping Ilya Dryomov
2020-09-04 12:27 ` Jeff Layton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.