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

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

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