Re: [PATCH v2] scsi: scsi_debug: fix one-partition tape setup bounds

[Samuel Moelius]

On Thu, Jun 4, 2026 at 3:29 PM James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, 2026-06-04 at 22:14 +0300, Kai Mäkisara (Kolumbus) wrote:
> >
> > > On 4. Jun 2026, at 21.33, Samuel Moelius
> > > <sam.moelius@xxxxxxxxxxxxxxx> wrote:
> > >
> > > On Thu, Jun 4, 2026 at 9:38 AM James Bottomley
> > > <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > > >
> > > > On Wed, 2026-06-03 at 23:55 +0000, Samuel Moelius wrote:
> > > > > The tape setup path writes partition metadata one element past
> > > > > the
> > > > > allocated tape_blocks array when a one-partition configuration
> > > > > is
> > > > > selected.
> > > > >
> > > > > That corrupts adjacent state during device initialization
> > > > > before any
> > > > > command is issued.
> > > >
> > > > I still don't get what the actual problem is.  For a single
> > > > partition
> > > > tape I can't see where scsi_debug would actually do anything with
> > > > tape_blocks[1].  What is it that you're seeing when using
> > > > scsi_debug
> > > > that motivates this?
> > >
> > > The bug is a kernel OOB write. I can share a PoC if desired. The
> > > PoC
> > > sends this SCSI command through /dev/sgN:
> > >
> > > ...
> >
> > > Then the bug: it initializes partition 1 even though there is only
> > > one
> > > partition:
> > >
> > >    devip->tape_eop[1] = part_1_size;
> > >    devip->tape_blocks[1] = devip->tape_blocks[0] +
> > >                            devip->tape_eop[0];
> > >    devip->tape_blocks[1]->fl_size = TAPE_BLOCK_EOD_FLAG;
> > >
> > > Because devip->tape_eop[0] == 10000, this computes:
> > >
> > >    devip->tape_blocks[1] = devip->tape_blocks[0] + 10000
> > >
> > > But the allocation has only 10000 elements. So this write is one
> > > element past the allocation.
> >
> > OK. The bug is not initialization of the pointer but writing the
> > fl_size using the pointer. Good catch!
>
> Isn't the fix actually to allocate an extra block for the EOF:
>
> @@ -6648,7 +6648,7 @@ static int scsi_debug_sdev_configure(struct scsi_device *sdp,
>         if (sdebug_ptype == TYPE_TAPE) {
>                 if (!devip->tape_blocks[0]) {
>                         devip->tape_blocks[0] =
> -                               kzalloc_objs(struct tape_block, TAPE_UNITS);
> +                               kzalloc_objs(struct tape_block, TAPE_UNITS + 1);
>                         if (!devip->tape_blocks[0])
>                                 return 1;

I'll send a v3 that uses that approach.