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
| ||
|
Message-ID: <20170914080814.GO12824@x1> Date: Thu, 14 Sep 2017 16:08:14 +0800 From: Baoquan He <bhe@...hat.com> To: Dave Young <dyoung@...hat.com> Cc: linux-kernel@...r.kernel.org, x86@...nel.org, mingo@...hat.com, tglx@...utronix.de, hpa@...or.com, thgarnie@...gle.com, keescook@...omium.org, akpm@...ux-foundation.org, yamada.masahiro@...ionext.com, rja@....com, frank.ramsay@....com Subject: Re: [PATCH v2 RESEND 1/2] x86/UV: Introduce a helper function to check UV system at earlier stage On 09/14/17 at 03:49pm, Dave Young wrote: > > > diff --git a/arch/x86/include/asm/uv/uv.h b/arch/x86/include/asm/uv/uv.h > > > index b5a32231abd8..93d7ad8763ba 100644 > > > --- a/arch/x86/include/asm/uv/uv.h > > > +++ b/arch/x86/include/asm/uv/uv.h > > > @@ -18,6 +18,11 @@ extern void uv_nmi_init(void); > > > extern void uv_system_init(void); > > > extern const struct cpumask *uv_flush_tlb_others(const struct cpumask *cpumask, > > > const struct flush_tlb_info *info); > > > +#include <linux/efi.h> > > > +static inline int is_early_uv_system(void) > > > +{ > > > + return !((efi.uv_systab == EFI_INVALID_TABLE_ADDR) || !efi.uv_systab); > > > +} > Thanks for looking into this, Dave! > > Sorry for jumping in late, I have two questions about the patch: > > 1) For efi tables, the only invalid value is EFI_INVALID_TABLE_ADDR, and > efi struct is initialized as EFI_INVALID_TABLE_ADDR by default so no > need to check "|| !efi.uv_systab". Do we have any UV firmware specific > assumption that "0" is also possible to be assigned? Hmm, in uv_bios_init() it also checks the !efi.uv_systab case. And EFI_INVALID_TABLE_ADDR checking is earlier, it won't affect the result if it's EFI_INVALID_TABLE_ADDR. And !efi.uv_systab can make it safer since it doesn't work either if efi.uv_systab is 0. Mainly it's not harmful. Mike, what's your thought? Should I only check the (efi.uv_systab == EFI_INVALID_TABLE_ADDR) case? > > 2) It seems adding this function in uv.h for separating this for uv > system only purpose. But I feel it is better to put it in efi.h instead. At the beginning I put it in efi.c, later Mike suggested putting it in asm/uv/uv.h. You can also find the discussion in below link. https://patchwork.kernel.org/patch/9732787/ Thanks Baoquan > > uv_systab is already a member of struct efi, it is in efi.h so it is > natural to check the table exist or not. Then just include efi.h in > kaslr.c and use the function. > > something like drivers/firmware/efi/esrt.c: esrt_table_exists() > > Anyway I have no strong opinon, it looks more natural to me though. > > > > > > > #else /* X86_UV */ > > > > > > @@ -30,6 +35,7 @@ static inline const struct cpumask * > > > uv_flush_tlb_others(const struct cpumask *cpumask, > > > const struct flush_tlb_info *info) > > > { return cpumask; } > > > +static inline int is_early_uv_system(void) { return 0; } > > > > > > #endif /* X86_UV */ > > > > > > -- > > > 2.5.5 > > > > > Thanks > Dave
Powered by blists - more mailing lists