Message ID | 20200424143953.22543-2-gpiccoli@canonical.com |
---|---|
State | New |
Headers | show |
Series | [Bionic,1/1] UBUNTU: SAUCE: x86/purgatory: Fix Makefile to prevent undefined symbols | expand |
On 24.04.20 16:39, Guilherme G. Piccoli wrote: > BugLink: https://bugs.launchpad.net/bugs/1869672 > > Two purgatory issues were introduced after the merge of commit [0] > (upstream commit b059f801a937) in Ubuntu kernel Ubuntu-4.15.0-65.74, > related to kexec through kexec_file_load() syscall. > > (a) First, such kexec fails due to undefined symbol in purgatory > object (specifically __stack_chk_fail). The reasoning of this issue > is the lack of the following 2 commits on Ubuntu kernel series 4.15: > > 44c6dc940b19 ("Makefile: introduce CONFIG_CC_STACKPROTECTOR_AUTO") > 050e9baa9dc9 ("Kbuild: rename CC_STACKPROTECTOR[_STRONG] config variables") > > These commits were introduced in v4.16 and v4.18 - specially relevant is > the latter (which is a "fix" for the former), since it renames the > config options CONFIG_CC_STACKPROTECTOR and CONFIG_CC_STACKPROTECTOR_STRONG, > removing the "CC_" substring. The purgatory patch [0] (originally > written for mainline v5.3 kernel) makes use of the new config options > names to guard purgatory against stack protector, so given that our > 4.15 kernels still use the old name, purgatory ends up containing the > undefined symbol, effectively breaking kexec/kdump in secureboot systems > (which requires kexec_file_load() syscall). > > This patch fixes purgatory Makefile, in order it does use the right > config options. I'd like to thank the Launchpad user "yamato" for the > report and testing that led to this specific fix. > > (b) Also, purgatory flags were changed by commit [0] as we know, > and it ends up lacking -fno-builtin flag. On top of it, the patch > series [1] that introduced commit [0] also brought the following commit: > > 4ce97317f41d ("x86/purgatory: Do not use __builtin_memcpy and __builtin_memset") > > This commit is not present in Bionic 4.15 tree (nor one of its > dependencies), but part of this commit is required in order to avoid > undefined symbol "memcpy" in purgatory. Specially, we need to use the > arch/x86/boot/compressed/string.c memcpy() implementation and mark it > on Makefile, which is also done here. > > With parts (a) and (b) fixed, we can now succeed in kexec'ing on secure > boot environment. > > [0] Ubuntu commit 9b9b35b8f982 ("x86/purgatory: Use CFLAGS_REMOVE rather than reset KBUILD_CFLAGS") > [1] lore.kernel.org/lkml/20190807221539.94583-3-ndesaulniers@google.com/T/#u > > Fixes: Ubuntu commit [0] above > Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com> The changes look good to me, and thanks for the detailed explanation on the patch! Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > arch/x86/purgatory/Makefile | 7 +++++-- > arch/x86/purgatory/purgatory.c | 6 ++++++ > arch/x86/purgatory/string.c | 13 ------------- > 3 files changed, 11 insertions(+), 15 deletions(-) > delete mode 100644 arch/x86/purgatory/string.c > > diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile > index 931ab3eece2c..37ab2a5aa838 100644 > --- a/arch/x86/purgatory/Makefile > +++ b/arch/x86/purgatory/Makefile > @@ -6,6 +6,9 @@ purgatory-y := purgatory.o stack.o setup-x86_$(BITS).o sha256.o entry64.o string > targets += $(purgatory-y) > PURGATORY_OBJS = $(addprefix $(obj)/,$(purgatory-y)) > > +$(obj)/string.o: $(srctree)/arch/x86/boot/compressed/string.c FORCE > + $(call if_changed_rule,cc_o_c) > + > LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined -nostdlib -z nodefaultlib > targets += purgatory.ro > > @@ -26,11 +29,11 @@ ifdef CONFIG_FUNCTION_TRACER > PURGATORY_CFLAGS_REMOVE += $(CC_FLAGS_FTRACE) > endif > > -ifdef CONFIG_STACKPROTECTOR > +ifdef CONFIG_CC_STACKPROTECTOR > PURGATORY_CFLAGS_REMOVE += -fstack-protector > endif > > -ifdef CONFIG_STACKPROTECTOR_STRONG > +ifdef CONFIG_CC_STACKPROTECTOR_STRONG > PURGATORY_CFLAGS_REMOVE += -fstack-protector-strong > endif > > diff --git a/arch/x86/purgatory/purgatory.c b/arch/x86/purgatory/purgatory.c > index 470edad96bb9..4f3c15d0bb86 100644 > --- a/arch/x86/purgatory/purgatory.c > +++ b/arch/x86/purgatory/purgatory.c > @@ -70,3 +70,9 @@ void purgatory(void) > } > copy_backup_region(); > } > + > +/* > + * Defined in order to reuse memcpy() and memset() from > + * arch/x86/boot/compressed/string.c > + */ > +void warn(const char *msg) {} > diff --git a/arch/x86/purgatory/string.c b/arch/x86/purgatory/string.c > deleted file mode 100644 > index d886b1fa36f0..000000000000 > --- a/arch/x86/purgatory/string.c > +++ /dev/null > @@ -1,13 +0,0 @@ > -/* > - * Simple string functions. > - * > - * Copyright (C) 2014 Red Hat Inc. > - * > - * Author: > - * Vivek Goyal <vgoyal@redhat.com> > - * > - * This source code is licensed under the GNU General Public License, > - * Version 2. See the file COPYING for more details. > - */ > - > -#include "../boot/string.c" >
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile index 931ab3eece2c..37ab2a5aa838 100644 --- a/arch/x86/purgatory/Makefile +++ b/arch/x86/purgatory/Makefile @@ -6,6 +6,9 @@ purgatory-y := purgatory.o stack.o setup-x86_$(BITS).o sha256.o entry64.o string targets += $(purgatory-y) PURGATORY_OBJS = $(addprefix $(obj)/,$(purgatory-y)) +$(obj)/string.o: $(srctree)/arch/x86/boot/compressed/string.c FORCE + $(call if_changed_rule,cc_o_c) + LDFLAGS_purgatory.ro := -e purgatory_start -r --no-undefined -nostdlib -z nodefaultlib targets += purgatory.ro @@ -26,11 +29,11 @@ ifdef CONFIG_FUNCTION_TRACER PURGATORY_CFLAGS_REMOVE += $(CC_FLAGS_FTRACE) endif -ifdef CONFIG_STACKPROTECTOR +ifdef CONFIG_CC_STACKPROTECTOR PURGATORY_CFLAGS_REMOVE += -fstack-protector endif -ifdef CONFIG_STACKPROTECTOR_STRONG +ifdef CONFIG_CC_STACKPROTECTOR_STRONG PURGATORY_CFLAGS_REMOVE += -fstack-protector-strong endif diff --git a/arch/x86/purgatory/purgatory.c b/arch/x86/purgatory/purgatory.c index 470edad96bb9..4f3c15d0bb86 100644 --- a/arch/x86/purgatory/purgatory.c +++ b/arch/x86/purgatory/purgatory.c @@ -70,3 +70,9 @@ void purgatory(void) } copy_backup_region(); } + +/* + * Defined in order to reuse memcpy() and memset() from + * arch/x86/boot/compressed/string.c + */ +void warn(const char *msg) {} diff --git a/arch/x86/purgatory/string.c b/arch/x86/purgatory/string.c deleted file mode 100644 index d886b1fa36f0..000000000000 --- a/arch/x86/purgatory/string.c +++ /dev/null @@ -1,13 +0,0 @@ -/* - * Simple string functions. - * - * Copyright (C) 2014 Red Hat Inc. - * - * Author: - * Vivek Goyal <vgoyal@redhat.com> - * - * This source code is licensed under the GNU General Public License, - * Version 2. See the file COPYING for more details. - */ - -#include "../boot/string.c"
BugLink: https://bugs.launchpad.net/bugs/1869672 Two purgatory issues were introduced after the merge of commit [0] (upstream commit b059f801a937) in Ubuntu kernel Ubuntu-4.15.0-65.74, related to kexec through kexec_file_load() syscall. (a) First, such kexec fails due to undefined symbol in purgatory object (specifically __stack_chk_fail). The reasoning of this issue is the lack of the following 2 commits on Ubuntu kernel series 4.15: 44c6dc940b19 ("Makefile: introduce CONFIG_CC_STACKPROTECTOR_AUTO") 050e9baa9dc9 ("Kbuild: rename CC_STACKPROTECTOR[_STRONG] config variables") These commits were introduced in v4.16 and v4.18 - specially relevant is the latter (which is a "fix" for the former), since it renames the config options CONFIG_CC_STACKPROTECTOR and CONFIG_CC_STACKPROTECTOR_STRONG, removing the "CC_" substring. The purgatory patch [0] (originally written for mainline v5.3 kernel) makes use of the new config options names to guard purgatory against stack protector, so given that our 4.15 kernels still use the old name, purgatory ends up containing the undefined symbol, effectively breaking kexec/kdump in secureboot systems (which requires kexec_file_load() syscall). This patch fixes purgatory Makefile, in order it does use the right config options. I'd like to thank the Launchpad user "yamato" for the report and testing that led to this specific fix. (b) Also, purgatory flags were changed by commit [0] as we know, and it ends up lacking -fno-builtin flag. On top of it, the patch series [1] that introduced commit [0] also brought the following commit: 4ce97317f41d ("x86/purgatory: Do not use __builtin_memcpy and __builtin_memset") This commit is not present in Bionic 4.15 tree (nor one of its dependencies), but part of this commit is required in order to avoid undefined symbol "memcpy" in purgatory. Specially, we need to use the arch/x86/boot/compressed/string.c memcpy() implementation and mark it on Makefile, which is also done here. With parts (a) and (b) fixed, we can now succeed in kexec'ing on secure boot environment. [0] Ubuntu commit 9b9b35b8f982 ("x86/purgatory: Use CFLAGS_REMOVE rather than reset KBUILD_CFLAGS") [1] lore.kernel.org/lkml/20190807221539.94583-3-ndesaulniers@google.com/T/#u Fixes: Ubuntu commit [0] above Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com> --- arch/x86/purgatory/Makefile | 7 +++++-- arch/x86/purgatory/purgatory.c | 6 ++++++ arch/x86/purgatory/string.c | 13 ------------- 3 files changed, 11 insertions(+), 15 deletions(-) delete mode 100644 arch/x86/purgatory/string.c