[U-Boot] [PATCH 3/3] sandbox: sandbox_flash.c: Ensure NUL-termination on product/vendor strings

[Simon Glass]

Hi Tom,

On Dec 8, 2015 4:28 PM, "Tom Rini" <trini at konsulko.com> wrote:
>
> On Tue, Dec 08, 2015 at 12:35:23PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On 7 December 2015 at 20:26, Tom Rini <trini at konsulko.com> wrote:
> > > Coverity notes that we do not ensure when we copy in the
product/vendor
> > > strings that they have NULL termination.  In this case the answer is
to
> > > increase the buffer we have and then set the last entry to NULL.
> > >
> > > Reported-by: Coverity (CID 131095)
> > > Cc: Simon Glass <sjg at chromium.org>
> > > Signed-off-by: Tom Rini <trini at konsulko.com>
> > > ---
> > >  drivers/usb/emul/sandbox_flash.c |    6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/usb/emul/sandbox_flash.c
b/drivers/usb/emul/sandbox_flash.c
> > > index 0965ad0..2811f52 100644
> > > --- a/drivers/usb/emul/sandbox_flash.c
> > > +++ b/drivers/usb/emul/sandbox_flash.c
> > > @@ -79,8 +79,8 @@ struct scsi_inquiry_resp {
> > >         u8 data_format;
> > >         u8 additional_len;
> > >         u8 spare[3];
> > > -       char vendor[8];
> > > -       char product[16];
> > > +       char vendor[9]; /* 8 + NUL-termination */
> > > +       char product[17]; /* 16 + NUL-termination */
> > >         char revision[4];
> > >  };
> > >
> > > @@ -260,6 +260,8 @@ static int handle_ufi_command(struct
sandbox_flash_plat *plat,
> > >                 strncpy(resp->product,
> > >                         plat->flash_strings[STRINGID_PRODUCT - 1].s,
> > >                         sizeof(resp->product));
> > > +               resp->vendor[8] = 0;
> > > +               resp->product[16] = 0;
> > >                 strncpy(resp->revision, "1.0",
sizeof(resp->revision));
> > >                 setup_response(priv, resp, sizeof(*resp));
> > >                 break;
> > > --
> > > 1.7.9.5
> > >
> >
> > Does this work? It seems like it is changing the SCSI protocol spec,
> > which we can't do.
>
> Yes.  We memset resp to NULL to start with so we know it will always be
> NULL terminated now.  Elsewhere in our SCSI code is where I got the
> idea, common/usb_storage.c::usb_stor_get_info for example.

Yes but you are changing the struct.

Regards,
Simon