| Message ID | 20220401143720.23160-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | firmware: build fixes with gcc-11 | expand |
On 01/04/2022 15:37, Roger Pau Monne wrote: > Setting the fcf-protection=none option in EMBEDDED_EXTRA_CFLAGS in the > Makefile doesn't get it propagated to the subdirectories, so instead > set the flag in firmware/Rules.mk, like it's done for other compiler > flags. > > Fixes: 3667f7f8f7 ('x86: Introduce support for CET-IBT') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
On Fri, Apr 01, 2022 at 04:37:18PM +0200, Roger Pau Monne wrote: > Setting the fcf-protection=none option in EMBEDDED_EXTRA_CFLAGS in the > Makefile doesn't get it propagated to the subdirectories, so instead > set the flag in firmware/Rules.mk, like it's done for other compiler > flags. > > Fixes: 3667f7f8f7 ('x86: Introduce support for CET-IBT') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > tools/firmware/Makefile | 2 -- > tools/firmware/Rules.mk | 2 ++ > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile > index 53ed4f161e..345037b93b 100644 > --- a/tools/firmware/Makefile > +++ b/tools/firmware/Makefile > @@ -6,8 +6,6 @@ TARGET := hvmloader/hvmloader > INST_DIR := $(DESTDIR)$(XENFIRMWAREDIR) > DEBG_DIR := $(DESTDIR)$(DEBUG_DIR)$(XENFIRMWAREDIR) > > -EMBEDDED_EXTRA_CFLAGS += -fcf-protection=none > - > SUBDIRS-y := > SUBDIRS-$(CONFIG_OVMF) += ovmf-dir > SUBDIRS-$(CONFIG_SEABIOS) += seabios-dir > diff --git a/tools/firmware/Rules.mk b/tools/firmware/Rules.mk > index 9f78a7dec9..efbbc73a45 100644 > --- a/tools/firmware/Rules.mk > +++ b/tools/firmware/Rules.mk > @@ -13,6 +13,8 @@ endif > > CFLAGS += -Werror > > +EMBEDDED_EXTRA_CFLAGS += -fcf-protection=none > + I think making modification to $(EMBEDDED_EXTRA_CFLAGS) outside of "Config.mk" is confusing and would be better be avoided. Could you instead call $(cc-option-add ) just for that extra CFLAGS? > $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) > > # Extra CFLAGS suitable for an embedded type of environment. Thanks,
On 01/04/2022 15:50, Anthony PERARD wrote: > On Fri, Apr 01, 2022 at 04:37:18PM +0200, Roger Pau Monne wrote: >> Setting the fcf-protection=none option in EMBEDDED_EXTRA_CFLAGS in the >> Makefile doesn't get it propagated to the subdirectories, so instead >> set the flag in firmware/Rules.mk, like it's done for other compiler >> flags. >> >> Fixes: 3667f7f8f7 ('x86: Introduce support for CET-IBT') >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >> --- >> tools/firmware/Makefile | 2 -- >> tools/firmware/Rules.mk | 2 ++ >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile >> index 53ed4f161e..345037b93b 100644 >> --- a/tools/firmware/Makefile >> +++ b/tools/firmware/Makefile >> @@ -6,8 +6,6 @@ TARGET := hvmloader/hvmloader >> INST_DIR := $(DESTDIR)$(XENFIRMWAREDIR) >> DEBG_DIR := $(DESTDIR)$(DEBUG_DIR)$(XENFIRMWAREDIR) >> >> -EMBEDDED_EXTRA_CFLAGS += -fcf-protection=none >> - >> SUBDIRS-y := >> SUBDIRS-$(CONFIG_OVMF) += ovmf-dir >> SUBDIRS-$(CONFIG_SEABIOS) += seabios-dir >> diff --git a/tools/firmware/Rules.mk b/tools/firmware/Rules.mk >> index 9f78a7dec9..efbbc73a45 100644 >> --- a/tools/firmware/Rules.mk >> +++ b/tools/firmware/Rules.mk >> @@ -13,6 +13,8 @@ endif >> >> CFLAGS += -Werror >> >> +EMBEDDED_EXTRA_CFLAGS += -fcf-protection=none >> + > I think making modification to $(EMBEDDED_EXTRA_CFLAGS) outside of > "Config.mk" is confusing and would be better be avoided. EMBEDDED_EXTRA_CFLAGS in the root Config.mk is conceptually broken and needs deleting. Yes, xen/ and tools/firmware/ are freestanding from C's point of view, and embedded from many peoples points of view, but this doesn't mean they have shared build requirements. -nopie isn't even a CFLAG. It's spelt -no-pie and is an LDFLAG. This bug is hidden by everything being cc-option'd behind the scenes. Stack protector we'd absolutely have in Xen if it weren't for a quirk of supporting PV guests. -fno-exceptions is C++ only so not relevant for anything in xen.git ~Andrew
On 01/04/2022 15:48, Andrew Cooper wrote: > On 01/04/2022 15:37, Roger Pau Monne wrote: >> Setting the fcf-protection=none option in EMBEDDED_EXTRA_CFLAGS in the >> Makefile doesn't get it propagated to the subdirectories, so instead >> set the flag in firmware/Rules.mk, like it's done for other compiler >> flags. >> >> Fixes: 3667f7f8f7 ('x86: Introduce support for CET-IBT') >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> This also needs backporting with the XSA-398 CET-IBT fixes. ~Andrew
On Fri, Apr 01, 2022 at 03:04:44PM +0000, Andrew Cooper wrote: > On 01/04/2022 15:50, Anthony PERARD wrote: > > On Fri, Apr 01, 2022 at 04:37:18PM +0200, Roger Pau Monne wrote: > >> +EMBEDDED_EXTRA_CFLAGS += -fcf-protection=none > >> + > > I think making modification to $(EMBEDDED_EXTRA_CFLAGS) outside of > > "Config.mk" is confusing and would be better be avoided. > > EMBEDDED_EXTRA_CFLAGS in the root Config.mk is conceptually broken and > needs deleting. I'm asking to remove "-fcf-protection=none" from the broken variable EMBEDDED_EXTRA_CFLAGS, and instead only modify the CFLAGS variable of "tools/firmware/". As this patch show, making modification to EMBEDDED_EXTRA_CFLAGS outside of Config.mk doesn't work because changes doesn't propagate. So the modification I've ask for the patch goes toward deleting EMBEDDED_EXTRA_CFLAGS, like you want. Cheers,
On 01.04.2022 17:05, Andrew Cooper wrote: > On 01/04/2022 15:48, Andrew Cooper wrote: >> On 01/04/2022 15:37, Roger Pau Monne wrote: >>> Setting the fcf-protection=none option in EMBEDDED_EXTRA_CFLAGS in the >>> Makefile doesn't get it propagated to the subdirectories, so instead >>> set the flag in firmware/Rules.mk, like it's done for other compiler >>> flags. >>> >>> Fixes: 3667f7f8f7 ('x86: Introduce support for CET-IBT') >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > > This also needs backporting with the XSA-398 CET-IBT fixes. I don't think so - the backports of the original commit didn't include what this patch fixes. I have queued patch 2 of this series though. Jan
On 05/04/2022 11:18, Jan Beulich wrote: > On 01.04.2022 17:05, Andrew Cooper wrote: >> On 01/04/2022 15:48, Andrew Cooper wrote: >>> On 01/04/2022 15:37, Roger Pau Monne wrote: >>>> Setting the fcf-protection=none option in EMBEDDED_EXTRA_CFLAGS in the >>>> Makefile doesn't get it propagated to the subdirectories, so instead >>>> set the flag in firmware/Rules.mk, like it's done for other compiler >>>> flags. >>>> >>>> Fixes: 3667f7f8f7 ('x86: Introduce support for CET-IBT') >>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> >> This also needs backporting with the XSA-398 CET-IBT fixes. > I don't think so - the backports of the original commit didn't include > what this patch fixes. I have queued patch 2 of this series though. In which case I screwed up the backport. (I remember spotting this bug and thought I'd corrected it, but clearly not.) tools/firmware really does need to be -fcf-protection=none to counteract the defaults in Ubuntu/etc. ~Andrew
On 05.04.2022 12:58, Andrew Cooper wrote: > On 05/04/2022 11:18, Jan Beulich wrote: >> On 01.04.2022 17:05, Andrew Cooper wrote: >>> On 01/04/2022 15:48, Andrew Cooper wrote: >>>> On 01/04/2022 15:37, Roger Pau Monne wrote: >>>>> Setting the fcf-protection=none option in EMBEDDED_EXTRA_CFLAGS in the >>>>> Makefile doesn't get it propagated to the subdirectories, so instead >>>>> set the flag in firmware/Rules.mk, like it's done for other compiler >>>>> flags. >>>>> >>>>> Fixes: 3667f7f8f7 ('x86: Introduce support for CET-IBT') >>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >>>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> This also needs backporting with the XSA-398 CET-IBT fixes. >> I don't think so - the backports of the original commit didn't include >> what this patch fixes. I have queued patch 2 of this series though. > > In which case I screwed up the backport. (I remember spotting this bug > and thought I'd corrected it, but clearly not.) tools/firmware really > does need to be -fcf-protection=none to counteract the defaults in > Ubuntu/etc. Okay, I'll adjust title and description some then while doing the backport. Jan
On 05/04/2022 12:04, Jan Beulich wrote: > On 05.04.2022 12:58, Andrew Cooper wrote: >> On 05/04/2022 11:18, Jan Beulich wrote: >>> On 01.04.2022 17:05, Andrew Cooper wrote: >>>> On 01/04/2022 15:48, Andrew Cooper wrote: >>>>> On 01/04/2022 15:37, Roger Pau Monne wrote: >>>>>> Setting the fcf-protection=none option in EMBEDDED_EXTRA_CFLAGS in the >>>>>> Makefile doesn't get it propagated to the subdirectories, so instead >>>>>> set the flag in firmware/Rules.mk, like it's done for other compiler >>>>>> flags. >>>>>> >>>>>> Fixes: 3667f7f8f7 ('x86: Introduce support for CET-IBT') >>>>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >>>>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> >>>> This also needs backporting with the XSA-398 CET-IBT fixes. >>> I don't think so - the backports of the original commit didn't include >>> what this patch fixes. I have queued patch 2 of this series though. >> In which case I screwed up the backport. (I remember spotting this bug >> and thought I'd corrected it, but clearly not.) tools/firmware really >> does need to be -fcf-protection=none to counteract the defaults in >> Ubuntu/etc. > Okay, I'll adjust title and description some then while doing the backport. Thanks, and sorry for this mess. ~Andrew
diff --git a/tools/firmware/Makefile b/tools/firmware/Makefile index 53ed4f161e..345037b93b 100644 --- a/tools/firmware/Makefile +++ b/tools/firmware/Makefile @@ -6,8 +6,6 @@ TARGET := hvmloader/hvmloader INST_DIR := $(DESTDIR)$(XENFIRMWAREDIR) DEBG_DIR := $(DESTDIR)$(DEBUG_DIR)$(XENFIRMWAREDIR) -EMBEDDED_EXTRA_CFLAGS += -fcf-protection=none - SUBDIRS-y := SUBDIRS-$(CONFIG_OVMF) += ovmf-dir SUBDIRS-$(CONFIG_SEABIOS) += seabios-dir diff --git a/tools/firmware/Rules.mk b/tools/firmware/Rules.mk index 9f78a7dec9..efbbc73a45 100644 --- a/tools/firmware/Rules.mk +++ b/tools/firmware/Rules.mk @@ -13,6 +13,8 @@ endif CFLAGS += -Werror +EMBEDDED_EXTRA_CFLAGS += -fcf-protection=none + $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) # Extra CFLAGS suitable for an embedded type of environment.
Setting the fcf-protection=none option in EMBEDDED_EXTRA_CFLAGS in the Makefile doesn't get it propagated to the subdirectories, so instead set the flag in firmware/Rules.mk, like it's done for other compiler flags. Fixes: 3667f7f8f7 ('x86: Introduce support for CET-IBT') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- tools/firmware/Makefile | 2 -- tools/firmware/Rules.mk | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-)