[PATCH] blktrace: require CAP_SYS_ADMIN for the BLKTRACE ioctls

[PATCH] blktrace: require CAP_SYS_ADMIN for the BLKTRACE ioctls

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

BLKTRACESETUP allocates user-sized relay buffers and sets up a trace
that outlives the creating fd.  Every command of the interface,
BLKTRACESETUP, BLKTRACESETUP2, BLKTRACESTART, BLKTRACESTOP,
BLKTRACETEARDOWN and the 32-bit variant, can be issued by anyone who
can open the block device node.  The sg passthrough path
(sg_ioctl_common) reaches the same code with an sg char device, so a
container that is handed either node can hold trace state and relay
memory on shared devices.  The trace data itself stays root-only: the
debugfs relay files are not readable past root.

Gate the whole family behind CAP_SYS_ADMIN.

Verified on next-20261104 under qemu: with the patch, BLKTRACESETUP
with all capabilities dropped is rejected with EPERM; without the
patch the same call succeeds.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 kernel/trace/blktrace.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 8cd2520b4c99..c1bf8ca350fe 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -927,6 +927,9 @@ int blk_trace_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg)
 	int ret, start = 0;
 	char b[BDEVNAME_SIZE];
 
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
 	switch (cmd) {
 	case BLKTRACESETUP2:
 		snprintf(b, sizeof(b), "%pg", bdev);
-- 
2.43.0

Re: [PATCH] blktrace: require CAP_SYS_ADMIN for the BLKTRACE ioctls

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] blktrace: missing CAP_SYS_ADMIN check in sg_ioctl_common()
--

commit 8392196724a6f06dfb860d63cf54fba3357cc04e
Author: Tao Cui <cuitao@kylinos.cn>

blktrace: require CAP_SYS_ADMIN for the BLKTRACE ioctls

This commit adds a CAP_SYS_ADMIN capability check to blk_trace_ioctl() to
restrict block tracing ioctls to privileged users.

> diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
> index 8cd2520b4c99e..c1bf8ca350fee 100644
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c
> @@ -927,6 +927,9 @@ int blk_trace_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg)
>  	int ret, start = 0;
>  	char b[BDEVNAME_SIZE];
>  
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
>  	switch (cmd) {
>  	case BLKTRACESETUP2:
>  		snprintf(b, sizeof(b), "%pg", bdev);

[Severity: Medium]
Does checking CAP_SYS_ADMIN only in blk_trace_ioctl() leave the sg
passthrough path in sg_ioctl_common() ungated?

drivers/scsi/sg.c:sg_ioctl_common() {
    ...
	case BLKTRACESETUP:
		return blk_trace_setup(sdp->device->request_queue, sdp->name,
				       MKDEV(SCSI_GENERIC_MAJOR, sdp->index),
				       NULL, p);
	case BLKTRACESTART:
		return blk_trace_startstop(sdp->device->request_queue, 1);
	case BLKTRACESTOP:
		return blk_trace_startstop(sdp->device->request_queue, 0);
	case BLKTRACETEARDOWN:
		return blk_trace_remove(sdp->device->request_queue);
    ...
}

Because sg_ioctl_common() calls blk_trace_setup(), blk_trace_startstop()
and blk_trace_remove() directly rather than going through
blk_trace_ioctl(), an unprivileged process with access to an /dev/sg* node
can still allocate persistent relay buffers and control tracing state on
the underlying request_queue.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106142852.3233950-1-cui.tao@linux.dev?part=1

Re: [PATCH] blktrace: require CAP_SYS_ADMIN for the BLKTRACE ioctls

[Tao Cui]

Hi，

在 2026/11/6 22:39, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] blktrace: missing CAP_SYS_ADMIN check in sg_ioctl_common()
> --
> 
> commit 8392196724a6f06dfb860d63cf54fba3357cc04e
> Author: Tao Cui <cuitao@kylinos.cn>
> 
> blktrace: require CAP_SYS_ADMIN for the BLKTRACE ioctls
> 
> This commit adds a CAP_SYS_ADMIN capability check to blk_trace_ioctl() to
> restrict block tracing ioctls to privileged users.
> 
>> diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
>> index 8cd2520b4c99e..c1bf8ca350fee 100644
>> --- a/kernel/trace/blktrace.c
>> +++ b/kernel/trace/blktrace.c
>> @@ -927,6 +927,9 @@ int blk_trace_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg)
>>  	int ret, start = 0;
>>  	char b[BDEVNAME_SIZE];
>>  
>> +	if (!capable(CAP_SYS_ADMIN))
>> +		return -EPERM;
>> +
>>  	switch (cmd) {
>>  	case BLKTRACESETUP2:
>>  		snprintf(b, sizeof(b), "%pg", bdev);
> 
> [Severity: Medium]
> Does checking CAP_SYS_ADMIN only in blk_trace_ioctl() leave the sg
> passthrough path in sg_ioctl_common() ungated?
> 
> drivers/scsi/sg.c:sg_ioctl_common() {
>     ...
> 	case BLKTRACESETUP:
> 		return blk_trace_setup(sdp->device->request_queue, sdp->name,
> 				       MKDEV(SCSI_GENERIC_MAJOR, sdp->index),
> 				       NULL, p);
> 	case BLKTRACESTART:
> 		return blk_trace_startstop(sdp->device->request_queue, 1);
> 	case BLKTRACESTOP:
> 		return blk_trace_startstop(sdp->device->request_queue, 0);
> 	case BLKTRACETEARDOWN:
> 		return blk_trace_remove(sdp->device->request_queue);
>     ...
> }
> 
> Because sg_ioctl_common() calls blk_trace_setup(), blk_trace_startstop()
> and blk_trace_remove() directly rather than going through
> blk_trace_ioctl(), an unprivileged process with access to an /dev/sg* node
> can still allocate persistent relay buffers and control tracing state on
> the underlying request_queue.
Thanks for catching this.

The sg path is covered by patch 1/7 of the memcg accounting series:
https://lore.kernel.org/all/20261106131829.3228926-2-cui.tao@linux.dev/

Originally these changes were part of a single series.  Since the
blktrace capability check isn't itself a memcg accounting fix, I split
it out into a separate series, but the corresponding sg_ioctl_common()
change remained with the memcg patches.

You're right that the standalone blktrace patch is incomplete by
itself.  I'll fold the sg-side check into the blktrace patch in the
next revision so it is self-contained.

Thanks>