blktrace: require CAP_SYS_ADMIN for the BLKTRACE ioctls

PATCH Superseded Tao Cui 2 participants 3 messages Nov 6 – Nov 9

Subsystems (2)

Block layer Tracing

lore

Share & tools

lore

Tao Cui

author

2026-11-06 14:29 UTC #0 lore raw

PATCH 1/1 blktrace: require CAP_SYS_ADMIN for the BLKTRACE ioctls

From: Tao Cui <[email protected]>

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 <[email protected]>

---
 kernel/trace/blktrace.c | 3 +++
 1 file changed, 3 insertions(+)

kernel/trace/blktrace.c +3 -0

index 8cd2520b4c99..c1bf8ca350fe 100644
@@ -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);

Signature · 1 line

2.43.0

[email protected]

AI review

2026-11-06 14:39 UTC #1 lore raw

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

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 <[email protected]>

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.

Quoted 5 lines

kernel/trace/blktrace.c +0 -0
index 8cd2520b4c99e..c1bf8ca350fee 100644

kernel/trace/blktrace.c int blk_trace_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg) · line 935

Quoted 9 lines

kernel/trace/blktrace.c +3 -0

@@ -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.

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Tao Cui

author

2026-11-09 02:23 UTC #2 lore raw

Hi，

Quoted 47 lines · 在 2026/11/6 22:39, [email protected] 写道:

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 <[email protected]>
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.
Quoted 5 lines
kernel/trace/blktrace.c +0 -0
index 8cd2520b4c99e..c1bf8ca350fee 100644
kernel/trace/blktrace.c int blk_trace_ioctl(struct block_device *bdev, unsigned cmd, char __user *arg) · line 935
Quoted 9 lines
kernel/trace/blktrace.c +3 -0
@@ -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);
    ...
}

Quoted 5 lines

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/[email protected]/

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>