On Wed, Dec 09, 2015 at 04:58:12PM -0700, Simon Glass wrote:
> Hi Tom,
>
> On 8 December 2015 at 16:58, Tom Rini <tr...@konsulko.com> wrote:
> >
> > On Tue, Dec 08, 2015 at 04:42:17PM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Dec 8, 2015 4:28 PM, "Tom Rini" <tr...@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 <tr...@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 <s...@chromium.org>
> > > > > > Signed-off-by: Tom Rini <tr...@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.
> >
> > Yeah, true too. I'm having a bit of trouble tracing how all of this
> > ties back into where in the rest of the stack. Can you figure out if
> > this is safe or if we should NULL terminate [7]/etc or what? Thanks!
>
> See usb_inquiry() which issues the request and usb_stor_get_info()
> which processes the response.. Sandbox needs to send the response back
> in the expected format or it will not work. You can run the driver
> model usb tests to check. However at present there are three separate
> test breakages in driver model...Well blarg. I really do need to get into the habit of running the tests every time I also do a big build cycle, along with doing gcc 5.. -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
