[PATCH v4] kdump-lib: switch to the kexec_file_load() syscall on x86_64 by default

[Lianbo Jiang]

UEFI Secure boot is a signature verification mechanism, 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 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&gt;
---
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

[Bhupesh Sharma]

Hi Lianbo,

Thanks for the v4. It looks better.
Some nitpicks in-line:

On Thu, Dec 26, 2019 at 3:31 PM Lianbo Jiang <lijiang(a)redhat.com&gt; 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&gt;
>  ---
>  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&gt;

[Dave Young]

Hi,

On 12/26/19 at 06:00pm, Lianbo Jiang wrote:
>  UEFI Secure boot is a signature verification mechanism, 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 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&gt;
>  ---
>  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 
This version looks much better, thanks for the effort!

But if people want to use old userspace loader, then we should have a
way to let user to choose.  Currently it is hardcoded in code to use
kexec_file_load, probably add some new config field in
/etc/sysconfig/kdump so that people can change it. 

Agreed that an auto fallback maybe not good if we have a bug in kexec_file_load,
but sometimes kexec_file_load and kexec_load support different scopes
like different kernel image formats etc. In these cases a fallback
should be good.  The upstream kexec-tools added an option named "-a"
which will fallback for these cases.

Anyway, if we can fail in case some bugs and fallback for those
unsupported issues then it would be good.

The bottom line should be people can choose if they have to.

Opinions?

Thanks
Dave

[lijiang]

在 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&gt; 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&gt;
> > ---
> > 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&gt;
>

[lijiang]

在 2019年12月30日 15:13, Dave Young 写道:
>  Hi,
>  
>  On 12/26/19 at 06:00pm, Lianbo Jiang wrote:
> > UEFI Secure boot is a signature verification mechanism, 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 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&gt;
> > ---
> > 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
>  
>  This version looks much better, thanks for the effort!
>  
>  But if people want to use old userspace loader, then we should have a
>  way to let user to choose.  Currently it is hardcoded in code to use
>  kexec_file_load, probably add some new config field in
>  /etc/sysconfig/kdump so that people can change it. 
>  
>  Agreed that an auto fallback maybe not good if we have a bug in kexec_file_load,
>  but sometimes kexec_file_load and kexec_load support different scopes
>  like different kernel image formats etc. In these cases a fallback
>  should be good.  The upstream kexec-tools added an option named "-a"
>  which will fallback for these cases.
>  
>  Anyway, if we can fail in case some bugs and fallback for those
>  unsupported issues then it would be good.
>  
>  The bottom line should be people can choose if they have to.
>  
>  Opinions?
>   
Thanks for your suggestion.

As we discussed in irc, I will consider how to provide a chance for user to switch
it between the kexec-load and kexec-file-load. And I tend to use an option in
/etc/sysconfig/kdump to change it.

Anyway, after improve it, I will post again, and we can still discuss it.

Thanks.

>  Thanks
>  Dave
>