ACK: [Bionic] [PATCH 1/1] UBUNTU: SAUCE: x86/purgatory: Fix Makefile to prevent undefined symbols

[Kleber Souza]

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 at google.com/T/#u
> 
> Fixes: Ubuntu commit [0] above
> Signed-off-by: Guilherme G. Piccoli <gpiccoli at 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 at 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 at 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"
>