Hello
On Wed, May 20, 2020 at 08:26:32AM +0200, Uwe Kleine-König wrote:
> On Tue, May 19, 2020 at 11:55:55PM -0400, David Dgien wrote:
> > When CONFIG_PASSWORD_DEFAULT is unset, the default_passwd buffer is set
>
> I assume you mean "If CONFIG_PASSWORD_DEFAULT is set to an empty
> string".
Yes.
>
> > to the empty string. The read_default_passwd() function wants to read at
> > least two characters from that buffer, causing GCC to generate an array
> > bounds warning.
>
> I cannot reproduce that warning. Which gcc version do you use and on
> which platform? Mentioning the exact warning in the commit log helps
> finding the resulting commit when searching for a fix.
arm-none-eabi-gcc --version prints "arm-none-eabi-gcc (Arch Repository)
10.1.0"
I found the issue when building for rpi_defconfig and
vexpress_defconfig.
The warning I get when building from master (commit c10b20dc83ac):
barebox/common/password.c: In function 'login':
barebox/common/password.c:173:5: warning: array subscript [1, 2147483647] is outside array bounds of 'const char[1]' [-Warray-bounds]
173 | c = buf[i];
| ~~^~~~~~~~
In file included from barebox/common/password.c:30:
include/generated/passwd.h:1:19: note: while referencing 'default_passwd'
1 | static const char default_passwd[] = "";
| ^~~~~~~~~~~~~~
I guess the compiler doesn't know that strlen(default_passwd) = 0, just
that length > 0 so the most it can assume is that the loop has to
consume at least two chars, and the empty string only contains one.
>
> > Make the default_passwd buffer have at least 2 bytes so
> > this warning is not generated.
> >
> > Since the read_default_passwd() function is only called when
> > default_passwd is not the empty string, this is not a functional change.
>
> I don't understand the problem for the empty password. With
> default_passwd = "" we have strlen(default_passwd) = 0 so the for loop
> doesn't run at all.
Yes, that's correct, which is one reason why this is not functionally
different. But the compiler doesn't seem to be smart enough to know
that.
>
> As I understand the code (at commit c10b20dc83ac) for uneven lengths of
> default_passwd the last accessed byte is the trailing '\0' and for even
> length it's the byte before the trailing '\0'. This should be ok?!
>
> Am I missing something?
When working on this reply, I realized there was another solution I
missed when I was trying to find ways to short-circut the compiler
previously. If I add:
if (ARRAY_SIZE(default_passwd) == 1)
return -ENOSYS;
in the read_default_passwd() function, that would short-circut the
compiler preventing the warning message, and is less hacky. I can
resubmit with that change instead.
>
> Best regards
> Uwe
>
> --
> Pengutronix e.K. | Uwe Kleine-König |
> Industrial Linux Solutions | https://www.pengutronix.de/ |
Thanks,
David Dgien
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
