Re: [PATCH 1/2] scsi: bsg: fix TOCTOU in io_uring passthrough command setup

On Thu, Jul 23, 2026 at 7:03 PM Yang Xiuwei <[email protected]> wrote:
>
> Hi Caleb,
>
> On Thu, Jul 23, 2026 at 06:10:27PM -0700, Caleb Sander Mateos wrote:
> > On Thu, Jul 23, 2026 at 5:50 PM Yang Xiuwei <[email protected]> wrote:
> > >
> > > max_response_len is similar here: we clamp with
> > > min(..., SCSI_SENSE_BUFFERSIZE), so it cannot overflow the sense
> > > buffer. Same idea for response.
> >
> > I don't think that's true. Without READ_ONCE(), the compiler can
> > assume no other thread will concurrently write cmd->max_response_len
> > (else it would be UB). So it's allowed to load it multiple times:
> > cmd->max_response_len ? min(cmd->max_response_len, SCSI_SENSE_BUFFERSIZE)
> >                       : SCSI_SENSE_BUFFERSIZE
> > becomes
> > cmd->max_response_len ? (cmd->max_response_len < SCSI_SENSE_BUFFERSIZE ?
> >                          cmd->max_response_len : SCSI_SENSE_BUFFERSIZE)
> >                       : SCSI_SENSE_BUFFERSIZE
> > And if cmd->max_response_len changes between the second and third
> > loads, this could definitely evaluate to a value greater than
> > SCSI_SENSE_BUFFERSIZE.
>
> IIUC Kernel min() is __careful_cmp() -> __careful_cmp_once(), which does:
>
>   auto ux = (x); auto uy = (y);
>   (ux < uy) ? ux : uy;
>
> so cmd->max_response_len is only loaded once inside min(). The outer
> ?: may load it again for the condition, but the value coming out of
> min() should still be <= SCSI_SENSE_BUFFERSIZE. Did I get that right?

Without READ_ONCE(), there is no requirement that
cmd->max_response_len only be loaded once. The compiler is allowed to
assume the value won't be modified concurrently by another thread
(that would be a data race, undefined behavior). So it's free to
combine or separate the loads. If you want to ensure that the value
read from cmd->max_response_len is consistent, READ_ONCE() is
required.

Best,
Caleb