[RFC] scsi: handling negative CDROM_SEND_PACKET timeouts

[RFC] scsi: handling negative CDROM_SEND_PACKET timeouts

[Wang Yingjie]

Hi,

CDROM_SEND_PACKET accepts a negative cgc.timeout and converts it into a
very long request timeout. Before sending a final fix, I'd like to check
whether rejecting negative values would break an existing userspace
convention: libcdio describes -1 as the default timeout and passes it
through to this ioctl on Linux.

scsi_cdrom_send_packet() passes the signed timeout to clock_t_to_jiffies(),
which takes an unsigned long. On x86-64 with HZ=1000 and USER_HZ=100, -5
produces ULONG_MAX, is stored back as -1 in cgc.timeout, and becomes
UINT_MAX milliseconds in the SG_IO header. The subsequent conversion and
assignment produce rq->timeout = 4294967294 jiffies, nominally about 49.7
days. This is a source-derived timeout, not a measured 49-day hang. The
unchecked conversion is still present at mainline commit
c03c1a14507dfaf8b856c2608f9ac216021fc222.

A QEMU test on 4056b3c9d3c92ea45955641dbafeb9b960e146ef used a malformed
packet with the allowed REQUEST SENSE opcode and timeout -5. The caller
was UID 65534 with no effective capabilities, after being granted access
to /dev/sr0. At the eight-second observation point it was in D state in:

  sr_block_ioctl -> scsi_ioctl -> scsi_cdrom_send_packet
    -> sg_io -> blk_execute_rq

The device-event worker was also blocked. Source reproducer, isolated
guest setup and logs are available privately on request. SG_IO already
allows the same opener to request a large timeout, so this does not by
itself establish a new privilege-boundary bypass.

The candidate below rejects negative values before the conversion, for
both native and compat callers. Earlier testing on Linux 6.12.80 recorded
a hung-task report with the minimized negative-timeout input, but no
hung-task report over a six-minute window with this guard. A timeout=1
control also completed that window without a hung-task report. The
same-base mainline before/after test has not yet been run.

The compatibility concern is in libcdio's mmc_run_cmd() and
run_mmc_cmd_linux() [1,2]. Their comments describe -1 as selecting a
default timeout, but the unsigned argument reaches cgc.timeout without
normalization when the timeout field is enabled. The kernel does not
currently give -1 that default meaning, but rejecting it could also
reject commands that complete normally today. I have not established
whether a particular application relies on this value.

Would rejecting negative values be acceptable here, or should -1 remain
accepted, for example by mapping it to the default timeout?

[1] https://github.com/libcdio/libcdio/blob/master/lib/driver/mmc/mmc.c
[2] https://github.com/libcdio/libcdio/blob/master/lib/driver/gnu_linux.c

Thanks,
Yingjie

---
Candidate for discussion, not a final patch for merging. The unchecked
conversion is already present in the initial Git import; no later refactor
is claimed as its introducing commit. This diff applies to mainline
32c52ec87d6245ee2d12ed5dd509e40062939b50.

diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -805,6 +805,8 @@ static int scsi_cdrom_send_packet(struct scsi_device *sdev, bool open_for_write,
 	err = scsi_get_cdrom_generic_arg(&cgc, arg);
 	if (err)
 		return err;
+	if (cgc.timeout < 0)
+		return -EINVAL;
 
 	cgc.timeout = clock_t_to_jiffies(cgc.timeout);
 	memset(&hdr, 0, sizeof(hdr));