Messages in this thread | | | From | Michael Ellerman <> | Subject | Re: [PATCH v2] powerpc/64: Fix build failure with GCC 8.1 | Date | Thu, 31 May 2018 15:54:16 +1000 |
| |
Christophe LEROY <christophe.leroy@c-s.fr> writes: > Le 29/05/2018 à 11:05, Geert Uytterhoeven a écrit : >> Hi Christophe, >> On Tue, May 29, 2018 at 10:56 AM, Christophe LEROY >> <christophe.leroy@c-s.fr> wrote: >>> Le 29/05/2018 à 09:47, Geert Uytterhoeven a écrit : >>>> On Tue, May 29, 2018 at 8:03 AM, Christophe Leroy >>>>> --- a/arch/powerpc/kernel/nvram_64.c >>>>> +++ b/arch/powerpc/kernel/nvram_64.c >>>>> @@ -1039,7 +1039,7 @@ loff_t __init nvram_create_partition(const char >>>>> *name, int sig, >>>>> new_part->index = free_part->index; >>>>> new_part->header.signature = sig; >>>>> new_part->header.length = size; >>>>> - strncpy(new_part->header.name, name, 12); >>>>> + memcpy(new_part->header.name, name, strnlen(name, >>>>> sizeof(new_part->header.name))); >>>> >>>> >>>> The comment for nvram_header.lgnth says: >>>> >>>> /* Terminating null required only for names < 12 chars. */ >>>> >>>> This will not terminate the string with a zero (the struct is >>>> allocated with kmalloc). >>>> So the original code is correct, the new one isn't. >>> >>> Right, then I have to first zeroize the destination. >> >> Using kzalloc() instead of kmalloc() will do. >> >> Still, papering around these warnings seems to obscure things, IMHO. >> And it increases code size, as you had to add a call to strnlen().
The right fix is to not try and mirror the on-device structure in the kernel struct. We should just use a proper NULL terminated string, which would avoid the need to explicitly do strncmp(.., .., 12) in the code and be less bug prone in general.
The only place where we should need to worry about the 12 byte buffer is in nvram_write_header().
Anyway that's a bigger change, so I'll take this for now with kzalloc().
cheers
| |