diff mbox series

xen/build: Fix build failure from LDFLAGS mismatch

Message ID 20220425230656.12808-1-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series xen/build: Fix build failure from LDFLAGS mismatch | expand

Commit Message

Andrew Cooper April 25, 2022, 11:06 p.m. UTC
In a GNU compatbile makefile, $(LDFLAGS) are passed to $(CC), not $(LD).

In a default CentOS 7 build environment, $(LDFLAGS) is set to -Wl,-z,relro,
which causes the Xen build to explode with:

  ld: unrecognized option '-Wl,-z,relro'
  ld: use the --help option for usage information

It turns out that many downstreams identify this as a breakage in Xen's build
system and bodge around it in various ways, mostly by unsetting all of
$(CFLAGS), $(AFLAGS) and $(LDFLAGS).

However, that is a security issue because it means that tools/ is not built
with the distro-wide hardening flags that are otherwise expected of
packages (relro, _FORTIFY_SOURCE, stack-protector, etc).

tools/ specifically should honour the packaging environment's choice of flags,
while xen/ must not pass $(LDFLAGS) to $(LD), and should not be influenced by
the others either.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@citrix.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Juergen Gross <jgross@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Wei Liu <wl@xen.org>

RFC, because CFLAGS/AFLAGS need nuking too, and they're rather more entangled.
I expect this to cause some disgreement, but Xen is behaving in a very
nonstandard way even among embedded projects and all downstreams are suffering
security problems as a consequence.
---
 xen/Makefile | 2 ++
 1 file changed, 2 insertions(+)

Comments

Jan Beulich April 26, 2022, 8:04 a.m. UTC | #1
On 26.04.2022 01:06, Andrew Cooper wrote:
> In a GNU compatbile makefile, $(LDFLAGS) are passed to $(CC), not $(LD).

I have to admit that I have always been puzzled by this anomaly.

> In a default CentOS 7 build environment, $(LDFLAGS) is set to -Wl,-z,relro,
> which causes the Xen build to explode with:
> 
>   ld: unrecognized option '-Wl,-z,relro'
>   ld: use the --help option for usage information
> 
> It turns out that many downstreams identify this as a breakage in Xen's build
> system and bodge around it in various ways, mostly by unsetting all of
> $(CFLAGS), $(AFLAGS) and $(LDFLAGS).
> 
> However, that is a security issue because it means that tools/ is not built
> with the distro-wide hardening flags that are otherwise expected of
> packages (relro, _FORTIFY_SOURCE, stack-protector, etc).

This "security issue" is introduced by them, I would say. They simply
shouldn't build everything in one go, but rather build "tools" with
the flags left intact and "xen" with the flags suitably pruned. (We
do build "xen" separately, albeit for different reasons.) The way
./Config.mk works this would look to be advisable anyway. And
xen/Makefile should perhaps arrange for Config.mk to skip this
massaging when including it.

> tools/ specifically should honour the packaging environment's choice of flags,
> while xen/ must not pass $(LDFLAGS) to $(LD), and should not be influenced by
> the others either.

I'm not convinced of the last part of what you say. Why should it be
impossible to say "make CFLAGS=... xen" just like "make CC=... xen"
can be used?

> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -254,6 +254,8 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig
>  # reparsing Config.mk by e.g. arch/x86/boot/.
>  export XEN_TREEWIDE_CFLAGS := $(CFLAGS)
>  
> +LDFLAGS :=

Instead of this, why don't we do away with the few uses of $(LDFLAGS)?
If I haven't overlooked anything, there are exactly two lines (three
if also counting a comment) which would need changing.

Or why don't we transform -Wl,... into the form understood by $(LD)?
-z relro, for example, looks to be benign to the linking of Xen, the
more that this option can also be enabled by default and we haven't
found a need to disable it (afaics this option solely determines
which linker script to use when none was specified).

Jan
Bertrand Marquis April 26, 2022, 10:20 a.m. UTC | #2
Hi Andrew,

> On 26 Apr 2022, at 00:06, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> In a GNU compatbile makefile, $(LDFLAGS) are passed to $(CC), not $(LD).

You mean because CC is used for linking or even when compiling object files ?

If not, what is the expected way to pass linker flags ?

> 
> In a default CentOS 7 build environment, $(LDFLAGS) is set to -Wl,-z,relro,
> which causes the Xen build to explode with:
> 
>  ld: unrecognized option '-Wl,-z,relro'
>  ld: use the --help option for usage information
> 
> It turns out that many downstreams identify this as a breakage in Xen's build
> system and bodge around it in various ways, mostly by unsetting all of
> $(CFLAGS), $(AFLAGS) and $(LDFLAGS).
> 
> However, that is a security issue because it means that tools/ is not built
> with the distro-wide hardening flags that are otherwise expected of
> packages (relro, _FORTIFY_SOURCE, stack-protector, etc).
> 
> tools/ specifically should honour the packaging environment's choice of flags,
> while xen/ must not pass $(LDFLAGS) to $(LD), and should not be influenced by
> the others either.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Anthony PERARD <anthony.perard@citrix.com>
> CC: Bertrand Marquis <bertrand.marquis@arm.com>
> CC: George Dunlap <George.Dunlap@eu.citrix.com>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Julien Grall <julien@xen.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> CC: Wei Liu <wl@xen.org>
> 
> RFC, because CFLAGS/AFLAGS need nuking too, and they're rather more entangled.
> I expect this to cause some disgreement, but Xen is behaving in a very
> nonstandard way even among embedded projects and all downstreams are suffering
> security problems as a consequence.
> ---
> xen/Makefile | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/xen/Makefile b/xen/Makefile
> index ec34524ed21d..a8e1de54823b 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -254,6 +254,8 @@ export KBUILD_DEFCONFIG := $(ARCH)_defconfig
> # reparsing Config.mk by e.g. arch/x86/boot/.
> export XEN_TREEWIDE_CFLAGS := $(CFLAGS)
> 
> +LDFLAGS :=
> +

This would require a comment in the Makefile to explain why this is done.

Also how could anybody specify linker specific flags if this is done ?

Regards
Bertrand


> # CLANG_FLAGS needs to be calculated before calling Kconfig
> ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
> CLANG_FLAGS :=
> -- 
> 2.11.0
>
diff mbox series

Patch

diff --git a/xen/Makefile b/xen/Makefile
index ec34524ed21d..a8e1de54823b 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -254,6 +254,8 @@  export KBUILD_DEFCONFIG := $(ARCH)_defconfig
 # reparsing Config.mk by e.g. arch/x86/boot/.
 export XEN_TREEWIDE_CFLAGS := $(CFLAGS)
 
+LDFLAGS :=
+
 # CLANG_FLAGS needs to be calculated before calling Kconfig
 ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
 CLANG_FLAGS :=