linux-kernel - Re: [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR

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]

Date:   Wed, 26 May 2021 08:50:20 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Randy Dunlap <rdunlap@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, Mel Gorman <mgorman@...hsingularity.net>
Subject: Re: [PATCH] x86: fixmap: use CONFIG_NR_CPUS instead of NR_CPUS


* Randy Dunlap <rdunlap@...radead.org> wrote:

> Use CONFIG_NR_CPUS instead of NR_CPUS for an enum entry item.
> (Alternatively, #include <linux/threads.h> unconditionally instead of
> conditionally.)
> 
> This fixes 100+ build errors like so:
> 
> In file included from ../include/asm-generic/early_ioremap.h:6:0,
>                  from ./arch/x86/include/generated/asm/early_ioremap.h:1,
>                  from ../arch/x86/include/asm/io.h:44,
>                  from ../include/linux/io.h:13,
>                  from ../mm/early_ioremap.c:13:
> ../arch/x86/include/asm/fixmap.h:103:48: error: ‘NR_CPUS’ undeclared here (not in a function); did you mean ‘NR_OPEN’?
>   FIX_KMAP_END = FIX_KMAP_BEGIN + (KM_MAX_IDX * NR_CPUS) - 1,
> 
> Fixes: e972c2511967 ("mm/early_ioremap: add prototype for early_memremap_pgprot_adjust")

I believe this patch is in the -mm tree, not the x86 tree.

> @@ -100,7 +100,7 @@ enum fixed_addresses {
>  #endif
>  #ifdef CONFIG_KMAP_LOCAL
>  	FIX_KMAP_BEGIN,	/* reserved pte's for temporary kernel mappings */
> -	FIX_KMAP_END = FIX_KMAP_BEGIN + (KM_MAX_IDX * NR_CPUS) - 1,
> +	FIX_KMAP_END = FIX_KMAP_BEGIN + (KM_MAX_IDX * CONFIG_NR_CPUS) - 1,
>  #ifdef CONFIG_PCI_MMCONFIG
>  	FIX_PCIE_MCFG,
>  #endif

Please resolve this bug properly:

 - Don't sprinkle low level headers with random CONFIG_NR_CPUS conversions.

 - <asm/io.h> currently includes <asm/early_ioremap.h>, but this seems 
   unjustified.

 - Once early_ioremap.h is gone from io.h, it's potentially possible to 
   include <linux/threads.h>. More work to resolve dependencies might be 
   needed though.

Frankly, I'd prefer if such a low level header dependencies change came in 
via the x86 tree so we can properly review it, test it, and keep it 
working. Right now I can only guess what is needed here...

Thanks,

	Ingo