[PATCH] mini-os: Avoid segfaults in tc{g,s}etattr

[PATCH] mini-os: Avoid segfaults in tc{g,s}etattr

[Jason Andryuk]

Commit c96c22f1d94 "mini-os: minimal implementations of some termios
functions" introduced implementations of tcgetattr and tcsetattr.
However, they do not check if files[fildes].cons.dev is non-NULL before
dereferencing.  This is not a problem for FDs allocated through
alloc_fd, but the files array pre-allocates FDs 0-2 for stdio.  Those
entries have a NULL .dev, so tc{g,s}etattr on them segfault.

ioemu-stubdom segfaults when term_init() calls tcgetattr on FD 0.

Restore tcgetattr and tcsetattr behavior when .dev is NULL equivalent to
unsupported_function as it was before c96c22f1d94.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
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.

 lib/sys.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/sys.c b/lib/sys.c
index da434fc..c6a7b9f 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -1472,6 +1472,11 @@ int tcsetattr(int fildes, int action, const struct termios *tios)
             return -1;
     }
 
+    if (files[fildes].cons.dev == NULL) {
+        errno = ENOSYS;
+        return -1;
+    }
+
     if (tios->c_oflag & OPOST)
         files[fildes].cons.dev->is_raw = false;
     else
@@ -1492,6 +1497,11 @@ int tcgetattr(int fildes, struct termios *tios)
         return -1;
     }
 
+    if (files[fildes].cons.dev == NULL) {
+        errno = ENOSYS;
+        return 0;
+    }
+
     if (tios == NULL) {
         errno = EINVAL;
         return -1;
-- 
2.20.1

Re: [PATCH] mini-os: Avoid segfaults in tc{g,s}etattr

[Samuel Thibault]

Jason Andryuk, le dim. 26 avril 2020 23:40:19 -0400, a ecrit:
> Commit c96c22f1d94 "mini-os: minimal implementations of some termios
> functions" introduced implementations of tcgetattr and tcsetattr.
> However, they do not check if files[fildes].cons.dev is non-NULL before
> dereferencing.  This is not a problem for FDs allocated through
> alloc_fd, but the files array pre-allocates FDs 0-2 for stdio.  Those
> entries have a NULL .dev, so tc{g,s}etattr on them segfault.
> 
> ioemu-stubdom segfaults when term_init() calls tcgetattr on FD 0.
> 
> Restore tcgetattr and tcsetattr behavior when .dev is NULL equivalent to
> unsupported_function as it was before c96c22f1d94.
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

Thanks!

> ---
> 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.
> 
>  lib/sys.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/lib/sys.c b/lib/sys.c
> index da434fc..c6a7b9f 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -1472,6 +1472,11 @@ int tcsetattr(int fildes, int action, const struct termios *tios)
>              return -1;
>      }
>  
> +    if (files[fildes].cons.dev == NULL) {
> +        errno = ENOSYS;
> +        return -1;
> +    }
> +
>      if (tios->c_oflag & OPOST)
>          files[fildes].cons.dev->is_raw = false;
>      else
> @@ -1492,6 +1497,11 @@ int tcgetattr(int fildes, struct termios *tios)
>          return -1;
>      }
>  
> +    if (files[fildes].cons.dev == NULL) {
> +        errno = ENOSYS;
> +        return 0;
> +    }
> +
>      if (tios == NULL) {
>          errno = EINVAL;
>          return -1;
> -- 
> 2.20.1
> 

Re: [PATCH] mini-os: Avoid segfaults in tc{g,s}etattr

[Jason Andryuk]

On Mon, Apr 27, 2020 at 3:54 AM Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
>
> Jason Andryuk, le dim. 26 avril 2020 23:40:19 -0400, a ecrit:
> > Commit c96c22f1d94 "mini-os: minimal implementations of some termios
> > functions" introduced implementations of tcgetattr and tcsetattr.
> > However, they do not check if files[fildes].cons.dev is non-NULL before
> > dereferencing.  This is not a problem for FDs allocated through
> > alloc_fd, but the files array pre-allocates FDs 0-2 for stdio.  Those
> > entries have a NULL .dev, so tc{g,s}etattr on them segfault.
> >
> > ioemu-stubdom segfaults when term_init() calls tcgetattr on FD 0.
> >
> > Restore tcgetattr and tcsetattr behavior when .dev is NULL equivalent to
> > unsupported_function as it was before c96c22f1d94.
> >
> > Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
>
> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
>
> Thanks!

Thank you!

> > ---
> > 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.  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 .

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/tools/firmware/Rules.mk b/tools/firmware/Rules.mk
index 26bbddccd4..0d33514d53 100644
--- a/tools/firmware/Rules.mk
+++ b/tools/firmware/Rules.mk
@@ -17,3 +17,4 @@ $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))

 # Extra CFLAGS suitable for an embedded type of environment.
 CFLAGS += -fno-builtin -msoft-float
