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 <[email protected]>
Cc: Kirill A. Shutemov <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: x86-ml <[email protected]>
Cc: Arnd Bergmann <[email protected]>
---
arch/x86/boot/compressed/pgtable_64.c | 2 ++
1 file changed, 2 insertions(+)
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 */
--
2.24.1
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 <[email protected]> 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 <[email protected]> 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
<[email protected]> 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 <[email protected]> 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
> <[email protected]> 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 <[email protected]> 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