| Message ID | 20190313194314.5664-1-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: selftests: disable stack protector for all KVM tests | expand |
On Wed, Mar 13, 2019 at 12:43:14PM -0700, Sean Christopherson wrote: > Since 4.8.3, gcc has enabled -fstack-protector by default. This is > problematic for the KVM selftests as they do not configure fs or gs > segments (the stack canary is pulled from fs:0x28). With the default > behavior, gcc will insert a stack canary on any function that creates > buffers of 8 bytes or more. As a result, ucall() will hit a triple > fault shutdown due to reading a bad fs segment when inserting its > stack canary, i.e. every test fails with an unexpected SHUTDOWN. > > Fixes: 14c47b7530e2d ("kvm: selftests: introduce ucall") > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > tools/testing/selftests/kvm/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > index 3c1f4bdf9000..596c0864d0df 100644 > --- a/tools/testing/selftests/kvm/Makefile > +++ b/tools/testing/selftests/kvm/Makefile > @@ -29,7 +29,7 @@ LIBKVM += $(LIBKVM_$(UNAME_M)) > INSTALL_HDR_PATH = $(top_srcdir)/usr > LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/ > LINUX_TOOL_INCLUDE = $(top_srcdir)/tools/include > -CFLAGS += -O2 -g -std=gnu99 -I$(LINUX_TOOL_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(UNAME_M) -I.. > +CFLAGS += -O2 -g -std=gnu99 -fno-stack-protector -I$(LINUX_TOOL_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(UNAME_M) -I.. > LDFLAGS += -pthread > > # After inclusion, $(OUTPUT) is defined and Please ignore this patch, I figured out why the TLS wasn't in the elf headers, which is a separate fix of its own. With the TLS in hand a better fix is to actually set fs.
On Wed, Mar 13, 2019 at 03:56:25PM -0700, Sean Christopherson wrote: > On Wed, Mar 13, 2019 at 12:43:14PM -0700, Sean Christopherson wrote: > > Since 4.8.3, gcc has enabled -fstack-protector by default. This is > > problematic for the KVM selftests as they do not configure fs or gs > > segments (the stack canary is pulled from fs:0x28). With the default > > behavior, gcc will insert a stack canary on any function that creates > > buffers of 8 bytes or more. As a result, ucall() will hit a triple > > fault shutdown due to reading a bad fs segment when inserting its > > stack canary, i.e. every test fails with an unexpected SHUTDOWN. > > > > Fixes: 14c47b7530e2d ("kvm: selftests: introduce ucall") > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > --- > > tools/testing/selftests/kvm/Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > > index 3c1f4bdf9000..596c0864d0df 100644 > > --- a/tools/testing/selftests/kvm/Makefile > > +++ b/tools/testing/selftests/kvm/Makefile > > @@ -29,7 +29,7 @@ LIBKVM += $(LIBKVM_$(UNAME_M)) > > INSTALL_HDR_PATH = $(top_srcdir)/usr > > LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/ > > LINUX_TOOL_INCLUDE = $(top_srcdir)/tools/include > > -CFLAGS += -O2 -g -std=gnu99 -I$(LINUX_TOOL_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(UNAME_M) -I.. > > +CFLAGS += -O2 -g -std=gnu99 -fno-stack-protector -I$(LINUX_TOOL_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(UNAME_M) -I.. > > LDFLAGS += -pthread > > > > # After inclusion, $(OUTPUT) is defined and > > Please ignore this patch, I figured out why the TLS wasn't in the elf > headers, which is a separate fix of its own. With the TLS in hand a > better fix is to actually set fs. Jumped the gun a bit... Turns out the other issue was due to recent gcc versions enabling pie by default. The "separate fix" I referred to was to build the tests with '-static', whose side effect was to disable pie. Another side side effect of '-static' is that the TLS section is listed in the elf headers, which in turn allows the KVM selftests to define a legitimate fs.base for the stack canary. I have working code to point fs at the TLS, so we can easily go that route if we want to. But AFAICT, building with '-static' and accessing TLS in the guest is unecessary (except for the stack canary). Overall I think disabling the stack protector is a little less ugly than building a static binary and adding all of the TLS/fs logic.
On 13/03/19 20:43, Sean Christopherson wrote: > Since 4.8.3, gcc has enabled -fstack-protector by default. This is > problematic for the KVM selftests as they do not configure fs or gs > segments (the stack canary is pulled from fs:0x28). With the default > behavior, gcc will insert a stack canary on any function that creates > buffers of 8 bytes or more. As a result, ucall() will hit a triple > fault shutdown due to reading a bad fs segment when inserting its > stack canary, i.e. every test fails with an unexpected SHUTDOWN. > > Fixes: 14c47b7530e2d ("kvm: selftests: introduce ucall") > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > tools/testing/selftests/kvm/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile > index 3c1f4bdf9000..596c0864d0df 100644 > --- a/tools/testing/selftests/kvm/Makefile > +++ b/tools/testing/selftests/kvm/Makefile > @@ -29,7 +29,7 @@ LIBKVM += $(LIBKVM_$(UNAME_M)) > INSTALL_HDR_PATH = $(top_srcdir)/usr > LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/ > LINUX_TOOL_INCLUDE = $(top_srcdir)/tools/include > -CFLAGS += -O2 -g -std=gnu99 -I$(LINUX_TOOL_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(UNAME_M) -I.. > +CFLAGS += -O2 -g -std=gnu99 -fno-stack-protector -I$(LINUX_TOOL_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(UNAME_M) -I.. > LDFLAGS += -pthread > > # After inclusion, $(OUTPUT) is defined and > Queued for after the merge window, thanks. Paolo
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile index 3c1f4bdf9000..596c0864d0df 100644 --- a/tools/testing/selftests/kvm/Makefile +++ b/tools/testing/selftests/kvm/Makefile @@ -29,7 +29,7 @@ LIBKVM += $(LIBKVM_$(UNAME_M)) INSTALL_HDR_PATH = $(top_srcdir)/usr LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/ LINUX_TOOL_INCLUDE = $(top_srcdir)/tools/include -CFLAGS += -O2 -g -std=gnu99 -I$(LINUX_TOOL_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(UNAME_M) -I.. +CFLAGS += -O2 -g -std=gnu99 -fno-stack-protector -I$(LINUX_TOOL_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude -I$(<D) -Iinclude/$(UNAME_M) -I.. LDFLAGS += -pthread # After inclusion, $(OUTPUT) is defined and
Since 4.8.3, gcc has enabled -fstack-protector by default. This is problematic for the KVM selftests as they do not configure fs or gs segments (the stack canary is pulled from fs:0x28). With the default behavior, gcc will insert a stack canary on any function that creates buffers of 8 bytes or more. As a result, ucall() will hit a triple fault shutdown due to reading a bad fs segment when inserting its stack canary, i.e. every test fails with an unexpected SHUTDOWN. Fixes: 14c47b7530e2d ("kvm: selftests: introduce ucall") Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- tools/testing/selftests/kvm/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)