+CFLAGS += -fcf-protection=none

Re: [PATCH] mini-os: Avoid segfaults in tc{g,s}etattr

[Wei Liu]

On Mon, Apr 27, 2020 at 09:30:50AM -0400, Jason Andryuk wrote:
> On Mon, Apr 27, 2020 at 3:54 AM Samuel Thibault
> <samuel.thibault@ens-lyon.org> wrote:
> >
> > Jason Andryuk, le dim. 26 avril 2020 23:40:19 -0400, a ecrit:
> > > Commit c96c22f1d94 "mini-os: minimal implementations of some termios
> > > functions" introduced implementations of tcgetattr and tcsetattr.
> > > However, they do not check if files[fildes].cons.dev is non-NULL before
> > > dereferencing.  This is not a problem for FDs allocated through
> > > alloc_fd, but the files array pre-allocates FDs 0-2 for stdio.  Those
> > > entries have a NULL .dev, so tc{g,s}etattr on them segfault.
> > >
> > > ioemu-stubdom segfaults when term_init() calls tcgetattr on FD 0.
> > >
> > > Restore tcgetattr and tcsetattr behavior when .dev is NULL equivalent to
> > > unsupported_function as it was before c96c22f1d94.
> > >
> > > Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> >
> > Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> >
> > Thanks!
> 
> Thank you!
> 
> > > ---
> > > 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.  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.

> 
> 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/tools/firmware/Rules.mk b/tools/firmware/Rules.mk
> index 26bbddccd4..0d33514d53 100644
> --- a/tools/firmware/Rules.mk
> +++ b/tools/firmware/Rules.mk
> @@ -17,3 +17,4 @@ $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
> 
>  # Extra CFLAGS suitable for an embedded type of environment.
>  CFLAGS += -fno-builtin -msoft-float
> +CFLAGS += -fcf-protection=none
> 

Re: [PATCH] mini-os: Avoid segfaults in tc{g,s}etattr

[Andrew Cooper]

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.

~Andrew

Re: [PATCH] mini-os: Avoid segfaults in tc{g,s}etattr

[Jason Andryuk]

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

Re: [PATCH] mini-os: Avoid segfaults in tc{g,s}etattr

[Andrew Cooper]

On 28/04/2020 12:44, Jason Andryuk wrote:
> 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

Yes.  This is false.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93654

but sadly tumbleweeds.

I'll start a thread on the email list.

>
> 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>

Sadly, this isn't really appropriate.  We specifically do want to use
both -fcf-protection and -mindirect-branch=thunk-extern together, when
GCC isn't broken.

Overriding -fcf-protection is ok but only when we're certain we've got a
buggy GCC, so that when this bug is fixed, we can return to sensible
behaviour.

~Andrew

> ---
>  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

Re: [PATCH] mini-os: Avoid segfaults in tc{g,s}etattr

[Andrew Cooper]

On 28/04/2020 12:55, Andrew Cooper wrote:
>> 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>
> Sadly, this isn't really appropriate.  We specifically do want to use
> both -fcf-protection and -mindirect-branch=thunk-extern together, when
> GCC isn't broken.
>
> Overriding -fcf-protection is ok but only when we're certain we've got a
> buggy GCC, so that when this bug is fixed, we can return to sensible
> behaviour.

GCC has been adjusted on master
(https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=9be3bb2c0a258fd6a7d3d05d232a21930c757d3c)
and the gcc-9 branch
(https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=a03efb266fcbf4a01285fff871a5bfe5caac4944). 
This should be fixed for GCC 10 and 9.4

I checked the resulting hypervisor build with both -fcf-protection and
retpolines, and it works fine.

The question now is what to do all the buggy GCCs out there.  We can
either ignore the problem and it will eventually go away, or spot the
problematic compiler and clobber -fcf-protection.

