diff mbox series

[v3,3/6] kexec_file: Don't opencode appended signature verification.

Message ID 378d956adfa3be2a6d95a71391b4bb2f7458ada3.1641555875.git.msuchanek@suse.de (mailing list archive)
State New, archived
Headers show
Series KEXEC_SIG with appended signature | expand

Commit Message

Michal Suchánek Jan. 7, 2022, 11:53 a.m. UTC 
Module verification already implements appeded signature verification.

Reuse it for kexec_file.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v3: - Philipp Rudo <prudo@redhat.com>: Update the dependency on
      MODULE_SIG_FORMAT to MODULE_SIG
    - Include linux/verification.h - previously added in earlier patch
---
 arch/powerpc/Kconfig                  |  2 +-
 arch/powerpc/kexec/elf_64.c           | 22 +++++-----------------
 arch/s390/Kconfig                     |  2 +-
 arch/s390/kernel/machine_kexec_file.c | 21 ++++-----------------
 include/linux/verification.h          |  3 +++
 kernel/module-internal.h              |  2 --
 kernel/module.c                       |  4 +++-
 kernel/module_signing.c               | 24 +++++++++++++++---------
 8 files changed, 32 insertions(+), 48 deletions(-)

Comments

kernel test robot Jan. 7, 2022, 6:36 p.m. UTC | #1 
Hi Michal,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on s390/features linus/master jeyu/modules-next v5.16-rc8 next-20220106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Michal-Suchanek/KEXEC_SIG-with-appended-signature/20220107-195818
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: hexagon-randconfig-r016-20220107 (https://download.01.org/0day-ci/archive/20220108/202201080202.yy2w2Wmg-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project f3a344d2125fa37e59bae1b0874442c650a19607)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c59400c94a653abe5a5fbfd5bc166bd3ac1ebb41
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Michal-Suchanek/KEXEC_SIG-with-appended-signature/20220107-195818
        git checkout c59400c94a653abe5a5fbfd5bc166bd3ac1ebb41
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> kernel/module.c:2898:40: error: incompatible pointer types passing 'unsigned long *' to parameter of type 'size_t *' (aka 'unsigned int *') [-Werror,-Wincompatible-pointer-types]
                   err = verify_appended_signature(mod, &info->len,
                                                        ^~~~~~~~~~
   include/linux/verification.h:63:57: note: passing argument to parameter 'len' here
   int verify_appended_signature(const void *data, size_t *len, struct key *trusted_keys,
                                                           ^
   kernel/module.c:4804:6: warning: no previous prototype for function 'module_layout' [-Wmissing-prototypes]
   void module_layout(struct module *mod,
        ^
   kernel/module.c:4804:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void module_layout(struct module *mod,
   ^
   static 
   1 warning and 1 error generated.


vim +2898 kernel/module.c

  2880	
  2881	#ifdef CONFIG_MODULE_SIG
  2882	static int module_sig_check(struct load_info *info, int flags)
  2883	{
  2884		int err = -ENODATA;
  2885		const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
  2886		const char *reason;
  2887		const void *mod = info->hdr;
  2888	
  2889		/*
  2890		 * Require flags == 0, as a module with version information
  2891		 * removed is no longer the module that was signed
  2892		 */
  2893		if (flags == 0 &&
  2894		    info->len > markerlen &&
  2895		    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
  2896			/* We truncate the module to discard the signature */
  2897			info->len -= markerlen;
> 2898			err = verify_appended_signature(mod, &info->len,
  2899							VERIFY_USE_SECONDARY_KEYRING, "module");
  2900			if (!err) {
  2901				info->sig_ok = true;
  2902				return 0;
  2903			}
  2904		}
  2905	
  2906		/*
  2907		 * We don't permit modules to be loaded into the trusted kernels
  2908		 * without a valid signature on them, but if we're not enforcing,
  2909		 * certain errors are non-fatal.
  2910		 */
  2911		switch (err) {
  2912		case -ENODATA:
  2913			reason = "unsigned module";
  2914			break;
  2915		case -ENOPKG:
  2916			reason = "module with unsupported crypto";
  2917			break;
  2918		case -ENOKEY:
  2919			reason = "module with unavailable key";
  2920			break;
  2921	
  2922		default:
  2923			/*
  2924			 * All other errors are fatal, including lack of memory,
  2925			 * unparseable signatures, and signature check failures --
  2926			 * even if signatures aren't required.
  2927			 */
  2928			return err;
  2929		}
  2930	
  2931		if (is_module_sig_enforced()) {
  2932			pr_notice("Loading of %s is rejected\n", reason);
  2933			return -EKEYREJECTED;
  2934		}
  2935	
  2936		return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
  2937	}
  2938	#else /* !CONFIG_MODULE_SIG */
  2939	static int module_sig_check(struct load_info *info, int flags)
  2940	{
  2941		return 0;
  2942	}
  2943	#endif /* !CONFIG_MODULE_SIG */
  2944	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Jan. 8, 2022, 2:58 p.m. UTC | #2 
Hi Michal,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on s390/features linus/master jeyu/modules-next v5.16-rc8 next-20220107]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Michal-Suchanek/KEXEC_SIG-with-appended-signature/20220107-195818
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: arc-randconfig-r043-20220107 (https://download.01.org/0day-ci/archive/20220108/202201082218.opQ7qKfJ-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c59400c94a653abe5a5fbfd5bc166bd3ac1ebb41
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Michal-Suchanek/KEXEC_SIG-with-appended-signature/20220107-195818
        git checkout c59400c94a653abe5a5fbfd5bc166bd3ac1ebb41
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   kernel/module.c: In function 'module_sig_check':
>> kernel/module.c:2898:54: error: passing argument 2 of 'verify_appended_signature' from incompatible pointer type [-Werror=incompatible-pointer-types]
    2898 |                 err = verify_appended_signature(mod, &info->len,
         |                                                      ^~~~~~~~~~
         |                                                      |
         |                                                      long unsigned int *
   In file included from kernel/module.c:60:
   include/linux/verification.h:63:57: note: expected 'size_t *' {aka 'unsigned int *'} but argument is of type 'long unsigned int *'
      63 | int verify_appended_signature(const void *data, size_t *len, struct key *trusted_keys,
         |                                                 ~~~~~~~~^~~
   cc1: some warnings being treated as errors


vim +/verify_appended_signature +2898 kernel/module.c

  2880	
  2881	#ifdef CONFIG_MODULE_SIG
  2882	static int module_sig_check(struct load_info *info, int flags)
  2883	{
  2884		int err = -ENODATA;
  2885		const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
  2886		const char *reason;
  2887		const void *mod = info->hdr;
  2888	
  2889		/*
  2890		 * Require flags == 0, as a module with version information
  2891		 * removed is no longer the module that was signed
  2892		 */
  2893		if (flags == 0 &&
  2894		    info->len > markerlen &&
  2895		    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
  2896			/* We truncate the module to discard the signature */
  2897			info->len -= markerlen;
> 2898			err = verify_appended_signature(mod, &info->len,
  2899							VERIFY_USE_SECONDARY_KEYRING, "module");
  2900			if (!err) {
  2901				info->sig_ok = true;
  2902				return 0;
  2903			}
  2904		}
  2905	
  2906		/*
  2907		 * We don't permit modules to be loaded into the trusted kernels
  2908		 * without a valid signature on them, but if we're not enforcing,
  2909		 * certain errors are non-fatal.
  2910		 */
  2911		switch (err) {
  2912		case -ENODATA:
  2913			reason = "unsigned module";
  2914			break;
  2915		case -ENOPKG:
  2916			reason = "module with unsupported crypto";
  2917			break;
  2918		case -ENOKEY:
  2919			reason = "module with unavailable key";
  2920			break;
  2921	
  2922		default:
  2923			/*
  2924			 * All other errors are fatal, including lack of memory,
  2925			 * unparseable signatures, and signature check failures --
  2926			 * even if signatures aren't required.
  2927			 */
  2928			return err;
  2929		}
  2930	
  2931		if (is_module_sig_enforced()) {
  2932			pr_notice("Loading of %s is rejected\n", reason);
  2933			return -EKEYREJECTED;
  2934		}
  2935	
  2936		return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
  2937	}
  2938	#else /* !CONFIG_MODULE_SIG */
  2939	static int module_sig_check(struct load_info *info, int flags)
  2940	{
  2941		return 0;
  2942	}
  2943	#endif /* !CONFIG_MODULE_SIG */
  2944	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1cde9b6c5987..4092187474ff 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -562,7 +562,7 @@  config ARCH_HAS_KEXEC_PURGATORY
 
 config KEXEC_SIG
 	bool "Verify kernel signature during kexec_file_load() syscall"
-	depends on KEXEC_FILE && MODULE_SIG_FORMAT
+	depends on KEXEC_FILE && MODULE_SIG
 	help
 	  This option makes kernel signature verification mandatory for
 	  the kexec_file_load() syscall.
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 98d1cb5135b4..9442666ca69d 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -23,6 +23,7 @@ 
 #include <linux/of_fdt.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/verification.h>
 #include <linux/module_signature.h>
 
 static void *elf64_load(struct kimage *image, char *kernel_buf,
@@ -153,12 +154,10 @@  static void *elf64_load(struct kimage *image, char *kernel_buf,
 }
 
 #ifdef CONFIG_KEXEC_SIG
-int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
+int elf64_verify_sig(const char *kernel, unsigned long length)
 {
+	size_t kernel_len = length;
 	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-	struct module_signature *ms;
-	unsigned long sig_len;
-	int ret;
 
 	if (marker_len > kernel_len)
 		return -EKEYREJECTED;
@@ -168,19 +167,8 @@  int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
 		return -EKEYREJECTED;
 	kernel_len -= marker_len;
 
-	ms = (void *)kernel + kernel_len - sizeof(*ms);
-	ret = mod_check_sig(ms, kernel_len, "kexec");
-	if (ret)
-		return ret;
-
-	sig_len = be32_to_cpu(ms->sig_len);
-	kernel_len -= sizeof(*ms) + sig_len;
-
-	return verify_pkcs7_signature(kernel, kernel_len,
-				      kernel + kernel_len, sig_len,
-				      VERIFY_USE_PLATFORM_KEYRING,
-				      VERIFYING_MODULE_SIGNATURE,
-				      NULL, NULL);
+	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
+					 "kexec_file");
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 2a5bb4f29cfe..cece7152ea35 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -544,7 +544,7 @@  config ARCH_HAS_KEXEC_PURGATORY
 
 config KEXEC_SIG
 	bool "Verify kernel signature during kexec_file_load() syscall"
-	depends on KEXEC_FILE && MODULE_SIG_FORMAT
+	depends on KEXEC_FILE && MODULE_SIG
 	help
 	  This option makes kernel signature verification mandatory for
 	  the kexec_file_load() syscall.
diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index c944d71316c7..75e0c17cf0eb 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -26,12 +26,10 @@  const struct kexec_file_ops * const kexec_file_loaders[] = {
 };
 
 #ifdef CONFIG_KEXEC_SIG
-int s390_verify_sig(const char *kernel, unsigned long kernel_len)
+int s390_verify_sig(const char *kernel, unsigned long length)
 {
+	size_t kernel_len = length;
 	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-	struct module_signature *ms;
-	unsigned long sig_len;
-	int ret;
 
 	/* Skip signature verification when not secure IPLed. */
 	if (!ipl_secure_flag)
@@ -45,19 +43,8 @@  int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 		return -EKEYREJECTED;
 	kernel_len -= marker_len;
 
-	ms = (void *)kernel + kernel_len - sizeof(*ms);
-	ret = mod_check_sig(ms, kernel_len, "kexec");
-	if (ret)
-		return ret;
-
-	sig_len = be32_to_cpu(ms->sig_len);
-	kernel_len -= sizeof(*ms) + sig_len;
-
-	return verify_pkcs7_signature(kernel, kernel_len,
-				      kernel + kernel_len, sig_len,
-				      VERIFY_USE_PLATFORM_KEYRING,
-				      VERIFYING_MODULE_SIGNATURE,
-				      NULL, NULL);
+	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
+					"kexec_file");
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/include/linux/verification.h b/include/linux/verification.h
index a655923335ae..c1cf0582012a 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -60,5 +60,8 @@  extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
 				   enum key_being_used_for usage);
 #endif
 
+int verify_appended_signature(const void *data, size_t *len, struct key *trusted_keys,
+			      const char *what);
+
 #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
 #endif /* _LINUX_VERIFY_PEFILE_H */
diff --git a/kernel/module-internal.h b/kernel/module-internal.h
index 33783abc377b..80461e14bf29 100644
--- a/kernel/module-internal.h
+++ b/kernel/module-internal.h
@@ -27,5 +27,3 @@  struct load_info {
 		unsigned int sym, str, mod, vers, info, pcpu;
 	} index;
 };
-
-extern int mod_verify_sig(const void *mod, struct load_info *info);
diff --git a/kernel/module.c b/kernel/module.c
index 84a9141a5e15..8481933dfa92 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -57,6 +57,7 @@ 
 #include <linux/bsearch.h>
 #include <linux/dynamic_debug.h>
 #include <linux/audit.h>
+#include <linux/verification.h>
 #include <uapi/linux/module.h>
 #include "module-internal.h"
 
@@ -2894,7 +2895,8 @@  static int module_sig_check(struct load_info *info, int flags)
 	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
 		/* We truncate the module to discard the signature */
 		info->len -= markerlen;
-		err = mod_verify_sig(mod, info);
+		err = verify_appended_signature(mod, &info->len,
+						VERIFY_USE_SECONDARY_KEYRING, "module");
 		if (!err) {
 			info->sig_ok = true;
 			return 0;
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 8723ae70ea1f..f492e410564d 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -14,13 +14,19 @@ 
 #include <crypto/public_key.h>
 #include "module-internal.h"
 
-/*
- * Verify the signature on a module.
+/**
+ * verify_appended_signature - Verify the signature on a module with the
+ * signature marker stripped.
+ * @data: The data to be verified
+ * @len: Size of @data.
+ * @trusted_keys: Keyring to use for verification
+ * @what: Informational string for log messages
  */
-int mod_verify_sig(const void *mod, struct load_info *info)
+int verify_appended_signature(const void *data, size_t *len,
+			      struct key *trusted_keys, const char *what)
 {
 	struct module_signature ms;
-	size_t sig_len, modlen = info->len;
+	size_t sig_len, modlen = *len;
 	int ret;
 
 	pr_devel("==>%s(,%zu)\n", __func__, modlen);
@@ -28,18 +34,18 @@  int mod_verify_sig(const void *mod, struct load_info *info)
 	if (modlen <= sizeof(ms))
 		return -EBADMSG;
 
-	memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
+	memcpy(&ms, data + (modlen - sizeof(ms)), sizeof(ms));
 
-	ret = mod_check_sig(&ms, modlen, "module");
+	ret = mod_check_sig(&ms, modlen, what);
 	if (ret)
 		return ret;
 
 	sig_len = be32_to_cpu(ms.sig_len);
 	modlen -= sig_len + sizeof(ms);
-	info->len = modlen;
+	*len = modlen;
 
-	return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
-				      VERIFY_USE_SECONDARY_KEYRING,
+	return verify_pkcs7_signature(data, modlen, data + modlen, sig_len,
+				      trusted_keys,
 				      VERIFYING_MODULE_SIGNATURE,
 				      NULL, NULL);
 }