[U-Boot] cmd/pxe.c: Rework bootargs construction to clarify string checks

Message ID 1507750473-10998-1-git-send-email-trini@konsulko.com
State Accepted
Delegated to: Tom Rini
Headers show
Series
  • [U-Boot] cmd/pxe.c: Rework bootargs construction to clarify string checks
Related show

Commit Message

Tom Rini Oct. 11, 2017, 7:34 p.m.
As the code currently stands, we first check that the length of the
given command line, along with ip_str/mac_str along with an additional 1
for the NULL termination will fit within the buffer we have, and if not,
we return an error.  The way this code was originally written however
left Coverity "unhappy" due to using strcat rather than strncat.
Switching this to strncat however causes clang to be unhappy that we
aren't enforcing the "1" portion within strncat.  Rather than further
re-work the code to include a "- 1" in this case as well, make the
strcat code only be done within the else side of the length test.  This
keeps both clang and Coverity happy.

Fixes: 48ee0a87bc46 ("cmd/pxe.c: Rework initrd and bootargs handling slightly")
Signed-off-by: Tom Rini <trini@konsulko.com>
---
 cmd/pxe.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Comments

Tom Rini Oct. 17, 2017, 12:48 a.m. | #1
On Wed, Oct 11, 2017 at 03:34:33PM -0400, Tom Rini wrote:

> As the code currently stands, we first check that the length of the
> given command line, along with ip_str/mac_str along with an additional 1
> for the NULL termination will fit within the buffer we have, and if not,
> we return an error.  The way this code was originally written however
> left Coverity "unhappy" due to using strcat rather than strncat.
> Switching this to strncat however causes clang to be unhappy that we
> aren't enforcing the "1" portion within strncat.  Rather than further
> re-work the code to include a "- 1" in this case as well, make the
> strcat code only be done within the else side of the length test.  This
> keeps both clang and Coverity happy.
> 
> Fixes: 48ee0a87bc46 ("cmd/pxe.c: Rework initrd and bootargs handling slightly")
> Signed-off-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!

Patch

diff --git a/cmd/pxe.c b/cmd/pxe.c
index a62cbe192a32..7043ad11fdd8 100644
--- a/cmd/pxe.c
+++ b/cmd/pxe.c
@@ -686,16 +686,17 @@  static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)
 			       strlen(ip_str), strlen(mac_str),
 			       sizeof(bootargs));
 			return 1;
+		} else {
+			if (label->append)
+				strncpy(bootargs, label->append,
+					sizeof(bootargs));
+			strcat(bootargs, ip_str);
+			strcat(bootargs, mac_str);
+
+			cli_simple_process_macros(bootargs, finalbootargs);
+			env_set("bootargs", finalbootargs);
+			printf("append: %s\n", finalbootargs);
 		}
-
-		if (label->append)
-			strncpy(bootargs, label->append, sizeof(bootargs));
-		strncat(bootargs, ip_str, sizeof(bootargs) - strlen(bootargs));
-		strncat(bootargs, mac_str, sizeof(bootargs) - strlen(bootargs));
-
-		cli_simple_process_macros(bootargs, finalbootargs);
-		env_set("bootargs", finalbootargs);
-		printf("append: %s\n", finalbootargs);
 	}
 
 	bootm_argv[1] = env_get("kernel_addr_r");