We also need to see what is wrong with RomBIOS, because that is weird. 
However, we should not be interfering with the HOSTCC settings.

~Andrew

rombios triple fault with -fcf-protection

[Jason Andryuk]

This is the output from a failed rombios boot:

(d11) Invoking ROMBIOS ...
(XEN) stdvga.c:173:d11v0 entering stdvga mode
(d11) VGABios $Id: vgabios.c,v 1.67 2008/01/27 09:44:12 vruppert Exp $
(d11) Bochs BIOS - build: 06/23/99
(d11) $Revision: 1.221 $ $Date: 2008/12/07 17:32:29 $
(d11) Options: apmbios pcibios eltorito PMM
(d11)
(d11) ata0 master: QEMU HARDDISK ATA-7 Hard-Disk ( 112 MBytes)
(d11)
(XEN) MMIO emulation failed (1): d11v0 32bit @ 0008:ffffffff ->
(XEN) d11v0 Triple fault - invoking HVM shutdown action 1
(XEN) *** Dumping Dom11 vcpu#0 state: ***
(XEN) ----[ Xen-4.14-unstable  x86_64  debug=y   Not tainted ]----
(XEN) CPU:    1
(XEN) RIP:    0008:[<00000000ffffffff>]
(XEN) RFLAGS: 0000000000010086   CONTEXT: hvm guest (d11v0)
(XEN) rax: 0000000043001000   rbx: 000000000009c30e   rcx: 0000000000000000
(XEN) rdx: 0000000000000000   rsi: 000000000000031e   rdi: 0000000000000020
(XEN) rbp: 00000000000002de   rsp: 000000000009ef48   r8:  0000000000000000
(XEN) r9:  0000000000000000   r10: 0000000000000000   r11: 0000000000000000
(XEN) r12: 0000000000000000   r13: 0000000000000000   r14: 0000000000000000
(XEN) r15: 0000000000000000   cr0: 0000000000000011   cr4: 0000000000000000
(XEN) cr3: 0000000000400000   cr2: 0000000000000000
(XEN) fsb: 0000000000000000   gsb: 00000000000c9000   gss: 0000000000000002
(XEN) ds: 0018   es: 0018   fs: 0000   gs: c900   ss: 0018   cs: 0008

For a traditional stubdom with sdl enabled, I also see the init_message of iPXE
iPXE (http://ipxe.org) 00:04.0 C900 PCI2.10 PMM_

The underscore might be the cursor?

The VM reboots.  If I set on_reboot="preserve" and use gdbsx:

(gdb) target remote :1234
Remote debugging using :1234
0xdeadf00d in ?? ()
(gdb) bt
#0  0xdeadf00d in ?? ()
(gdb) i r
eax            0x0                 0
ecx            0x0                 0
edx            0xe5a08016          -442466282
ebx            0x87                135
esp            0x23aa              0x23aa <memset+22>
ebp            0x0                 0x0
esi            0x0                 0
edi            0x0                 0
eip            0xdeadf00d          0xdeadf00d
eflags         0xdeadbeef          [ CF PF ZF SF IF DF OF RF AC VIF ID ]
cs             0xdeadf00d          -559026163
ss             0xdeadbeef          -559038737
ds             0x5ef890            6224016
es             0x0                 0
fs             0x5ef868            6223976
gs             0x0                 0

Re: [PATCH] mini-os: Avoid segfaults in tc{g,s}etattr

[Wei Liu]

On Mon, Apr 27, 2020 at 09:54:29AM +0200, Samuel Thibault wrote:
> Jason Andryuk, le dim. 26 avril 2020 23:40:19 -0400, a ecrit:
> > Commit c96c22f1d94 "mini-os: minimal implementations of some termios
> > functions" introduced implementations of tcgetattr and tcsetattr.
> > However, they do not check if files[fildes].cons.dev is non-NULL before
> > dereferencing.  This is not a problem for FDs allocated through
> > alloc_fd, but the files array pre-allocates FDs 0-2 for stdio.  Those
> > entries have a NULL .dev, so tc{g,s}etattr on them segfault.
> > 
> > ioemu-stubdom segfaults when term_init() calls tcgetattr on FD 0.
> > 
> > Restore tcgetattr and tcsetattr behavior when .dev is NULL equivalent to
> > unsupported_function as it was before c96c22f1d94.
> > 
> > Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> 
> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> 
> Thanks!

Applied. Thanks.