Messages in this thread | | | Date | Tue, 27 Feb 2024 08:32:51 -0800 | From | "H. Peter Anvin" <> | Subject | Re: [PATCH] x86, relocs: Ignore relocations in .notes section |
| |
On February 27, 2024 8:13:35 AM PST, "Jürgen Groß" <jgross@suse.com> wrote: >On 22.02.24 18:18, Kees Cook wrote: >> When building with CONFIG_XEN_PV=y, .text symbols are emitted into the >> .notes section so that Xen can find the "startup_xen" entry point. This >> information is used prior to booting the kernel, so relocations are not >> useful. In fact, performing relocations against the .notes section means >> that the KASLR base is exposed since /sys/kernel/notes is world-readable. >> >> To avoid leaking the KASLR base without breaking unprivileged tools that >> are expecting to read /sys/kernel/notes, skip performing relocations in >> the .notes section. The values readable in .notes are then identical to >> those found in System.map. >> >> Reported-by: Guixiong Wei <guixiongwei@gmail.com> >> Closes: https://lore.kernel.org/all/20240218073501.54555-1-guixiongwei@gmail.com/ >> Fixes: 5ead97c84fa7 ("xen: Core Xen implementation") >> Fixes: da1a679cde9b ("Add /sys/kernel/notes") >> Signed-off-by: Kees Cook <keescook@chromium.org> >> --- >> Cc: Borislav Petkov <bp@alien8.de> >> Cc: Thomas Gleixner <tglx@linutronix.de> >> Cc: Ingo Molnar <mingo@redhat.com> >> Cc: Dave Hansen <dave.hansen@linux.intel.com> >> Cc: x86@kernel.org >> Cc: "H. Peter Anvin" <hpa@zytor.com> >> Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: Tony Luck <tony.luck@intel.com> >> Cc: Kristen Carlson Accardi <kristen@linux.intel.com> >> Cc: "Jürgen Groß" <jgross@suse.com> >> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> >> Cc: Stefano Stabellini <sstabellini@kernel.org> >> Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> Cc: Guixiong Wei <weiguixiong@bytedance.com> >> Cc: Jann Horn <jannh@google.com> >> --- >> arch/x86/tools/relocs.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c >> index a3bae2b24626..0811fff23b9c 100644 >> --- a/arch/x86/tools/relocs.c >> +++ b/arch/x86/tools/relocs.c >> @@ -733,6 +733,16 @@ static void walk_relocs(int (*process)(struct section *sec, Elf_Rel *rel, >> if (sec->shdr.sh_type != SHT_REL_TYPE) { >> continue; >> } >> + >> + /* >> + * Do not perform relocations in .notes section; any >> + * values there are meant for pre-boot consumption (e.g. >> + * startup_xen). >> + */ >> + if (strcmp(sec_name(sec->shdr.sh_info), ".notes") == 0) { > >Instead of a strcmp(), wouldnt't ... > >> + continue; >> + } >> + >> sec_symtab = sec->link; >> sec_applies = &secs[sec->shdr.sh_info]; >> if (!(sec_applies->shdr.sh_flags & SHF_ALLOC)) { > >... a test of "sec_applies->shdr.sh_type == SHT_NOTE" work as well? > >In the end I'm fine with both variants, so: > >Reviewed-by: Juergen Gross <jgross@suse.com> > > >Juergen
A type check would probably be better...
| |