Re: [PATCH 2/2] [FIXED] Replace magic for trusting the secondary keyring with #define

From: Yannik Sembritzki <yannik@sembritzki.me>
To: kbuild test robot <lkp@intel.com>
Cc: kbuild-all@01.org, Linus Torvalds <torvalds@linux-foundation.org>,
	David Howells <dhowells@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Peter Anvin <hpa@zytor.com>,
	the arch/x86 maintainers <x86@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Dave Young <dyoung@redhat.com>, Baoquan He <bhe@redhat.com>,
	"Justin M. Forbes" <jforbes@redhat.com>,
	Peter Jones <pjones@redhat.com>,
	James Bottomley <James.Bottomley@HansenPartnership.com>,
	Matthew Garrett <mjg59@google.com>,
	Vivek Goyal <vgoyal@redhat.com>
Subject: Re: [PATCH 2/2] [FIXED] Replace magic for trusting the secondary keyring with #define
Date: Wed, 15 Aug 2018 23:19:51 +0200	[thread overview]
Message-ID: <1bfa03ed-c9f8-d0f2-700c-c93e96d5b99c@sembritzki.me> (raw)
In-Reply-To: <201808160443.5h16PxVs%fengguang.wu@intel.com>

Signed-off-by: Yannik Sembritzki <yannik@sembritzki.me>
---
 arch/x86/kernel/kexec-bzimage64.c       | 2 +-
 certs/system_keyring.c                  | 3 ++-
 crypto/asymmetric_keys/pkcs7_key_type.c | 2 +-
 include/linux/verification.h            | 5 +++++
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/kexec-bzimage64.c
b/arch/x86/kernel/kexec-bzimage64.c
index 74628275..97d199a3 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -532,7 +532,7 @@ static int bzImage64_cleanup(void *loader_data)
 static int bzImage64_verify_sig(const char *kernel, unsigned long
kernel_len)
 {
     return verify_pefile_signature(kernel, kernel_len,
-                       ((struct key *)1UL),
+                       TRUST_SECONDARY_KEYRING,
                        VERIFYING_KEXEC_PE_SIGNATURE);
 }
 #endif
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 6251d1b2..777ac7d2 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -15,6 +15,7 @@
 #include <linux/cred.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/verification.h>
 #include <keys/asymmetric-type.h>
 #include <keys/system_keyring.h>
 #include <crypto/pkcs7.h>
@@ -230,7 +231,7 @@ int verify_pkcs7_signature(const void *data, size_t len,
 
     if (!trusted_keys) {
         trusted_keys = builtin_trusted_keys;
-    } else if (trusted_keys == (void *)1UL) {
+    } else if (trusted_keys == TRUST_SECONDARY_KEYRING) {
 #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
         trusted_keys = secondary_trusted_keys;
 #else
diff --git a/crypto/asymmetric_keys/pkcs7_key_type.c
b/crypto/asymmetric_keys/pkcs7_key_type.c
index e284d9cb..0783e555 100644
--- a/crypto/asymmetric_keys/pkcs7_key_type.c
+++ b/crypto/asymmetric_keys/pkcs7_key_type.c
@@ -63,7 +63,7 @@ static int pkcs7_preparse(struct key_preparsed_payload
*prep)
 
     return verify_pkcs7_signature(NULL, 0,
                       prep->data, prep->datalen,
-                      (void *)1UL, usage,
+                      TRUST_SECONDARY_KEYRING, usage,
                       pkcs7_view_content, prep);
 }
 
diff --git a/include/linux/verification.h b/include/linux/verification.h
index a10549a6..ad221ab2 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -12,6 +12,11 @@
 #ifndef _LINUX_VERIFICATION_H
 #define _LINUX_VERIFICATION_H
 
+// Allow both builtin trusted keys and secondary trusted keys
+#ifndef TRUST_SECONDARY_KEYRING
+#define TRUST_SECONDARY_KEYRING ((struct key *)1UL)
+#endif
+
 /*
  * The use to which an asymmetric key is being put.
  */
-- 
2.17.1

Sorry, I've fixed this now.

On 15.08.2018 23:14, kbuild test robot wrote:
> Hi Yannik,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.18 next-20180814]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Yannik-Sembritzki/Fix-kexec-forbidding-kernels-signed-with-keys-in-the-secondary-keyring-to-boot/20180816-042529
> config: x86_64-randconfig-x019-201832 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
>
> All errors (new ones prefixed by >>):
>
>    In file included from include/keys/asymmetric-type.h:18:0,
>                     from crypto/asymmetric_keys/asymmetric_keys.h:12,
>                     from crypto/asymmetric_keys/restrict.c:17:
>>> include/linux/verification.h:12:0: error: unterminated #ifndef
>     #ifndef _LINUX_VERIFICATION_H
>     
> --
>    In file included from include/crypto/pkcs7.h:15:0,
>                     from crypto/asymmetric_keys/pkcs7_parser.h:13,
>                     from crypto/asymmetric_keys/pkcs7_parser.c:20:
>>> include/linux/verification.h:12:0: error: unterminated #ifndef
>     #ifndef _LINUX_VERIFICATION_H
>     
>    In file included from include/keys/asymmetric-type.h:18:0,
>                     from crypto/asymmetric_keys/x509_parser.h:14,
>                     from crypto/asymmetric_keys/pkcs7_parser.h:14,
>                     from crypto/asymmetric_keys/pkcs7_parser.c:20:
>>> include/linux/verification.h:12:0: error: unterminated #ifndef
>     #ifndef _LINUX_VERIFICATION_H
>     
>
> vim +12 include/linux/verification.h
>
> e68503bd David Howells 2016-04-06 @12  #ifndef _LINUX_VERIFICATION_H
> e68503bd David Howells 2016-04-06  13  #define _LINUX_VERIFICATION_H
> e68503bd David Howells 2016-04-06  14  
>
> :::::: The code at line 12 was first introduced by commit
> :::::: e68503bd6836ba765dc8e0ee77ea675fedc07e41 KEYS: Generalise system_verify_data() to provide access to internal content
>
> :::::: TO: David Howells <dhowells@redhat.com>
> :::::: CC: David Howells <dhowells@redhat.com>
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation


