[U-Boot] [PATCH 1/3] serial-uclass.c: Copy at most sdev.name - 1 characters into the buffer

[U-Boot] [PATCH 1/3] serial-uclass.c: Copy at most sdev.name - 1 characters into the buffer

[Tom Rini]

Coverity notes that we do not ensure a NULL terminated string here as we
could fill the entire buffer with our strncpy call.  Fix this by
subtracting one.

Reported-by: Coverity (CID 131093)
Cc: Simon Glass <sjg@chromium.org>
Signed-off-by: Tom Rini <trini@konsulko.com>
---
 drivers/serial/serial-uclass.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 842f78b..2ef82b0 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -324,7 +324,7 @@ static int serial_post_probe(struct udevice *dev)
 		return 0;
 	memset(&sdev, '\0', sizeof(sdev));
 
-	strncpy(sdev.name, dev->name, sizeof(sdev.name));
+	strncpy(sdev.name, dev->name, sizeof(sdev.name) - 1);
 	sdev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT;
 	sdev.priv = dev;
 	sdev.putc = serial_stub_putc;
-- 
1.7.9.5

[U-Boot] [PATCH 2/3] sandbox: eth-raw-os.c: Ensure that our interface name is not too long

[Tom Rini]

Coverity notes that we do not ensure when we copy ifname we still have
space left to ensure NULL termination.  As cannot control the size of
ifr_name we must make sure that our argument will not overflow the
buffer.

Reported-by: Coverity (CID 131094)
Cc: Simon Glass <sjg@chromium.org>
Signed-off-by: Tom Rini <trini@konsulko.com>
---
 arch/sandbox/cpu/eth-raw-os.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/sandbox/cpu/eth-raw-os.c b/arch/sandbox/cpu/eth-raw-os.c
index b76a731..528865f 100644
--- a/arch/sandbox/cpu/eth-raw-os.c
+++ b/arch/sandbox/cpu/eth-raw-os.c
@@ -76,6 +76,10 @@ static int _raw_packet_start(const char *ifname, unsigned char *ethmac,
 		printf("Failed to set promiscuous mode: %d %s\n"
 		       "Falling back to the old \"flags\" way...\n",
 			errno, strerror(errno));
+		if (strlen(ifname) >= IFNAMSIZ) {
+			printf("Interface name %s is too long.\n", ifname);
+			return -EINVAL;
+		}
 		strncpy(ifr.ifr_name, ifname, IFNAMSIZ);
 		if (ioctl(priv->sd, SIOCGIFFLAGS, &ifr) < 0) {
 			printf("Failed to read flags: %d %s\n", errno,
-- 
1.7.9.5

[U-Boot] [PATCH 2/3] sandbox: eth-raw-os.c: Ensure that our interface name is not too long

[Simon Glass]

On 7 December 2015 at 20:26, Tom Rini <trini@konsulko.com> wrote:
> Coverity notes that we do not ensure when we copy ifname we still have
> space left to ensure NULL termination.  As cannot control the size of
> ifr_name we must make sure that our argument will not overflow the
> buffer.
>
> Reported-by: Coverity (CID 131094)
> Cc: Simon Glass <sjg@chromium.org>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>  arch/sandbox/cpu/eth-raw-os.c |    4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH 2/3] sandbox: eth-raw-os.c: Ensure that our interface name is not too long

[Simon Glass]

Applied to u-boot-dm/next.

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

[Tom Rini]

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@chromium.org>
Signed-off-by: Tom Rini <trini@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

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

[Simon Glass]

Hi Tom,

On 7 December 2015 at 20:26, Tom Rini <trini@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@chromium.org>
> Signed-off-by: Tom Rini <trini@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.

Regards,
Simon

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

[Tom Rini]

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@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@chromium.org>
> > Signed-off-by: Tom Rini <trini@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.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151208/ee594076/attachment.sig>

[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@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@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@chromium.org>
> > > Signed-off-by: Tom Rini <trini@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

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

[Tom Rini]

On Tue, Dec 08, 2015 at 04:42:17PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Dec 8, 2015 4:28 PM, "Tom Rini" <trini@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@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@chromium.org>
> > > > Signed-off-by: Tom Rini <trini@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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151208/dcf09062/attachment.sig>

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

[Simon Glass]

Hi Tom,

On 8 December 2015 at 16:58, Tom Rini <trini@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" <trini@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@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@chromium.org>
> > > > > Signed-off-by: Tom Rini <trini@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...

Regards,
Simon

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

[Tom Rini]

On Wed, Dec 09, 2015 at 04:58:12PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On 8 December 2015 at 16:58, Tom Rini <trini@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" <trini@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@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@chromium.org>
> > > > > > Signed-off-by: Tom Rini <trini@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151209/a31c7b2a/attachment.sig>

[U-Boot] [PATCH 1/3] serial-uclass.c: Copy at most sdev.name - 1 characters into the buffer

[Simon Glass]

On 7 December 2015 at 20:26, Tom Rini <trini@konsulko.com> wrote:
> Coverity notes that we do not ensure a NULL terminated string here as we
> could fill the entire buffer with our strncpy call.  Fix this by
> subtracting one.
>
> Reported-by: Coverity (CID 131093)
> Cc: Simon Glass <sjg@chromium.org>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>  drivers/serial/serial-uclass.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

>
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 842f78b..2ef82b0 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -324,7 +324,7 @@ static int serial_post_probe(struct udevice *dev)
>                 return 0;
>         memset(&sdev, '\0', sizeof(sdev));
>
> -       strncpy(sdev.name, dev->name, sizeof(sdev.name));
> +       strncpy(sdev.name, dev->name, sizeof(sdev.name) - 1);

There is also strlcpy() if you want it.

>         sdev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT;
>         sdev.priv = dev;
>         sdev.putc = serial_stub_putc;
> --
> 1.7.9.5
>

[U-Boot] [PATCH 1/3] serial-uclass.c: Copy at most sdev.name - 1 characters into the buffer

[Tom Rini]

On Tue, Dec 08, 2015 at 12:35:18PM -0700, Simon Glass wrote:
> On 7 December 2015 at 20:26, Tom Rini <trini@konsulko.com> wrote:
> > Coverity notes that we do not ensure a NULL terminated string here as we
> > could fill the entire buffer with our strncpy call.  Fix this by
> > subtracting one.
> >
> > Reported-by: Coverity (CID 131093)
> > Cc: Simon Glass <sjg@chromium.org>
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> >  drivers/serial/serial-uclass.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> >
> > diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> > index 842f78b..2ef82b0 100644
> > --- a/drivers/serial/serial-uclass.c
> > +++ b/drivers/serial/serial-uclass.c
> > @@ -324,7 +324,7 @@ static int serial_post_probe(struct udevice *dev)
> >                 return 0;
> >         memset(&sdev, '\0', sizeof(sdev));
> >
> > -       strncpy(sdev.name, dev->name, sizeof(sdev.name));
> > +       strncpy(sdev.name, dev->name, sizeof(sdev.name) - 1);
> 
> There is also strlcpy() if you want it.

Ah good.  Yeah, I think I should v2 this patch and use strlcpy as
there's going to be many more of these I bet to come.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151208/545774f7/attachment.sig>