From: Jason Andryuk <jandryuk@gmail.com>
To: andrew.cooper3@citrix.com, Wei Liu <wl@xen.org>,
Jason Andryuk <jandryuk@gmail.com>
Cc: minios-devel@lists.xenproject.org, samuel.thibault@ens-lyon.org,
Stefan Bader <stefan.bader@canonical.com>,
JBeulich@suse.com, xen-devel@lists.xenproject.org
Subject: Re: [PATCH] mini-os: Avoid segfaults in tc{g,s}etattr
Date: Tue, 28 Apr 2020 07:44:07 -0400 [thread overview]
Message-ID: <5ea8173d.1c69fb81.915ba.8400@mx.google.com> (raw)
In-Reply-To: <20200428111645.pa6xfs6t6rifu6fu@liuwe-devbox-debian-v2.j3c5onc20sse1dnehy4noqpfcg.zx.internal.cloudapp.net>
From: Andrew Cooper <andrew.cooper3@citrix.com>
Andrew Cooper wrote:
>On 28/04/2020 12:16, Wei Liu wrote:
>>>>> ---
>>>>> I can't get ioemu-stubdom to start without this. With this, the guest
>>>>> just reboots immediately, but it does that with a non-stubdom
>>>>> device_model_version="qemu-xen-traditional" . The same guest disk image
>>>>> (cirros 0.5.1) boots with a linux stubdom or non-stubdom Ubuntu
>>>>> qemu-system-x86_64.
>>> Ubuntu gcc-9 adds -fcf-protection by default. Somehow that flag
>>> caused rombios (I think) to restart. Setting -fcf-protection=none
>>> like below (probably just the EMBEDDED_EXTRA_CFLAGS part) lets rombios
>>> start properly.
>
>All it does is insert ENDBR{32,64} instructions, which are nops on older
>processors.
>
>I suspect that it is not the -fcf-protection= directly, but some change
>in alignment of a critical function.
>
>>> The hypervisor needs it as well via
>>> EXTRA_CFLAGS_XEN_CORE=-fcf-protection=none and maybe also added to
>>> xen/arch/x86/boot/build32.mk .
>> Are you able to turn this into a proper patch? I suspect you will need
>> to test the availability of this new (?) flag.
>>
>> Also Cc Jan and Andrew because it affects hypervisor build too.
>
>I need to chase this up. It is a GCC bug breaking the hypervisor build,
>and I'm moderately disinclined to hack around it, seeing as
>-fcf-protection is something we want in due course.
>
>The bug is that GCC falsely declares that -fcf-protection is
>incompatible with -mindirect-thunk=extern, despite me spending a week
>during the Spectre embargo period specifically arranging for the two to
>be compatible, because we knew we'd want to build retpoline-safe
>binaries which could also use CET on newer hardware.
The gcc manual states:
"Note that -mindirect-branch=thunk-extern is incompatible with
-fcf-protection=branch since the external thunk cannot be modified
to disable control-flow check."
https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html
Below is what I was preparing to submit as a patch. So, yes it hacks around
it, but it isn't messy.
---
Disable fcf-protection to build working binaries
Ubuntu gcc-9 enables -fcf-protection by default, which conflicts with
-mindirect-branch=extern and prevents building the hypervisor with
CONFIG_INDIRECT_THUNK:
xmalloc.h:81:1: error: ‘-mindirect-branch’ and ‘-fcf-protection’ are not
compatible
Stefan Bader also noticed that build32.mk requires -fcf-protection=none
or else the hypervisor will not boot.
https://bugs.launchpad.net/ubuntu/+source/gcc-9/+bug/1863260 Similarly,
rombios reboots almost immediately without -fcf-protection=none. Both
of those can be handled by setting it in EMBEDDED_EXTRA_CFLAGS.
CC: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
Config.mk | 1 +
xen/arch/x86/Rules.mk | 1 +
2 files changed, 2 insertions(+)
diff --git a/Config.mk b/Config.mk
index 0f303c79b2..efb3d42bc4 100644
--- a/Config.mk
+++ b/Config.mk
@@ -205,6 +205,7 @@ APPEND_CFLAGS += $(foreach i, $(APPEND_INCLUDES), -I$(i))
EMBEDDED_EXTRA_CFLAGS := -nopie -fno-stack-protector -fno-stack-protector-all
EMBEDDED_EXTRA_CFLAGS += -fno-exceptions
+EMBEDDED_EXTRA_CFLAGS += -fcf-protection=none
XEN_EXTFILES_URL ?= http://xenbits.xen.org/xen-extfiles
# All the files at that location were downloaded from elsewhere on
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 4b7ab78467..c3cbae69d2 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -69,6 +69,7 @@ CFLAGS += -mno-sse $(call cc-option,$(CC),-mskip-rax-setup)
ifeq ($(CONFIG_INDIRECT_THUNK),y)
CFLAGS += -mindirect-branch=thunk-extern -mindirect-branch-register
CFLAGS += -fno-jump-tables
+$(call cc-option-add,CFLAGS,CC,-fcf-protection=none)
endif
# If supported by the compiler, reduce stack alignment to 8 bytes. But allow
--
2.20.1
next prev parent reply other threads:[~2020-04-28 11:45 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-27 3:40 [PATCH] mini-os: Avoid segfaults in tc{g,s}etattr Jason Andryuk
2020-04-27 7:54 ` Samuel Thibault
2020-04-27 13:30 ` Jason Andryuk
2020-04-28 11:16 ` Wei Liu
2020-04-28 11:24 ` Andrew Cooper
2020-04-28 11:44 ` Jason Andryuk [this message]
2020-04-28 11:55 ` Andrew Cooper
2020-04-29 12:34 ` Andrew Cooper
2020-05-12 19:31 ` rombios triple fault with -fcf-protection Jason Andryuk
2020-04-28 11:18 ` [PATCH] mini-os: Avoid segfaults in tc{g,s}etattr Wei Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5ea8173d.1c69fb81.915ba.8400@mx.google.com \
--to=jandryuk@gmail.com \
--cc=JBeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=minios-devel@lists.xenproject.org \
--cc=samuel.thibault@ens-lyon.org \
--cc=stefan.bader@canonical.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).