在 2019年12月30日 14:55, Bhupesh Sharma 写道:
Hi Lianbo,
Thanks for the v4. It looks better.
Some nitpicks in-line:
Thank you for the review, Bhupesh.
I will improve the log and post again.
On Thu, Dec 26, 2019 at 3:31 PM Lianbo Jiang
<lijiang(a)redhat.com> wrote:
>
> UEFI Secure boot is a signature verification mechanism, is
^^
You can remove the extra 'is'
> designed to
> prevent malicious code being loaded and executed at the early boot stage.
> This makes sure that code executed is trusted by firmware.
>
> Previously, with kexec_file_load() interface, kernel prevents unsigned
> kernel image from being loaded if secure boot is enabled. So kdump will
> detect whether secure boot is enabled firstly, then decide which interface
> is chosen to execute, kexec_load() or kexec_file_load(). Otherwise unsigned
> kernel loading will fail if secure boot enabled, and kexec_file_load() is
> entered.
>
> Now, the implementation of kexec_file_load() is adjusted in below commit.
> With this change, if CONFIG_KEXEC_SIG_FORCE is not set, unsigned kernel
> still has a chance to be allowed to load under some conditions.
>
> commit 99d5cadfde2b ("kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG
> and KEXEC_SIG_FORCE")
>
> And in the current Fedora, the CONFIG_KEXEC_SIG_FORCE is not set, only the
> CONFIG_KEXEC_SIG and CONFIG_BZIMAGE_VERIFY_SIG are set on x86_64 by default.
> It's time to spread kexec_file_load() onto all systems of x86_64, including
> Secure-boot platforms and legacy platforms. Please refer to the following
> form.
>
> .----------------------------------------------------------------------.
> | . | signed kernel | unsigned kernel |
> | . types |-----------------------|-----------------------|
> | . |Secure boot| Legacy |Secure boot| Legacy |
> | . |-----------|-----------|-----------|-----------|
> | options . | prev| now | prev| now | | | prev| now |
> | . |(file|(file|(only|(file| prev| now |(only|(file|
> | . |load)|load)|load)|load)| | |load)|load)|
> |----------------------|-----|-----|-----|-----|-----|-----|-----|-----|
> |KEXEC_SIG=y | | | | | | | | |
> |SIG_FORCE is not set |succ |succ |succ |succ | X | X |succ |succ |
> |BZIMAGE_VERIFY_SIG=y | | | | | | | | |
> |----------------------|-----|-----|-----|-----|-----|-----|-----|-----|
> |KEXEC_SIG=y | | | | | | | | |
> |SIG_FORCE is not set | | | | | | | | |
> |BZIMAGE_VERIFY_SIG is |fail |fail |succ |fail | X | X |succ |fail |
> |not set | | | | | | | | |
> |----------------------|-----|-----|-----|-----|-----|-----|-----|-----|
> |KEXEC_SIG=y | | | | | | | | |
> |SIG_FORCE=y |succ |succ |succ |fail | X | X |succ |fail |
> |BZIMAGE_VERIFY_SIG=y | | | | | | | | |
> |----------------------|-----|-----|-----|-----|-----|-----|-----|-----|
> |KEXEC_SIG=y | | | | | | | | |
> |SIG_FORCE=y | | | | | | | | |
> |BZIMAGE_VERIFY_SIG is |fail |fail |succ |fail | X | X |succ |fail |
> |not set | | | | | | | | |
> |----------------------|-----|-----|-----|-----|-----|-----|-----|-----|
> |KEXEC_SIG is not set | | | | | | | | |
> |SIG_FORCE is not set | | | | | | | | |
> |BZIMAGE_VERIFY_SIG is |fail |fail |succ |succ | X | X |succ |succ |
> |not set | | | | | | | | |
> ----------------------------------------------------------------------
> Note:
> [1] The 'X' indicates that the 1st kernel(unsigned) can not boot when the
> Secure boot is enabled.
>
> Hence, in this patch, if on x86_64, lets
s/lets/let's
> use the kexec_file_load() only. See if
> anything wrong happened in this case, in Fedora firstly for the time being.
>
> Signed-off-by: Lianbo Jiang <lijiang(a)redhat.com>
> ---
> Changes since v1:
> [1] Improve patch log.
> [2] Change the is_secure_boot_enforced() to use_kexec_file_load().
> [3] Aamend the code comment.
> [4] Add the form for the kbuild options.
>
> Changes since v2:
> [1] Improve patch log.
> [2] Also rewrite the form for the kbuild options.
>
> Changes since v3:
> [1] Improve patch log.
> [2] Display an error message and ask user to try kexec_load() once the
> kexec_file_load() failed.
>
> dracut-early-kdump.sh | 4 ++--
> kdump-lib.sh | 31 +++++++++++--------------------
> kdumpctl | 13 +++++++------
> 3 files changed, 20 insertions(+), 28 deletions(-)
>
> diff --git a/dracut-early-kdump.sh b/dracut-early-kdump.sh
> index 69a34eb996cd..bfb923d6efa1 100755
> --- a/dracut-early-kdump.sh
> +++ b/dracut-early-kdump.sh
> @@ -43,8 +43,8 @@ early_kdump_load()
>
> EARLY_KEXEC_ARGS=$(prepare_kexec_args "${KEXEC_ARGS}")
>
> - if is_secure_boot_enforced; then
> - echo "Secure Boot is enabled. Using kexec file based syscall."
> + if use_kexec_file_load; then
> + echo "Using kexec file based syscall."
> EARLY_KEXEC_ARGS="$EARLY_KEXEC_ARGS -s"
> fi
>
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index f393c76b9cbb..5b6684992553 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -589,30 +589,21 @@ need_64bit_headers()
> print (strtonum("0x" r[2]) > strtonum("0xffffffff"));
}'`
> }
>
> -# Check if secure boot is being enforced.
> #
> -# Per Peter Jones, we need check efivar SecureBoot-$(the UUID) and
> -# SetupMode-$(the UUID), they are both 5 bytes binary data. The first four
> -# bytes are the attributes associated with the variable and can safely be
> -# ignored, the last bytes are one-byte true-or-false variables. If SecureBoot
> -# is 1 and SetupMode is 0, then secure boot is being enforced.
> +# Currently, Secure Boot is only used on x86_64 and kernel enabled the
> +# kexec_file_load() on x86_64 by default.
> +# In addition, kernel also enables the option KEXEC_SIG, which makes the
> +# kexec_file_load() syscall checks for a valid signature of the kernel
> +# image if there is a signature(It must be valid). And the images can
> +# still be loaded without a valid signature. So let kexec-tools change
> +# as well.
> #
> -# Assume efivars is mounted at /sys/firmware/efi/efivars.
> -is_secure_boot_enforced()
> +use_kexec_file_load()
> {
> - local secure_boot_file setup_mode_file
> - local secure_boot_byte setup_mode_byte
> + local arch_name=`uname -m`
>
> - secure_boot_file=$(find /sys/firmware/efi/efivars -name SecureBoot-*
2>/dev/null)
> - setup_mode_file=$(find /sys/firmware/efi/efivars -name SetupMode-*
2>/dev/null)
> -
> - if [ -f "$secure_boot_file" ] && [ -f
"$setup_mode_file" ]; then
> - secure_boot_byte=$(hexdump -v -e '/1 "%d\ "'
$secure_boot_file|cut -d' ' -f 5)
> - setup_mode_byte=$(hexdump -v -e '/1 "%d\ "'
$setup_mode_file|cut -d' ' -f 5)
> -
> - if [ "$secure_boot_byte" = "1" ] && [
"$setup_mode_byte" = "0" ]; then
> - return 0
> - fi
> + if [ "$arch_name" == "x86_64" ]; then
> + return 0
> fi
>
> return 1
> diff --git a/kdumpctl b/kdumpctl
> index 2d21a416deb1..f119fd9305ff 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -678,11 +678,9 @@ load_kdump()
> KEXEC_ARGS=$(prepare_kexec_args "${KEXEC_ARGS}")
> KDUMP_COMMANDLINE=$(prepare_cmdline "${KDUMP_COMMANDLINE}"
"${KDUMP_COMMANDLINE_REMOVE}" "${KDUMP_COMMANDLINE_APPEND}")
>
> - # For secureboot enabled machines, use new kexec file based syscall.
> - # Old syscall will always fail as it does not have capability to
> - # to kernel signature verification.
> - if is_secure_boot_enforced; then
> - echo "Secure Boot is enabled. Using kexec file based
syscall."
> + # On x86_64 machine, lets use kexec file based syscall by default.
> + if use_kexec_file_load; then
> + echo "Using kexec file based syscall."
> KEXEC_ARGS="$KEXEC_ARGS -s"
> fi
>
> @@ -694,6 +692,9 @@ load_kdump()
> return 0
> else
> echo "kexec: failed to load kdump kernel" >&2
> + if [ -n "$KEXEC_ARGS" ]; then
> + echo "kexec_file_load() failed, please try
kexec_load()" >&2
> + fi
> return 1
> fi
> }
> @@ -1162,7 +1163,7 @@ stop_fadump()
>
> stop_kdump()
> {
> - if is_secure_boot_enforced; then
> + if use_kexec_file_load; then
> $KEXEC -s -p -u
> else
> $KEXEC -p -u
> --
> 2.17.1
The rest of the chnages LGTM, so with the above nitpicks addressed:
Acked-by: Bhupesh Sharma <bhsharma(a)redhat.com>