On Mon, Jul 22, 2019 at 10:40:40AM -0700, Nick Desaulniers wrote:
> On Fri, Jul 19, 2019 at 8:41 PM Kees Cook <keesc...@chromium.org> wrote:
> >
> > Much like commit 18c9a99bce2a ("libata: zpodd: small read overflow in
> > eject_tray()"), this fixes a cdb[] buffer length, this time in
> > zpodd_get_mech_type():
> >
> > We read from the cdb[] buffer in ata_exec_internal_sg(). It has to be
> > ATAPI_CDB_LEN (16) bytes long, but this buffer is only 12 bytes.
> >
> > The KASAN report was:
> >
> > BUG: KASAN: global-out-of-bounds in ata_exec_internal_sg+0x50f/0xc70
> > Read of size 16 at addr ffffffff91f41f80 by task scsi_eh_1/149
> > ...
> > The buggy address belongs to the variable:
> > cdb.48319+0x0/0x40
> >
> > Reported-by: Jeffrin Thalakkottoor <jeff...@rajagiritech.edu.in>
> > Fixes: afe759511808c ("libata: identify and init ZPODD devices")
> > Signed-off-by: Kees Cook <keesc...@chromium.org>
>
> Interesting, both initializer lists are less than ATAPI_CDB_LEN (16)
> elements, yet ata_exec_internal_sg() in drivers/ata/libata-core.c very
> clearly has a ATAPI_CDB_LEN byte memcpy on that element. Probably
> both initializer lists should just lead off the trailing zero's or
> provide the entire 16 elements. For now, thank you for the patch.
Yup, with the specific size added it'll be zero-padded.
> Reviewed-by: Nick Desaulniers <ndesaulni...@google.com>
Thanks!
-Kees
>
> > ---
> > drivers/ata/libata-zpodd.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/ata/libata-zpodd.c b/drivers/ata/libata-zpodd.c
> > index 173e6f2dd9af..eefda51f97d3 100644
> > --- a/drivers/ata/libata-zpodd.c
> > +++ b/drivers/ata/libata-zpodd.c
> > @@ -56,7 +56,7 @@ static enum odd_mech_type zpodd_get_mech_type(struct
> > ata_device *dev)
> > unsigned int ret;
> > struct rm_feature_desc *desc;
> > struct ata_taskfile tf;
> > - static const char cdb[] = { GPCMD_GET_CONFIGURATION,
> > + static const char cdb[ATAPI_CDB_LEN] = { GPCMD_GET_CONFIGURATION,
> > 2, /* only 1 feature descriptor requested */
> > 0, 3, /* 3, removable medium feature */
> > 0, 0, 0,/* reserved */
> > --
> > 2.17.1
> >
> >
> > --
> > Kees Cook
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Kees Cook
