| Message ID | 20191221151813.1573450-1-raj.khem@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86/boot/compressed/64: Define __force_order only when CONFIG_RANDOMIZE_BASE is unset | expand |
On Sat, Dec 21, 2019 at 07:18:13AM -0800, Khem Raj wrote: > Since arch/x86/boot/compressed/Makefile overrides global CFLAGS it loses > -fno-common option which would have caught this If this doesn't cause any visible problems, why bother? Hopefully, we will be able to drop it altogether once we ditch GCC 4.X support. -- Kirill A. Shutemov
On Mon, Dec 23, 2019 at 9:10 AM Kirill A. Shutemov <kirill@shutemov.name> wrote: > > On Sat, Dec 21, 2019 at 07:18:13AM -0800, Khem Raj wrote: > > Since arch/x86/boot/compressed/Makefile overrides global CFLAGS it loses > > -fno-common option which would have caught this > > If this doesn't cause any visible problems, why bother? > it does break builds with gcc trunk as of now e.g. > Hopefully, we will be able to drop it altogether once we ditch GCC 4.X > support. > gcc10 is switching defaults to -fno-common so we need to solve this one way or other, I am not sure if gcc 4.x will be dropped before gcc10 release which would be in mid of 2020 > -- > Kirill A. Shutemov
On Mon, Dec 23, 2019 at 02:25:02PM -0800, Khem Raj wrote: > On Mon, Dec 23, 2019 at 9:10 AM Kirill A. Shutemov <kirill@shutemov.name> wrote: > > > > On Sat, Dec 21, 2019 at 07:18:13AM -0800, Khem Raj wrote: > > > Since arch/x86/boot/compressed/Makefile overrides global CFLAGS it loses > > > -fno-common option which would have caught this > > > > If this doesn't cause any visible problems, why bother? > > > > it does break builds with gcc trunk as of now e.g. > > > Hopefully, we will be able to drop it altogether once we ditch GCC 4.X > > support. > > > > gcc10 is switching defaults to -fno-common so we need to solve this one way or > other, I am not sure if gcc 4.x will be dropped before gcc10 release > which would be > in mid of 2020 Okay, it makes sense then. Please include this info into the commit message. Also, I wounder if it would be cleaner to define both of them as __weak? -- Kirill A. Shutemov
On Tue, Dec 24, 2019 at 12:08 AM Kirill A. Shutemov <kirill@shutemov.name> wrote: > On Mon, Dec 23, 2019 at 02:25:02PM -0800, Khem Raj wrote: > > On Mon, Dec 23, 2019 at 9:10 AM Kirill A. Shutemov <kirill@shutemov.name> wrote: > > > > > > On Sat, Dec 21, 2019 at 07:18:13AM -0800, Khem Raj wrote: > > > > Since arch/x86/boot/compressed/Makefile overrides global CFLAGS it loses > > > > -fno-common option which would have caught this > > > > > > If this doesn't cause any visible problems, why bother? > > > > > > > it does break builds with gcc trunk as of now e.g. > > > > > Hopefully, we will be able to drop it altogether once we ditch GCC 4.X > > > support. > > > > > > > gcc10 is switching defaults to -fno-common so we need to solve this one way or > > other, I am not sure if gcc 4.x will be dropped before gcc10 release > > which would be > > in mid of 2020 > > Okay, it makes sense then. Please include this info into the commit > message. > > Also, I wounder if it would be cleaner to define both of them as __weak? Or maybe make the #ifdef check for gcc < 5 instead of checking for CONFIG_RANDOMIZE_BASE? That way it will be found by whoever cleans up the code when we increase the minimum compiler version to one that doesn't require the hack. Arnd
On Mon, Dec 30, 2019 at 01:12:31PM +0100, Arnd Bergmann wrote: > On Tue, Dec 24, 2019 at 12:08 AM Kirill A. Shutemov > <kirill@shutemov.name> wrote: > > On Mon, Dec 23, 2019 at 02:25:02PM -0800, Khem Raj wrote: > > > On Mon, Dec 23, 2019 at 9:10 AM Kirill A. Shutemov <kirill@shutemov.name> wrote: > > > > > > > > On Sat, Dec 21, 2019 at 07:18:13AM -0800, Khem Raj wrote: > > > > > Since arch/x86/boot/compressed/Makefile overrides global CFLAGS it loses > > > > > -fno-common option which would have caught this > > > > > > > > If this doesn't cause any visible problems, why bother? > > > > > > > > > > it does break builds with gcc trunk as of now e.g. > > > > > > > Hopefully, we will be able to drop it altogether once we ditch GCC 4.X > > > > support. > > > > > > > > > > gcc10 is switching defaults to -fno-common so we need to solve this one way or > > > other, I am not sure if gcc 4.x will be dropped before gcc10 release > > > which would be > > > in mid of 2020 > > > > Okay, it makes sense then. Please include this info into the commit > > message. > > > > Also, I wounder if it would be cleaner to define both of them as __weak? > > Or maybe make the #ifdef check for gcc < 5 instead of checking for > CONFIG_RANDOMIZE_BASE? That way it will be found by whoever > cleans up the code when we increase the minimum compiler > version to one that doesn't require the hack. I agree: that seems a better way to do this. -- Kees Cook
diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c index c8862696a47b..077d19268b0b 100644 --- a/arch/x86/boot/compressed/pgtable_64.c +++ b/arch/x86/boot/compressed/pgtable_64.c @@ -12,7 +12,9 @@ * It is not referenced from the code, but GCC < 5 with -fPIE would fail * due to an undefined symbol. Define it to make these ancient GCCs work. */ +#ifndef CONFIG_RANDOMIZE_BASE unsigned long __force_order; +#endif #define BIOS_START_MIN 0x20000U /* 128K, less than this is insane */ #define BIOS_START_MAX 0x9f000U /* 640K, absolute maximum */
kaslr_64.c also defines the same variable, however when both files are included into final link, linker complains about multiple definition of `__force_order' which is coming from kaslr_64.o and pgtable_64.o, its possible that kaslr_64.o is disabled via CONFIG_RANDOMIZE_BASE config option, therefore define it conditionally only when CONFIG_RANDOMIZE_BASE is not set Since arch/x86/boot/compressed/Makefile overrides global CFLAGS it loses -fno-common option which would have caught this Signed-off-by: Khem Raj <raj.khem@gmail.com> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Cc: Kees Cook <keescook@chromium.org> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Ingo Molnar <mingo@redhat.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: x86-ml <x86@kernel.org> Cc: Arnd Bergmann <arnd@arndb.de> --- arch/x86/boot/compressed/pgtable_64.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.24.1