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

Hi Caleb,

On Fri, Jul 24, 2026 at 08:43:42AM -0700, Caleb Sander Mateos wrote:
> 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.

Right —  I'll add READ_ONCE() for max_response_len in the next revision.

Thanks,
Yang Xiuwei