[Devel] [PATCH vz10 v4 2/2] fs: enforce cgroup permissions for bdevs on mount

[Konstantin Khorenko]

On 11/18/25 04:28, Pavel Tikhomirov wrote:
> 
> 
> On 11/18/25 07: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 <aleksey.oladko at virtuozzo.com>
>> ---
>>   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);
> 
> Oh, I noticed one more thing, should be also add the same check in mount_bdev()? (if container will try to mount something which is still on the old mount api)

Yeah, something like

      1 │--- a/fs/super.c
      2 │+++ b/fs/super.c
      3 │@@ -1710,6 +1710,7 @@ struct dentry *mount_bdev(struct file_system_type *fs_type,
      4 │ {
      5 │    struct super_block *s;
      6 │    int error;
      7 │+   blk_mode_t mode;
      8 │    dev_t dev;
      9 │
     10 │    error = lookup_bdev(dev_name, &dev);
     11 │    if (error)
     12 │        return ERR_PTR(error);
     13 │
     14 │+   /* Check cgroup permissions before getting super_block */
     15 │+   mode = sb_open_mode(flags) | BLK_OPEN_MOUNT;
     16 │+   error = bdev_permission(dev, mode, NULL);
     17 │+   if (error)
     18 │+       return ERR_PTR(error);
     19 │+
     20 │    flags |= SB_NOSEC;
     21 │    s = sget(fs_type, test_bdev_super, set_bdev_super, flags, &dev);
     22 │    if (IS_ERR(s))

+ do we have a test for this?

If not, please create a task to write a kselftest for this for different filesystems - which use both 
mounting ways - mount_bdev() and sget_dev().

> 
>>   	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) ({						\
>