linux-kernel - Re: [PATCH] cpu: microcode: replace 0 with NULL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <074cc723b3874f95b1b1ad89c1d2dcbae982deba.camel@perches.com>
Date:   Wed, 27 Nov 2019 13:11:31 -0800
From:   Joe Perches <joe@...ches.com>
To:     Jules Irenge <jbi.octave@...il.com>, Borislav Petkov <bp@...en8.de>
Cc:     tglx@...utronix.de, linux-kernel@...r.kernel.org, x86@...nel.org,
        hpa@...or.com, mingo@...hat.com
Subject: Re: [PATCH] cpu: microcode: replace 0 with NULL

On Tue, 2019-11-26 at 16:03 +0000, Jules Irenge wrote:
> 
> On Tue, 26 Nov 2019, Borislav Petkov wrote:
> 
> > On Tue, Nov 26, 2019 at 12:27:34AM +0000, Jules Irenge wrote:
> > > Replace 0 with NULL to fix sparse tool  warning
> > >  warning: Using plain integer as NULL pointer
> > > 
> > > Signed-off-by: Jules Irenge <jbi.octave@...il.com>
> > > ---
> > >  arch/x86/kernel/cpu/microcode/amd.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
> > > index a0e52bd00ecc..4934aa7c94e7 100644
> > > --- a/arch/x86/kernel/cpu/microcode/amd.c
> > > +++ b/arch/x86/kernel/cpu/microcode/amd.c
> > > @@ -418,7 +418,7 @@ static int __apply_microcode_amd(struct microcode_amd *mc)
> > >  static bool
> > >  apply_microcode_early_amd(u32 cpuid_1_eax, void *ucode, size_t size, bool save_patch)
> > >  {
> > > -	struct cont_desc desc = { 0 };
> > > +	struct cont_desc desc = { NULL };
> > 
> > So my gcc guy says that 0 and NULL are equivalent as designated
> > initializers in this case. And if you look at the resulting asm, it
> > doesn't change:
> > 
> > # arch/x86/kernel/cpu/microcode/amd.c:421: 	struct cont_desc desc = { 0 };
> > 	movq	$0, 8(%rsp)	#, desc
> > 	movq	$0, (%rsp)	#, desc
> > 	movq	$0, 16(%rsp)	#, desc
> > 	movq	$0, 24(%rsp)	#, desc
> > 
> > But what I'd prefer actually is, if you do them like this:
> > 
> > 			... = { 0,  };
> > 
> > because:
> > 
> > 1. It is clear that the memory for the struct is being cleared
> > 2. The following ones - the ones after "," are missing too, on purpose,
> >    because they're being cleared too.
> > 
> > Also pls add that explanation to the commit message.
> > 
> > Thx.
> > 
> > -- 
> > Regards/Gruss,
> >     Boris.
> > 
> > https://people.kernel.org/tglx/notes-about-netiquette
> > 
> Hi Boris,
> 
> Thanks for your reply and suggestion. 
> 
> I am learning patching with sparse trying to solve some problems that the 
> tool complains about.
> 
> Sometime the tool is not always right. If I take your suggestion that I 
> am about to do, sparse will however still complain.
> 
> so I will suggest my change to be discarded.
> 
> I will take another challenge.

This initializer should ether use named members with the appropriate
zeroing type or just use a blank {} so that regardless of type and
member order, the entire structure is zeroed.

	struct cont_desc desc = {};