| Message ID | 1412342941-32498-1-git-send-email-ijc@hellion.org.uk |
|---|---|
| State | New |
| Headers | show |
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!
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!
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);