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

[Tao Cui <cui.tao@linux.dev>]

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>