[U-Boot] [PATCH for-v2014.10?] pxe: Ensure we don't overflow bootargs

[U-Boot] [PATCH for-v2014.10?] pxe: Ensure we don't overflow bootargs

[Ian Campbell]

From: Ian Campbell <ian.campbell@citrix.com>

On a couple of platforms I've tripped over long PXE append lines overflowing
this array, due to having CONFIG_SYS_CBSIZE == 256. When doing preseeded Debian
installs it's pretty trivial to exceed that.

Since the symptom can be a silent hang or a crash add a check. Of course the
affected boards would also need an increased CBSIZE to actually work.

Note that due to the printing of the final bootargs string CONFIG_SYS_PBSIZE
also needs to be sufficiently large.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
I think this is v2014.10 material?
---
 common/cmd_pxe.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
index 0ab1e0a..e63a031 100644
--- a/common/cmd_pxe.c
+++ b/common/cmd_pxe.c
@@ -674,6 +674,15 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)
 		char bootargs[CONFIG_SYS_CBSIZE] = "";
 		char finalbootargs[CONFIG_SYS_CBSIZE];
 
+		if (strlen(label->append ?: "") +
+		    strlen(ip_str) + strlen(mac_str) + 1 > sizeof(bootargs)) {
+			printf("bootarg overflow %d+%d+%d+1 > %zd\n",
+			       strlen(label->append ?: ""),
+			       strlen(ip_str), strlen(mac_str),
+			       sizeof(bootargs));
+			return 1;
+		}
+
 		if (label->append)
 			strcpy(bootargs, label->append);
 		strcat(bootargs, ip_str);
-- 
2.1.0

[U-Boot] [U-Boot, for-v2014.10?] pxe: Ensure we don't overflow bootargs

[Tom Rini]

On Fri, Oct 03, 2014 at 02:29:01PM +0100, Ian Campbell wrote:

> From: Ian Campbell <ian.campbell@citrix.com>
> 
> On a couple of platforms I've tripped over long PXE append lines overflowing
> this array, due to having CONFIG_SYS_CBSIZE == 256. When doing preseeded Debian
> installs it's pretty trivial to exceed that.
> 
> Since the symptom can be a silent hang or a crash add a check. Of course the
> affected boards would also need an increased CBSIZE to actually work.
> 
> Note that due to the printing of the final bootargs string CONFIG_SYS_PBSIZE
> also needs to be sufficiently large.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> I think this is v2014.10 material?
> ---
>  common/cmd_pxe.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
> index 0ab1e0a..e63a031 100644
> --- a/common/cmd_pxe.c
> +++ b/common/cmd_pxe.c
> @@ -674,6 +674,15 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)
>  		char bootargs[CONFIG_SYS_CBSIZE] = "";
>  		char finalbootargs[CONFIG_SYS_CBSIZE];
>  
> +		if (strlen(label->append ?: "") +
> +		    strlen(ip_str) + strlen(mac_str) + 1 > sizeof(bootargs)) {
> +			printf("bootarg overflow %d+%d+%d+1 > %zd\n",

With a change to use %zd in all cases (aarch64 warns otherwise), applied
to u-boot/master, 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/20141010/8fe6e03e/attachment.pgp>

[U-Boot] [U-Boot, for-v2014.10?] pxe: Ensure we don't overflow bootargs

[Ian Campbell]

On Fri, 2014-10-10 at 10:39 -0400, Tom Rini wrote:
> On Fri, Oct 03, 2014 at 02:29:01PM +0100, Ian Campbell wrote:
> 
> > From: Ian Campbell <ian.campbell@citrix.com>
> > 
> > On a couple of platforms I've tripped over long PXE append lines overflowing
> > this array, due to having CONFIG_SYS_CBSIZE == 256. When doing preseeded Debian
> > installs it's pretty trivial to exceed that.
> > 
> > Since the symptom can be a silent hang or a crash add a check. Of course the
> > affected boards would also need an increased CBSIZE to actually work.
> > 
> > Note that due to the printing of the final bootargs string CONFIG_SYS_PBSIZE
> > also needs to be sufficiently large.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> > I think this is v2014.10 material?
> > ---
> >  common/cmd_pxe.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c
> > index 0ab1e0a..e63a031 100644
> > --- a/common/cmd_pxe.c
> > +++ b/common/cmd_pxe.c
> > @@ -674,6 +674,15 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)
> >  		char bootargs[CONFIG_SYS_CBSIZE] = "";
> >  		char finalbootargs[CONFIG_SYS_CBSIZE];
> >  
> > +		if (strlen(label->append ?: "") +
> > +		    strlen(ip_str) + strlen(mac_str) + 1 > sizeof(bootargs)) {
> > +			printf("bootarg overflow %d+%d+%d+1 > %zd\n",
> 
> With a change to use %zd in all cases (aarch64 warns otherwise),

Oops, sorry!

>  applied
> to u-boot/master, thanks!

thanks!