Re: Implement mode_select for atapi tape drives

[Paul Goyette <paul%whooppee.com@localhost>]

On Sat, 8 Aug 2009, Marc Balmer wrote:

> > > Why do you use
> > >
> > > memset(&atapi_select, 0, atapi_select_len);
> > >
> > > instead of
> > >
> > > memset(&atapi_select, 0, sizeof(atapi_select));
> > >
> > > and zap the u_int atapi_select_len variable?

I responded with:

> > Basically, this whole routine was copied almost verbatim from the 
> > equivalent code in sys/dev/scsipi/st_scsi.c which also calculates the 
> > number of bytes to clear.  Since it worked, I never looked any further.
> >
> > There is always the chance that the computed value of atapi_select_len 
> > (based on the _actual_ size of the device's page_0) is less than the 
> > possible maximum size of page_0.  So we could save a few nanoseconds by 
> > clearing fewer bytes.  In practice I don't expect this to make any real 
> > difference.
> >
> > ...
> >
> > If I change my patch, I'd want to update the calculation in st_scsi.c as 
> > well, to keep them in sync.

Marc countered with:

> And on the worse side of possibilities, there is the chance of buffer 
> overflow, if for some the reason the calculated value is bigger than the 
> structs sice.  This code should really use sizeof, to be unambigously clear.
>
> ...
>
> Yes, it should be changed there, too.

OK, I can change the memset() calls in both places to use sizeof.

But the calculated value of *_select_len is actually passed to routine 
scsipi_mode_select() a little bit later on, so we still need the 
calculated value.

If you're really concerned about a buffer overflow, it would probably 
make sense to validate the value of st->page_0_size when it is set in 
the load_quirk() code (for both atapi and scsi).



-------------------------------------------------------------------------
|   Paul Goyette   | PGP DSS Key fingerprint: |  E-mail addresses:      |
| Customer Service | FA29 0E3B 35AF E8AE 6651 |  paul at whooppee.com   |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer |                          | pgoyette at netbsd.org  |
-------------------------------------------------------------------------