linux-kernel - Re: [PATCH] x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aBnrBHfj_TM8gyit@gmail.com>
Date: Tue, 6 May 2025 12:57:08 +0200
From: Ingo Molnar <mingo@...nel.org>
To: Borislav Petkov <bp@...en8.de>
Cc: linux-kernel@...r.kernel.org, linux-tip-commits@...r.kernel.org,
	stable@...nel.org, x86@...nel.org
Subject: Re: [PATCH] x86/microcode: Add microcode_loader_disabled() storage
 class for the !CONFIG_MICROCODE case

* Borislav Petkov <bp@...en8.de> wrote:

> On Mon, May 05, 2025 at 07:15:00AM +0200, Ingo Molnar wrote:
> > 
> > Fix this build bug:
> > 
> >   ./arch/x86/include/asm/microcode.h:27:13: warning: no previous prototype for ‘microcode_loader_disabled’ [-Wmissing-prototypes]
> > 
> > by adding the 'static' storage class to the !CONFIG_MICROCODE 
> > prototype.
> 
> Thanks, I'll merge it with my patch.

Please don't forcibly rebase trees like that, because you've just 
reintroduced a bug this way in tip:x86/urgent:

  # 5214a9f6c0f5 x86/microcode: Consolidate the loader enablement checking

  +static inline bool __init microcode_loader_disabled(void) { return false; }

static inlines don't need an __init tag ...

This was done correctly in the commit you removed:

  59e820c6de60 ("x86/microcode: Add microcode_loader_disabled() storage class for the !CONFIG_MICROCODE case")

  +static inline bool microcode_loader_disabled(void) { return false; }

> > Also, while at it, add all the other storage classes as well for this 
> > block of prototypes, 'extern' and 'static', respectively.
> 
> This I won't add as it is unnecessary.

Technically the 'int' in 'unsigned int' is 'unnecessary' and could be 
skipped as well with 'unsigned', right? Yet clean kernel code uses it 
because it's easier to read and more consistent. Likewise, the 'extern' 
storage class is a well-known and widespread way in the kernel to 
document public APIs, a counterpart to 'static'. Most of our major 
headers use that style.

Your change is a doubly poor choice in this particular case of 
<asm/microcode.h> as well, because the header continues with an 
'extern':

  extern unsigned long initrd_start_early;

So by dropping that cleanup you reintroduced an inconsistency as 
well...

Thanks,

	Ingo