Messages in this thread | | | Subject | Re: [PATCH v2] powerpc/64: Fix build failure with GCC 8.1 | From | Christophe LEROY <> | Date | Thu, 31 May 2018 07:57:48 +0200 |
| |
Le 31/05/2018 à 07:54, Michael Ellerman a écrit : > 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().
Thanks. You take it as is and add the kzalloc() or you expect a v3 from me with the kzalloc()
Christophe
| |