From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Wed, 9 Dec 2015 20:01:21 -0500 Subject: [U-Boot] [PATCH 3/3] sandbox: sandbox_flash.c: Ensure NUL-termination on product/vendor strings In-Reply-To: References: <1449545195-18195-1-git-send-email-trini@konsulko.com> <1449545195-18195-3-git-send-email-trini@konsulko.com> <20151208232834.GF667@bill-the-cat> <20151208235845.GH667@bill-the-cat> Message-ID: <20151210010121.GP667@bill-the-cat> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Wed, Dec 09, 2015 at 04:58:12PM -0700, Simon Glass wrote: > Hi Tom, > > On 8 December 2015 at 16:58, Tom Rini 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" 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 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 > > > > > > Signed-off-by: Tom Rini > > > > > > --- > > > > > > 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: