On 11/19/25 4:44 PM, Konstantin Khorenko wrote:
On 11/18/25 00:14, Aleksei Oladko wrote:
In mount operations, cgroup device access permissions are checked only
when a new super_block is created. If the device has already been mounted,
no permission check is performed since the super_block already exists.
As a result, a process belonging to a cgroup without access permissions
to the device can still perform a mount operation successfully.

To reproduce:
  # mount /dev/loop0 /mnt/test0
  # echo $$ > /sys/fs/cgroup/machine.slice/CTID/cgroup.procs
  # mount /dev/loop0 /mnt/test1
  #

If the device was not mounted before, the mount operation fails
as expected:
  # echo $$ > /sys/fs/cgroup/machine.slice/CTID/cgroup.procs
  # mount /dev/loop0 /mnt/test1
mount: /mnt/test1: permission denied.

This patch adds a cgroup permission check for the mounting process
before obtaining the device's super_block. As a result, even if the
device has already been mounted and the super_block exists, the mount
operation will be denied when the process lacks the appropriate cgroup
permissions.

v2: removed redundant cgroup permission check when creating a new
superblock in setup_dbev_super.
v3: no changes
v4: revert v2

https://virtuozzo.atlassian.net/browse/VSTOR-105978

Signed-off-by: Aleksei Oladko <[email protected]>
---
  block/blk.h            |  1 -
  fs/super.c             | 14 +++++++++++++-
  include/linux/blkdev.h |  1 +
  include/linux/fs.h     |  1 +
  4 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/block/blk.h b/block/blk.h
index c718e4291db0..8610d87e5050 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -727,7 +727,6 @@ static inline void bio_issue_init(struct bio_issue *issue,
  void bdev_release(struct file *bdev_file);
  int bdev_open(struct block_device *bdev, blk_mode_t mode, void *holder,
            const struct blk_holder_ops *hops, struct file *bdev_file);
-int bdev_permission(dev_t dev, blk_mode_t mode, void *holder);
    void blk_integrity_generate(struct bio *bio);
  void blk_integrity_verify(struct bio *bio);
diff --git a/fs/super.c b/fs/super.c
index faf37790544e..6a2a88f07c12 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1407,6 +1407,18 @@ struct super_block *sget_dev(struct fs_context *fc, dev_t dev)
  }
  EXPORT_SYMBOL(sget_dev);
  +struct super_block *sget_dev_check(struct fs_context *fc, dev_t dev)
+{
+    int ret;
+
+    blk_mode_t mode = sb_open_mode(fc->sb_flags) | BLK_OPEN_MOUNT;
+    ret = bdev_permission(dev, mode, fc);
+    if (ret)
+        return ERR_PTR(ret);
+    return sget_dev(fc, dev);
+}
+EXPORT_SYMBOL(sget_dev_check);
+
  #ifdef CONFIG_BLOCK
  /*
   * Lock the superblock that is holder of the bdev. Returns the superblock
@@ -1664,7 +1676,7 @@ int get_tree_bdev_flags(struct fs_context *fc,
          return error;
      }
      fc->sb_flags |= SB_NOSEC;
-    s = sget_dev(fc, dev);
+    s = sget_dev_check(fc, dev);

Please, leave the original function name as a wrapper and rename the original function to, say, _sget_dev().
s/sget_dev_check/sget_dev/
s/sget_dev/_sget_dev/

The rationale: imagine the kernel will have another place with call to sget_dev(). We never catch the moment such a new call appears, and thus the new place will skip the BLK_OPEN_MOUNT check.

On the other hand - in case the generic func sget_dev() becomes a wrapper - the BLK_OPEN_MOUNT will be in place for any new call.
get_dev is called from mtd_get_sb, and cgroup permission checks are not needed there, so a new function was introduced

      if (IS_ERR(s))
          return PTR_ERR(s);
  diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 64c28dbb44b4..4a66f6aa0441 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1666,6 +1666,7 @@ struct file *bdev_file_open_by_path(const char *path, blk_mode_t mode,
  int bd_prepare_to_claim(struct block_device *bdev, void *holder,
          const struct blk_holder_ops *hops);
  void bd_abort_claiming(struct block_device *bdev, void *holder);
+int bdev_permission(dev_t dev, blk_mode_t mode, void *holder);
    /* just for blk-cgroup, don't use elsewhere */
  struct block_device *blkdev_get_no_open(dev_t dev);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index b6f7809cad29..1f535909524c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2608,6 +2608,7 @@ struct super_block *sget(struct file_system_type *type,
              int (*set)(struct super_block *,void *),
              int flags, void *data);
  struct super_block *sget_dev(struct fs_context *fc, dev_t dev);
+struct super_block *sget_dev_check(struct fs_context *fc, dev_t dev);
    /* Alas, no aliases. Too much hassle with bringing module.h everywhere */
  #define fops_get(fops) ({                        \

_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to