On 22 June 2011 12:56, Alexey I. Froloff <ra...@altlinux.org> wrote:
> When host gcc uses _FORTIFY_SOURCE=2 by default, tools/firmware-utils
> fails to compile:
>
> In file included from /usr/include/string.h:658:0,
> from src/imagetag.c:12:
> In function 'strncpy',
> inlined from 'tagfile' at src/imagetag.c:369:11:
> /usr/include/bits/string3.h:123:3: error: call to __builtin___strncpy_chk
will always overflow destination buffer
>
> Signed-off-by: Alexey I. Froloff <ra...@altlinux.org>
> ---
> tools/firmware-utils/src/imagetag.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/firmware-utils/src/imagetag.c
b/tools/firmware-utils/src/imagetag.c
> index bebaba2..cfe7cf4 100644
> --- a/tools/firmware-utils/src/imagetag.c
> +++ b/tools/firmware-utils/src/imagetag.c
> @@ -362,11 +362,11 @@ int tagfile(const char *kernel, const char *rootfs,
const char *bin, \
> }
>
> if (args->reserved2_given) {
> - strncpy(tag.reserved2, args->reserved2_arg, 16);
> + memcpy(tag.reserved2, args->reserved2_arg, 16);
NACK. First of all, this didn't generate a warning at all (but that doesn't
mean there can't a problem ;-). Secondly, you discard the null terminator if
args->reserved2_arg is longer than 15 chars, creating potential problems
later on. If it is less than 15, you'll read from the memory that comes
after arg->reserved2_arg.
> }
>
> if (args->altinfo_given) {
> - strncpy(&tag.information1[0], args->altinfo_arg,
ALTTAGINFO_LEN);
> + memcpy(&tag.information1[0], args->altinfo_arg, ALTTAGINFO_LEN);
NACK for the same reasons above.
Also the warning is mostly harmless; removing it isn't quite as easy. A
potential solution would be to define a union struct with either
information1, flashlayourver fsKernelCRC and information2 or a
ALTTAGINFO_LEN sized altinfo, then using them as needed in struct bcm_tag,
or make the bcm_tag itself a union of the different layouts.
An actual bug is the length argument, it should be actually one less than
_LEN (to leave space for the null terminator when the string is too long).
This is harmless though, this can never happen since ALTTAGINFO_LEN long
altinfos already get rejected earlier.
Jonas
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
