linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/boot/compressed/64: Define __force_order only when CONFIG_RANDOMIZE_BASE is unset
@ 2019-12-21 15:18 Khem Raj
  2019-12-23 17:10 ` Kirill A. Shutemov
  0 siblings, 1 reply; 6+ messages in thread
From: Khem Raj @ 2019-12-21 15:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: Khem Raj, Kirill A . Shutemov, Kees Cook, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86-ml, Arnd Bergmann

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(+)

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/boot/compressed/64: Define __force_order only when CONFIG_RANDOMIZE_BASE is unset
  2019-12-21 15:18 [PATCH] x86/boot/compressed/64: Define __force_order only when CONFIG_RANDOMIZE_BASE is unset Khem Raj
@ 2019-12-23 17:10 ` Kirill A. Shutemov
  2019-12-23 22:25   ` Khem Raj
  0 siblings, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2019-12-23 17:10 UTC (permalink / raw)
  To: Khem Raj
  Cc: linux-kernel, Kirill A . Shutemov, Kees Cook, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86-ml, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/boot/compressed/64: Define __force_order only when CONFIG_RANDOMIZE_BASE is unset
  2019-12-23 17:10 ` Kirill A. Shutemov
@ 2019-12-23 22:25   ` Khem Raj
  2019-12-23 23:08     ` Kirill A. Shutemov
  0 siblings, 1 reply; 6+ messages in thread
From: Khem Raj @ 2019-12-23 22:25 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: linux-kernel, Kirill A . Shutemov, Kees Cook, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86-ml, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/boot/compressed/64: Define __force_order only when CONFIG_RANDOMIZE_BASE is unset
  2019-12-23 22:25   ` Khem Raj
@ 2019-12-23 23:08     ` Kirill A. Shutemov
  2019-12-30 12:12       ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2019-12-23 23:08 UTC (permalink / raw)
  To: Khem Raj
  Cc: linux-kernel, Kirill A . Shutemov, Kees Cook, H. Peter Anvin,
	Ingo Molnar, Thomas Gleixner, x86-ml, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/boot/compressed/64: Define __force_order only when CONFIG_RANDOMIZE_BASE is unset
  2019-12-23 23:08     ` Kirill A. Shutemov
@ 2019-12-30 12:12       ` Arnd Bergmann
  2019-12-30 18:34         ` Kees Cook
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2019-12-30 12:12 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: Khem Raj, linux-kernel, Kirill A . Shutemov, Kees Cook,
	H. Peter Anvin, Ingo Molnar, Thomas Gleixner, x86-ml

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86/boot/compressed/64: Define __force_order only when CONFIG_RANDOMIZE_BASE is unset
  2019-12-30 12:12       ` Arnd Bergmann
@ 2019-12-30 18:34         ` Kees Cook
  0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2019-12-30 18:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Kirill A. Shutemov, Khem Raj, linux-kernel, Kirill A . Shutemov,
	H. Peter Anvin, Ingo Molnar, Thomas Gleixner, x86-ml

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-12-30 18:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-21 15:18 [PATCH] x86/boot/compressed/64: Define __force_order only when CONFIG_RANDOMIZE_BASE is unset Khem Raj
2019-12-23 17:10 ` Kirill A. Shutemov
2019-12-23 22:25   ` Khem Raj
2019-12-23 23:08     ` Kirill A. Shutemov
2019-12-30 12:12       ` Arnd Bergmann
2019-12-30 18:34         ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).