[Devel] [PATCH hci-8.1] block: let container root set the block size of and flush its device

[Devel] [PATCH hci-8.1] block: let container root set the block size of and flush its device

[Eva Kurchatova]

BLKBSZSET and BLKFLSBUF check CAP_SYS_ADMIN in the initial user
namespace, so root in a container cannot set the block size of a block
device it has been given, nor flush its buffers, and mkfs.xfs on a loop
device there stops with "cannot set blocksize 512 on block device
/dev/loopN: Permission denied", or with "Flushing the data device
failed, err=13" once it gets past the first.

Check the capability relative to the container, as the other block
code adapted for containers does. The device itself is reachable only
through the device cgroup, set_blocksize() still needs the device
exclusively, and a flush discards only cached pages of that device, so
nothing a container could not already write to is affected.

https://virtuozzo.atlassian.net/browse/VSTOR-137668
Feature: ve: allow various syscalls in CT root userns

Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
---
 block/ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index a3c53b04e1e3..14047cd8e611 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -419,7 +419,7 @@ static int blkdev_pr_clear(struct block_device *bdev, blk_mode_t mode,
 static int blkdev_flushbuf(struct block_device *bdev, unsigned cmd,
 		unsigned long arg)
 {
-	if (!capable(CAP_SYS_ADMIN))
+	if (!ve_capable(CAP_SYS_ADMIN))
 		return -EACCES;
 
 	mutex_lock(&bdev->bd_holder_lock);
@@ -531,7 +531,7 @@ static int blkdev_bszset(struct file *file, blk_mode_t mode,
 	struct file *excl_file;
 	int ret, n;
 
-	if (!capable(CAP_SYS_ADMIN))
+	if (!ve_capable(CAP_SYS_ADMIN))
 		return -EACCES;
 	if (!argp)
 		return -EINVAL;
-- 
2.56.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1] block: let container root set the block size of and flush its device

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] block: blkdev_flushbuf() relaxed to ve_capable() with no write or exclusive check
--

commit 4af19ef77a12f7b83b4ce6c7194a3cac87d71388
Author: Eva Kurchatova <eva.kurchatova@virtuozzo.com>

block: let container root set the block size of and flush its device

This patch changes the BLKBSZSET and BLKFLSBUF ioctls to check
CAP_SYS_ADMIN with ve_capable() instead of capable(). Root in a container
can then set the block size of, and flush, a block device it has been
given.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-137668

> diff --git a/block/ioctl.c b/block/ioctl.c
> index a3c53b04e1e34..14047cd8e6116 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -419,7 +419,7 @@ static int blkdev_pr_clear(struct block_device *bdev, blk_mode_t mode,
>  static int blkdev_flushbuf(struct block_device *bdev, unsigned cmd,
>  		unsigned long arg)
>  {
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!ve_capable(CAP_SYS_ADMIN))
>  		return -EACCES;
>  
>  	mutex_lock(&bdev->bd_holder_lock);
>  	if (bdev->bd_holder_ops && bdev->bd_holder_ops->sync)
>  		bdev->bd_holder_ops->sync(bdev);
>  	else {
>  		mutex_unlock(&bdev->bd_holder_lock);
>  		sync_blockdev(bdev);
>  	}
>  
>  	invalidate_bdev(bdev);

[Severity: Medium]
Can this relaxation be reached through a read-only fd? In
blkdev_common_ioctl() the dispatch is:

    case BLKFLSBUF:
            return blkdev_flushbuf(bdev, cmd, arg);

No mode is passed down, so blkdev_flushbuf() has no FMODE_WRITE check and
takes no exclusive claim. After the ve_capable() test it goes straight to
bd_holder_ops->sync(bdev) or sync_blockdev(bdev), and then
invalidate_bdev(bdev).

With CONFIG_VE, ve_capable() evaluates ns_capable() against the container's
user namespace. Root in the container could therefore open a device that the
device cgroup allows read-only, including one held by a filesystem mounted
outside the container. It could then force writeback, invoke the holder's
->sync() op, and drop the clean page cache of that device.

The commit message says "a flush discards only cached pages of that device,
so nothing a container could not already write to is affected". Is that
accurate when the container only has read access through the device cgroup?
That statement also doesn't cover the holder ->sync() side effect.

Unlike BLKBSZSET, where set_blocksize() still requires an exclusive open,
nothing compensates for the relaxation here. Would it make sense to require
FMODE_WRITE (passing mode into blkdev_flushbuf()) when the capability is
only held in the container's user namespace?

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261021175410.2339797-1-eva.kurchatova@virtuozzo.com?part=1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1] block: let container root set the block size of and flush its device

[Vladimir Riabchun]

On 10/21/26 19:54, Eva Kurchatova wrote:
> BLKBSZSET and BLKFLSBUF check CAP_SYS_ADMIN in the initial user
> namespace, so root in a container cannot set the block size of a block
> device it has been given, nor flush its buffers, and mkfs.xfs on a loop
> device there stops with "cannot set blocksize 512 on block device
> /dev/loopN: Permission denied", or with "Flushing the data device
> failed, err=13" once it gets past the first.
> 
> Check the capability relative to the container, as the other block
> code adapted for containers does. The device itself is reachable only
> through the device cgroup, set_blocksize() still needs the device
> exclusively, and a flush discards only cached pages of that device, so
> nothing a container could not already write to is affected.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-137668
> Feature: ve: allow various syscalls in CT root userns
> 
> Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
> ---
>   block/ioctl.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/block/ioctl.c b/block/ioctl.c
> index a3c53b04e1e3..14047cd8e611 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -419,7 +419,7 @@ static int blkdev_pr_clear(struct block_device *bdev, blk_mode_t mode,
>   static int blkdev_flushbuf(struct block_device *bdev, unsigned cmd,
>   		unsigned long arg)
>   {
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!ve_capable(CAP_SYS_ADMIN))

Is it possible that some evil CT user can spam flush, keep mutex most
of the time and prevent some host actions?
I'm mostly talking about ploop devices passed by vzctl.

>   		return -EACCES;
>   
>   	mutex_lock(&bdev->bd_holder_lock);
> @@ -531,7 +531,7 @@ static int blkdev_bszset(struct file *file, blk_mode_t mode,
>   	struct file *excl_file;
>   	int ret, n;
>   
> -	if (!capable(CAP_SYS_ADMIN))
> +	if (!ve_capable(CAP_SYS_ADMIN))

The same question regarding hdd disks from vzctl: can't we break them?

>   		return -EACCES;
>   	if (!argp)
>   		return -EINVAL;

I'll check tomorrow what nasty thing could be done with them.

-- 
Best regards, Riabchun Vladimir
Linux Kernel Developer, Virtuozzo

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1] block: let container root set the block size of and flush its device

[Eva Kurchatova (Virtuozzo)]

On 10/21/26 23:56, Vladimir Riabchun wrote:
>
>
> On 10/21/26 19:54, Eva Kurchatova wrote:
>> BLKBSZSET and BLKFLSBUF check CAP_SYS_ADMIN in the initial user
>> namespace, so root in a container cannot set the block size of a block
>> device it has been given, nor flush its buffers, and mkfs.xfs on a loop
>> device there stops with "cannot set blocksize 512 on block device
>> /dev/loopN: Permission denied", or with "Flushing the data device
>> failed, err=13" once it gets past the first.
>>
>> Check the capability relative to the container, as the other block
>> code adapted for containers does. The device itself is reachable only
>> through the device cgroup, set_blocksize() still needs the device
>> exclusively, and a flush discards only cached pages of that device, so
>> nothing a container could not already write to is affected.
>>
>> https://virtuozzo.atlassian.net/browse/VSTOR-137668
>> Feature: ve: allow various syscalls in CT root userns
>>
>> Signed-off-by: Eva Kurchatova <eva.kurchatova@virtuozzo.com>
>> ---
>>   block/ioctl.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/ioctl.c b/block/ioctl.c
>> index a3c53b04e1e3..14047cd8e611 100644
>> --- a/block/ioctl.c
>> +++ b/block/ioctl.c
>> @@ -419,7 +419,7 @@ static int blkdev_pr_clear(struct block_device 
>> *bdev, blk_mode_t mode,
>>   static int blkdev_flushbuf(struct block_device *bdev, unsigned cmd,
>>           unsigned long arg)
>>   {
>> -    if (!capable(CAP_SYS_ADMIN))
>> +    if (!ve_capable(CAP_SYS_ADMIN))
>
> Is it possible that some evil CT user can spam flush, keep mutex most
> of the time and prevent some host actions?
> I'm mostly talking about ploop devices passed by vzctl.
>
I would imagine that the mutex fairness would not allow this,

given that the mutex enforces a wait queue. However, this could

lead to undesirable IO latency on the host, which is possible in

many other ways, esp. if the host uses a spinning disk.

Not allowing guests to flush properly can violate ACID consistency.

>>           return -EACCES;
>>         mutex_lock(&bdev->bd_holder_lock);
>> @@ -531,7 +531,7 @@ static int blkdev_bszset(struct file *file, 
>> blk_mode_t mode,
>>       struct file *excl_file;
>>       int ret, n;
>>   -    if (!capable(CAP_SYS_ADMIN))
>> +    if (!ve_capable(CAP_SYS_ADMIN))
>
> The same question regarding hdd disks from vzctl: can't we break them?
>
I'll look more into it, but generally the same points as above for now.

Best regards.

>>           return -EACCES;
>>       if (!argp)
>>           return -EINVAL;
>
> I'll check tomorrow what nasty thing could be done with them.
>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1] block: let container root set the block size of and flush its device

[Konstantin Khorenko]

> Is it possible that some evil CT user can spam flush, keep mutex most
> of the time and prevent some host actions?

No. bd_holder_lock is held only across the holder pointer test. The
->sync branch goes through bdev_super_lock(), which drops it before
taking s_umount and running sync_filesystem(). The other branch unlocks
before sync_blockdev(). Nothing heavy runs under it, so mount, umount,
freeze and mark_dead on the host are not affected.

> I'm mostly talking about ploop devices passed by vzctl.

In v2 those never reach the ioctl body. libvzctl configure_disk_perm()
grants CT disks (root ploop, secondary ploop, physical hdd via
use_device) r + W, plus M for non-root. W is "write only for mount": the
BPF program in bpf/cgroup_dev.c passes WRITE only when the request also
carries MOUNT. bdev_permission(BLK_OPEN_WRITE) has no MOUNT bit, so
blkdev_admin_capable() fails and the CT gets -EACCES, same as today.

> The same question regarding hdd disks from vzctl: can't we break them?

Even with a plain write grant, no. They are mounted by the host.
BLKFLSBUF hits the fs holder and v2 returns -EBUSY there. BLKBSZSET
cannot get the exclusive claim the mount holds and returns -EBUSY by the
unchanged upstream logic.

What v1 actually allowed on the CT's own ploop was sync plus
invalidate_bdev of the device under its own filesystem, i.e. what
syncfs() on its root already gives it. v2 closes even that at the
device cgroup level.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel