From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754007AbeEaF5z (ORCPT ); Thu, 31 May 2018 01:57:55 -0400 Received: from pegase1.c-s.fr ([93.17.236.30]:52114 "EHLO pegase1.c-s.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753790AbeEaF5u (ORCPT ); Thu, 31 May 2018 01:57:50 -0400 Subject: Re: [PATCH v2] powerpc/64: Fix build failure with GCC 8.1 To: Michael Ellerman , Geert Uytterhoeven Cc: Benjamin Herrenschmidt , Paul Mackerras , Linux Kernel Mailing List , linuxppc-dev , Geoff Levand References: <1fa5b220-4d21-b4cd-33bf-8a3ce3178063@c-s.fr> <3a438dd8-49c3-ad39-e2a1-040e0ce67279@c-s.fr> <87efhsz7xz.fsf@concordia.ellerman.id.au> From: Christophe LEROY Message-ID: Date: Thu, 31 May 2018 07:57:48 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <87efhsz7xz.fsf@concordia.ellerman.id.au> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: fr Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Le 31/05/2018 à 07:54, Michael Ellerman a écrit : > Christophe LEROY writes: >> Le 29/05/2018 à 11:05, Geert Uytterhoeven a écrit : >>> Hi Christophe, >>> On Tue, May 29, 2018 at 10:56 AM, Christophe LEROY >>> 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