linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] efi: cosmetic patches for the error messages when loading certificates
@ 2019-12-12  9:38 Lee, Chun-Yi
  2019-12-12  9:38 ` [PATCH 1/2] efi: add a function for transferring status to string Lee, Chun-Yi
  2019-12-12  9:38 ` [PATCH 2/2] efi: show error messages only when loading certificates is failed Lee, Chun-Yi
  0 siblings, 2 replies; 12+ messages in thread
From: Lee, Chun-Yi @ 2019-12-12  9:38 UTC (permalink / raw)
  To: Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar
  Cc: linux-efi, linux-security-module, linux-kernel, Lee, Chun-Yi

When loading certificates list from EFI variables, the error
message and efi status code always be emitted to dmesg. It looks
ugly:

[    2.335031] Couldn't get size: 0x800000000000000e
[    2.335032] Couldn't get UEFI MokListRT 
[    2.339985] Couldn't get size: 0x800000000000000e
[    2.339987] Couldn't get UEFI dbx list

This cosmetic patch set moved the above messages to the error
handling code path. And, it adds a function to transfer EFI status
code to string to improve the readability of debug log. The function
can also be used in other EFI log.

Lee, Chun-Yi (2):
  efi: add a function for transferring status to string
  efi: show error messages only when loading certificates is failed

 include/linux/efi.h                           | 26 +++++++++++++++++
 security/integrity/platform_certs/load_uefi.c | 41 ++++++++++++++-------------
 2 files changed, 48 insertions(+), 19 deletions(-)

-- 
2.16.4


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] efi: add a function for transferring status to string
  2019-12-12  9:38 [PATCH 0/2] efi: cosmetic patches for the error messages when loading certificates Lee, Chun-Yi
@ 2019-12-12  9:38 ` Lee, Chun-Yi
  2019-12-12 11:20   ` Ard Biesheuvel
  2019-12-12  9:38 ` [PATCH 2/2] efi: show error messages only when loading certificates is failed Lee, Chun-Yi
  1 sibling, 1 reply; 12+ messages in thread
From: Lee, Chun-Yi @ 2019-12-12  9:38 UTC (permalink / raw)
  To: Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar
  Cc: linux-efi, linux-security-module, linux-kernel, Lee, Chun-Yi

This function can be used to transfer EFI status code to string
to improve the readability of debug log.

Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 include/linux/efi.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index d87acf62958e..08daf4cdd807 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -42,6 +42,32 @@
 #define EFI_ABORTED		(21 | (1UL << (BITS_PER_LONG-1)))
 #define EFI_SECURITY_VIOLATION	(26 | (1UL << (BITS_PER_LONG-1)))
 
+#define EFI_STATUS_STR(_status) \
+	case EFI_##_status: \
+		return "EFI_" __stringify(_status);
+
+static inline char *
+efi_status_to_str(unsigned long status)
+{
+	switch (status) {
+	EFI_STATUS_STR(SUCCESS)
+	EFI_STATUS_STR(LOAD_ERROR)
+	EFI_STATUS_STR(INVALID_PARAMETER)
+	EFI_STATUS_STR(UNSUPPORTED)
+	EFI_STATUS_STR(BAD_BUFFER_SIZE)
+	EFI_STATUS_STR(BUFFER_TOO_SMALL)
+	EFI_STATUS_STR(NOT_READY)
+	EFI_STATUS_STR(DEVICE_ERROR)
+	EFI_STATUS_STR(WRITE_PROTECTED)
+	EFI_STATUS_STR(OUT_OF_RESOURCES)
+	EFI_STATUS_STR(NOT_FOUND)
+	EFI_STATUS_STR(ABORTED)
+	EFI_STATUS_STR(SECURITY_VIOLATION)
+	}
+
+	return "";
+}
+
 typedef unsigned long efi_status_t;
 typedef u8 efi_bool_t;
 typedef u16 efi_char16_t;		/* UNICODE character */
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] efi: show error messages only when loading certificates is failed
  2019-12-12  9:38 [PATCH 0/2] efi: cosmetic patches for the error messages when loading certificates Lee, Chun-Yi
  2019-12-12  9:38 ` [PATCH 1/2] efi: add a function for transferring status to string Lee, Chun-Yi
@ 2019-12-12  9:38 ` Lee, Chun-Yi
  1 sibling, 0 replies; 12+ messages in thread
From: Lee, Chun-Yi @ 2019-12-12  9:38 UTC (permalink / raw)
  To: Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar
  Cc: linux-efi, linux-security-module, linux-kernel, Lee, Chun-Yi

When loading certificates list from EFI variables, the error
message and efi status code always be emitted to dmesg. It looks
ugly:

[    2.335031] Couldn't get size: 0x800000000000000e
[    2.335032] Couldn't get UEFI MokListRT
[    2.339985] Couldn't get size: 0x800000000000000e
[    2.339987] Couldn't get UEFI dbx list

This cosmetic patch moved the messages to the error handling code
path. And, it also shows the corresponding status string of status
code.

Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 security/integrity/platform_certs/load_uefi.c | 41 ++++++++++++++-------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index 81b19c52832b..3b766831d2c5 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/kernel.h>
 #include <linux/sched.h>
@@ -39,7 +40,7 @@ static __init bool uefi_check_ignore_db(void)
  * Get a certificate list blob from the named EFI variable.
  */
 static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
-				  unsigned long *size)
+				  unsigned long *size, const char *source)
 {
 	efi_status_t status;
 	unsigned long lsize = 4;
@@ -48,23 +49,31 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid,
 
 	status = efi.get_variable(name, guid, NULL, &lsize, &tmpdb);
 	if (status != EFI_BUFFER_TOO_SMALL) {
-		pr_err("Couldn't get size: 0x%lx\n", status);
-		return NULL;
+		if (status == EFI_NOT_FOUND) {
+			pr_debug("%s list was not found\n", source);
+			return NULL;
+		}
+		goto err;
 	}
 
 	db = kmalloc(lsize, GFP_KERNEL);
-	if (!db)
-		return NULL;
+	if (!db) {
+		status = EFI_OUT_OF_RESOURCES;
+		goto err;
+	}
 
 	status = efi.get_variable(name, guid, NULL, &lsize, db);
 	if (status != EFI_SUCCESS) {
 		kfree(db);
-		pr_err("Error reading db var: 0x%lx\n", status);
-		return NULL;
+		goto err;
 	}
 
 	*size = lsize;
 	return db;
+err:
+	pr_err("Couldn't get %s list: %s (0x%lx)\n",
+		source, efi_status_to_str(status), status);
+	return NULL;
 }
 
 /*
@@ -153,10 +162,8 @@ static int __init load_uefi_certs(void)
 	 * an error if we can't get them.
 	 */
 	if (!uefi_check_ignore_db()) {
-		db = get_cert_list(L"db", &secure_var, &dbsize);
-		if (!db) {
-			pr_err("MODSIGN: Couldn't get UEFI db list\n");
-		} else {
+		db = get_cert_list(L"db", &secure_var, &dbsize, "UEFI:db");
+		if (db) {
 			rc = parse_efi_signature_list("UEFI:db",
 					db, dbsize, get_handler_for_db);
 			if (rc)
@@ -166,10 +173,8 @@ static int __init load_uefi_certs(void)
 		}
 	}
 
-	mok = get_cert_list(L"MokListRT", &mok_var, &moksize);
-	if (!mok) {
-		pr_info("Couldn't get UEFI MokListRT\n");
-	} else {
+	mok = get_cert_list(L"MokListRT", &mok_var, &moksize, "UEFI:MokListRT");
+	if (mok) {
 		rc = parse_efi_signature_list("UEFI:MokListRT",
 					      mok, moksize, get_handler_for_db);
 		if (rc)
@@ -177,10 +182,8 @@ static int __init load_uefi_certs(void)
 		kfree(mok);
 	}
 
-	dbx = get_cert_list(L"dbx", &secure_var, &dbxsize);
-	if (!dbx) {
-		pr_info("Couldn't get UEFI dbx list\n");
-	} else {
+	dbx = get_cert_list(L"dbx", &secure_var, &dbxsize, "UEFI:dbx");
+	if (dbx) {
 		rc = parse_efi_signature_list("UEFI:dbx",
 					      dbx, dbxsize,
 					      get_handler_for_dbx);
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] efi: add a function for transferring status to string
  2019-12-12  9:38 ` [PATCH 1/2] efi: add a function for transferring status to string Lee, Chun-Yi
@ 2019-12-12 11:20   ` Ard Biesheuvel
  2019-12-12 14:24     ` Joey Lee
  0 siblings, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2019-12-12 11:20 UTC (permalink / raw)
  To: Lee, Chun-Yi
  Cc: James Morris, Serge E . Hallyn, David Howells, Josh Boyer,
	Nayna Jain, Mimi Zohar, linux-efi, linux-security-module,
	Linux Kernel Mailing List, Lee, Chun-Yi

On Thu, 12 Dec 2019 at 10:38, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
>
> This function can be used to transfer EFI status code to string
> to improve the readability of debug log.
>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>

I think I mentioned this the last time you sent this patch: by making
this a static inline, those strings will be copied into each object
file that uses this routine.
Instead, please make it an ordinary function.

> ---
>  include/linux/efi.h | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index d87acf62958e..08daf4cdd807 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -42,6 +42,32 @@
>  #define EFI_ABORTED            (21 | (1UL << (BITS_PER_LONG-1)))
>  #define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG-1)))
>
> +#define EFI_STATUS_STR(_status) \
> +       case EFI_##_status: \
> +               return "EFI_" __stringify(_status);
> +
> +static inline char *
> +efi_status_to_str(unsigned long status)
> +{
> +       switch (status) {
> +       EFI_STATUS_STR(SUCCESS)
> +       EFI_STATUS_STR(LOAD_ERROR)
> +       EFI_STATUS_STR(INVALID_PARAMETER)
> +       EFI_STATUS_STR(UNSUPPORTED)
> +       EFI_STATUS_STR(BAD_BUFFER_SIZE)
> +       EFI_STATUS_STR(BUFFER_TOO_SMALL)
> +       EFI_STATUS_STR(NOT_READY)
> +       EFI_STATUS_STR(DEVICE_ERROR)
> +       EFI_STATUS_STR(WRITE_PROTECTED)
> +       EFI_STATUS_STR(OUT_OF_RESOURCES)
> +       EFI_STATUS_STR(NOT_FOUND)
> +       EFI_STATUS_STR(ABORTED)
> +       EFI_STATUS_STR(SECURITY_VIOLATION)
> +       }
> +
> +       return "";
> +}
> +
>  typedef unsigned long efi_status_t;
>  typedef u8 efi_bool_t;
>  typedef u16 efi_char16_t;              /* UNICODE character */
> --
> 2.16.4
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] efi: add a function for transferring status to string
  2019-12-12 11:20   ` Ard Biesheuvel
@ 2019-12-12 14:24     ` Joey Lee
  0 siblings, 0 replies; 12+ messages in thread
From: Joey Lee @ 2019-12-12 14:24 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Lee, Chun-Yi, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar, linux-efi,
	linux-security-module, Linux Kernel Mailing List

Hi Ard, 

On Thu, Dec 12, 2019 at 11:20:48AM +0000, Ard Biesheuvel wrote:
> On Thu, 12 Dec 2019 at 10:38, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> >
> > This function can be used to transfer EFI status code to string
> > to improve the readability of debug log.
> >
> > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> 
> I think I mentioned this the last time you sent this patch: by making
> this a static inline, those strings will be copied into each object
> file that uses this routine.
> Instead, please make it an ordinary function.
> 

Sorry for I just sent a old version patch. I will send a new one.

Thanks a lot!
Joey Lee 

> > ---
> >  include/linux/efi.h | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >
> > diff --git a/include/linux/efi.h b/include/linux/efi.h
> > index d87acf62958e..08daf4cdd807 100644
> > --- a/include/linux/efi.h
> > +++ b/include/linux/efi.h
> > @@ -42,6 +42,32 @@
> >  #define EFI_ABORTED            (21 | (1UL << (BITS_PER_LONG-1)))
> >  #define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG-1)))
> >
> > +#define EFI_STATUS_STR(_status) \
> > +       case EFI_##_status: \
> > +               return "EFI_" __stringify(_status);
> > +
> > +static inline char *
> > +efi_status_to_str(unsigned long status)
> > +{
> > +       switch (status) {
> > +       EFI_STATUS_STR(SUCCESS)
> > +       EFI_STATUS_STR(LOAD_ERROR)
> > +       EFI_STATUS_STR(INVALID_PARAMETER)
> > +       EFI_STATUS_STR(UNSUPPORTED)
> > +       EFI_STATUS_STR(BAD_BUFFER_SIZE)
> > +       EFI_STATUS_STR(BUFFER_TOO_SMALL)
> > +       EFI_STATUS_STR(NOT_READY)
> > +       EFI_STATUS_STR(DEVICE_ERROR)
> > +       EFI_STATUS_STR(WRITE_PROTECTED)
> > +       EFI_STATUS_STR(OUT_OF_RESOURCES)
> > +       EFI_STATUS_STR(NOT_FOUND)
> > +       EFI_STATUS_STR(ABORTED)
> > +       EFI_STATUS_STR(SECURITY_VIOLATION)
> > +       }
> > +
> > +       return "";
> > +}
> > +
> >  typedef unsigned long efi_status_t;
> >  typedef u8 efi_bool_t;
> >  typedef u16 efi_char16_t;              /* UNICODE character */
> > --
> > 2.16.4
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] efi: add a function for transferring status to string
  2019-03-27 19:04   ` Mimi Zohar
@ 2019-03-30  5:41     ` joeyli
  0 siblings, 0 replies; 12+ messages in thread
From: joeyli @ 2019-03-30  5:41 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Ard Biesheuvel, Lee, Chun-Yi, James Morris, Serge E . Hallyn,
	David Howells, Josh Boyer, Nayna Jain, linux-efi,
	linux-security-module, Linux Kernel Mailing List, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck

Hi Mimi,

On Wed, Mar 27, 2019 at 03:04:02PM -0400, Mimi Zohar wrote:
> On Wed, 2019-03-27 at 19:58 +0100, Ard Biesheuvel wrote:
> > On Sun, 24 Mar 2019 at 01:26, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> > >
> > > This function can be used to transfer EFI status code to string
> > > for printing out debug message. Using this function can improve
> > > the readability of log.
> 
> Maybe instead of "for transferring status" use "to convert the status
> value to a" in the Subject line and here in the patch description.
>

Thanks for your suggestion. I will change subject and description.
 
> > >
> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > Cc: Kees Cook <keescook@chromium.org>
> > > Cc: Anton Vorontsov <anton@enomsg.org>
> > > Cc: Colin Cross <ccross@android.com>
> > > Cc: Tony Luck <tony.luck@intel.com>
> > > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> > > ---
> > >  include/linux/efi.h | 28 ++++++++++++++++++++++++++++
> > >  1 file changed, 28 insertions(+)
> > >
> > > diff --git a/include/linux/efi.h b/include/linux/efi.h
> > > index 54357a258b35..a43cb0dc37af 100644
> > > --- a/include/linux/efi.h
> > > +++ b/include/linux/efi.h
> > > @@ -1768,4 +1768,32 @@ struct linux_efi_memreserve {
> > >  #define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
> > >         / sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
> > >
> > > +#define EFI_STATUS_STR(_status) \
> > > +case EFI_##_status: \
> > > +       return "EFI_" __stringify(_status);
> > > +
> > > +static inline char *
> > > +efi_status_to_str(efi_status_t status)
> > > +{
> > > +       switch (status) {
> > > +       EFI_STATUS_STR(SUCCESS)
> > > +       EFI_STATUS_STR(LOAD_ERROR)
> > > +       EFI_STATUS_STR(INVALID_PARAMETER)
> > > +       EFI_STATUS_STR(UNSUPPORTED)
> > > +       EFI_STATUS_STR(BAD_BUFFER_SIZE)
> > > +       EFI_STATUS_STR(BUFFER_TOO_SMALL)
> > > +       EFI_STATUS_STR(NOT_READY)
> > > +       EFI_STATUS_STR(DEVICE_ERROR)
> > > +       EFI_STATUS_STR(WRITE_PROTECTED)
> > > +       EFI_STATUS_STR(OUT_OF_RESOURCES)
> > > +       EFI_STATUS_STR(NOT_FOUND)
> > > +       EFI_STATUS_STR(ABORTED)
> > > +       EFI_STATUS_STR(SECURITY_VIOLATION)
> > > +       default:
> > > +               pr_warn("Unknown efi status: 0x%lx", status);
> > > +       }
> > > +
> > > +       return "Unknown efi status";
> > > +}
> > > +
> > >  #endif /* _LINUX_EFI_H */
> > > --
> > > 2.16.4
> > >
> > 
> > Please turn this into a proper function so that not every calling
> > object has to duplicate all these strings.
> 
> Hi Ard,
> 
> Keeping the status values and strings in sync is difficult.  I was
> going to suggest moving the macro immediately after the status value
> definitions.
>

I will move the code to after the status value definitions.

Thanks
Joey Lee 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] efi: add a function for transferring status to string
  2019-03-27 18:58 ` Ard Biesheuvel
  2019-03-27 19:04   ` Mimi Zohar
@ 2019-03-30  5:37   ` joeyli
  1 sibling, 0 replies; 12+ messages in thread
From: joeyli @ 2019-03-30  5:37 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Lee, Chun-Yi, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar, linux-efi,
	linux-security-module, Linux Kernel Mailing List, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck

Hi Ard,

On Wed, Mar 27, 2019 at 07:58:03PM +0100, Ard Biesheuvel wrote:
> On Sun, 24 Mar 2019 at 01:26, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> >
> > This function can be used to transfer EFI status code to string
> > for printing out debug message. Using this function can improve
> > the readability of log.
> >
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Anton Vorontsov <anton@enomsg.org>
> > Cc: Colin Cross <ccross@android.com>
> > Cc: Tony Luck <tony.luck@intel.com>
> > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> > ---
> >  include/linux/efi.h | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/include/linux/efi.h b/include/linux/efi.h
> > index 54357a258b35..a43cb0dc37af 100644
> > --- a/include/linux/efi.h
> > +++ b/include/linux/efi.h
> > @@ -1768,4 +1768,32 @@ struct linux_efi_memreserve {
> >  #define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
> >         / sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
> >
> > +#define EFI_STATUS_STR(_status) \
> > +case EFI_##_status: \
> > +       return "EFI_" __stringify(_status);
> > +
> > +static inline char *
> > +efi_status_to_str(efi_status_t status)
> > +{
> > +       switch (status) {
> > +       EFI_STATUS_STR(SUCCESS)
> > +       EFI_STATUS_STR(LOAD_ERROR)
> > +       EFI_STATUS_STR(INVALID_PARAMETER)
> > +       EFI_STATUS_STR(UNSUPPORTED)
> > +       EFI_STATUS_STR(BAD_BUFFER_SIZE)
> > +       EFI_STATUS_STR(BUFFER_TOO_SMALL)
> > +       EFI_STATUS_STR(NOT_READY)
> > +       EFI_STATUS_STR(DEVICE_ERROR)
> > +       EFI_STATUS_STR(WRITE_PROTECTED)
> > +       EFI_STATUS_STR(OUT_OF_RESOURCES)
> > +       EFI_STATUS_STR(NOT_FOUND)
> > +       EFI_STATUS_STR(ABORTED)
> > +       EFI_STATUS_STR(SECURITY_VIOLATION)
> > +       default:
> > +               pr_warn("Unknown efi status: 0x%lx", status);
> > +       }
> > +
> > +       return "Unknown efi status";
> > +}
> > +
> >  #endif /* _LINUX_EFI_H */
> > --
> > 2.16.4
> >
> 
> Please turn this into a proper function so that not every calling
> object has to duplicate all these strings.

OK! I will move them to function.

Thanks for your review!
Joey Lee

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] efi: add a function for transferring status to string
  2019-03-27 18:58 ` Ard Biesheuvel
@ 2019-03-27 19:04   ` Mimi Zohar
  2019-03-30  5:41     ` joeyli
  2019-03-30  5:37   ` joeyli
  1 sibling, 1 reply; 12+ messages in thread
From: Mimi Zohar @ 2019-03-27 19:04 UTC (permalink / raw)
  To: Ard Biesheuvel, Lee, Chun-Yi
  Cc: James Morris, Serge E . Hallyn, David Howells, Josh Boyer,
	Nayna Jain, linux-efi, linux-security-module,
	Linux Kernel Mailing List, Lee, Chun-Yi, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck

On Wed, 2019-03-27 at 19:58 +0100, Ard Biesheuvel wrote:
> On Sun, 24 Mar 2019 at 01:26, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> >
> > This function can be used to transfer EFI status code to string
> > for printing out debug message. Using this function can improve
> > the readability of log.

Maybe instead of "for transferring status" use "to convert the status
value to a" in the Subject line and here in the patch description.

> >
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Anton Vorontsov <anton@enomsg.org>
> > Cc: Colin Cross <ccross@android.com>
> > Cc: Tony Luck <tony.luck@intel.com>
> > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> > ---
> >  include/linux/efi.h | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/include/linux/efi.h b/include/linux/efi.h
> > index 54357a258b35..a43cb0dc37af 100644
> > --- a/include/linux/efi.h
> > +++ b/include/linux/efi.h
> > @@ -1768,4 +1768,32 @@ struct linux_efi_memreserve {
> >  #define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
> >         / sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
> >
> > +#define EFI_STATUS_STR(_status) \
> > +case EFI_##_status: \
> > +       return "EFI_" __stringify(_status);
> > +
> > +static inline char *
> > +efi_status_to_str(efi_status_t status)
> > +{
> > +       switch (status) {
> > +       EFI_STATUS_STR(SUCCESS)
> > +       EFI_STATUS_STR(LOAD_ERROR)
> > +       EFI_STATUS_STR(INVALID_PARAMETER)
> > +       EFI_STATUS_STR(UNSUPPORTED)
> > +       EFI_STATUS_STR(BAD_BUFFER_SIZE)
> > +       EFI_STATUS_STR(BUFFER_TOO_SMALL)
> > +       EFI_STATUS_STR(NOT_READY)
> > +       EFI_STATUS_STR(DEVICE_ERROR)
> > +       EFI_STATUS_STR(WRITE_PROTECTED)
> > +       EFI_STATUS_STR(OUT_OF_RESOURCES)
> > +       EFI_STATUS_STR(NOT_FOUND)
> > +       EFI_STATUS_STR(ABORTED)
> > +       EFI_STATUS_STR(SECURITY_VIOLATION)
> > +       default:
> > +               pr_warn("Unknown efi status: 0x%lx", status);
> > +       }
> > +
> > +       return "Unknown efi status";
> > +}
> > +
> >  #endif /* _LINUX_EFI_H */
> > --
> > 2.16.4
> >
> 
> Please turn this into a proper function so that not every calling
> object has to duplicate all these strings.

Hi Ard,

Keeping the status values and strings in sync is difficult.  I was
going to suggest moving the macro immediately after the status value
definitions.

Mimi 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] efi: add a function for transferring status to string
  2019-03-24  0:26 [PATCH 1/2] efi: add a function for transferring status to string Lee, Chun-Yi
@ 2019-03-27 18:58 ` Ard Biesheuvel
  2019-03-27 19:04   ` Mimi Zohar
  2019-03-30  5:37   ` joeyli
  0 siblings, 2 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2019-03-27 18:58 UTC (permalink / raw)
  To: Lee, Chun-Yi
  Cc: James Morris, Serge E . Hallyn, David Howells, Josh Boyer,
	Nayna Jain, Mimi Zohar, linux-efi, linux-security-module,
	Linux Kernel Mailing List, Lee, Chun-Yi, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck

On Sun, 24 Mar 2019 at 01:26, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
>
> This function can be used to transfer EFI status code to string
> for printing out debug message. Using this function can improve
> the readability of log.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Anton Vorontsov <anton@enomsg.org>
> Cc: Colin Cross <ccross@android.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
> ---
>  include/linux/efi.h | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 54357a258b35..a43cb0dc37af 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1768,4 +1768,32 @@ struct linux_efi_memreserve {
>  #define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
>         / sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
>
> +#define EFI_STATUS_STR(_status) \
> +case EFI_##_status: \
> +       return "EFI_" __stringify(_status);
> +
> +static inline char *
> +efi_status_to_str(efi_status_t status)
> +{
> +       switch (status) {
> +       EFI_STATUS_STR(SUCCESS)
> +       EFI_STATUS_STR(LOAD_ERROR)
> +       EFI_STATUS_STR(INVALID_PARAMETER)
> +       EFI_STATUS_STR(UNSUPPORTED)
> +       EFI_STATUS_STR(BAD_BUFFER_SIZE)
> +       EFI_STATUS_STR(BUFFER_TOO_SMALL)
> +       EFI_STATUS_STR(NOT_READY)
> +       EFI_STATUS_STR(DEVICE_ERROR)
> +       EFI_STATUS_STR(WRITE_PROTECTED)
> +       EFI_STATUS_STR(OUT_OF_RESOURCES)
> +       EFI_STATUS_STR(NOT_FOUND)
> +       EFI_STATUS_STR(ABORTED)
> +       EFI_STATUS_STR(SECURITY_VIOLATION)
> +       default:
> +               pr_warn("Unknown efi status: 0x%lx", status);
> +       }
> +
> +       return "Unknown efi status";
> +}
> +
>  #endif /* _LINUX_EFI_H */
> --
> 2.16.4
>

Please turn this into a proper function so that not every calling
object has to duplicate all these strings.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] efi: add a function for transferring status to string
@ 2019-03-24  0:26 Lee, Chun-Yi
  2019-03-27 18:58 ` Ard Biesheuvel
  0 siblings, 1 reply; 12+ messages in thread
From: Lee, Chun-Yi @ 2019-03-24  0:26 UTC (permalink / raw)
  To: Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar
  Cc: linux-efi, linux-security-module, linux-kernel, Lee, Chun-Yi,
	Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck

This function can be used to transfer EFI status code to string
for printing out debug message. Using this function can improve
the readability of log.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Tony Luck <tony.luck@intel.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 include/linux/efi.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 54357a258b35..a43cb0dc37af 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1768,4 +1768,32 @@ struct linux_efi_memreserve {
 #define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
 	/ sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
 
+#define EFI_STATUS_STR(_status) \
+case EFI_##_status: \
+	return "EFI_" __stringify(_status);
+
+static inline char *
+efi_status_to_str(efi_status_t status)
+{
+	switch (status) {
+	EFI_STATUS_STR(SUCCESS)
+	EFI_STATUS_STR(LOAD_ERROR)
+	EFI_STATUS_STR(INVALID_PARAMETER)
+	EFI_STATUS_STR(UNSUPPORTED)
+	EFI_STATUS_STR(BAD_BUFFER_SIZE)
+	EFI_STATUS_STR(BUFFER_TOO_SMALL)
+	EFI_STATUS_STR(NOT_READY)
+	EFI_STATUS_STR(DEVICE_ERROR)
+	EFI_STATUS_STR(WRITE_PROTECTED)
+	EFI_STATUS_STR(OUT_OF_RESOURCES)
+	EFI_STATUS_STR(NOT_FOUND)
+	EFI_STATUS_STR(ABORTED)
+	EFI_STATUS_STR(SECURITY_VIOLATION)
+	default:
+		pr_warn("Unknown efi status: 0x%lx", status);
+	}
+
+	return "Unknown efi status";
+}
+
 #endif /* _LINUX_EFI_H */
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 1/2] efi: add a function for transferring status to string
@ 2019-03-23  8:37 Lee, Chun-Yi
  0 siblings, 0 replies; 12+ messages in thread
From: Lee, Chun-Yi @ 2019-03-23  8:37 UTC (permalink / raw)
  To: Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar
  Cc: linux-efi, linux-security-module, linux-kernel, Lee, Chun-Yi,
	Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck

This function can be used to transfer EFI status code to string
for printing out debug message. Using this function can improve
the readability of log.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Tony Luck <tony.luck@intel.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 include/linux/efi.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 54357a258b35..a43cb0dc37af 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1768,4 +1768,32 @@ struct linux_efi_memreserve {
 #define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
 	/ sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
 
+#define EFI_STATUS_STR(_status) \
+case EFI_##_status: \
+	return "EFI_" __stringify(_status);
+
+static inline char *
+efi_status_to_str(efi_status_t status)
+{
+	switch (status) {
+	EFI_STATUS_STR(SUCCESS)
+	EFI_STATUS_STR(LOAD_ERROR)
+	EFI_STATUS_STR(INVALID_PARAMETER)
+	EFI_STATUS_STR(UNSUPPORTED)
+	EFI_STATUS_STR(BAD_BUFFER_SIZE)
+	EFI_STATUS_STR(BUFFER_TOO_SMALL)
+	EFI_STATUS_STR(NOT_READY)
+	EFI_STATUS_STR(DEVICE_ERROR)
+	EFI_STATUS_STR(WRITE_PROTECTED)
+	EFI_STATUS_STR(OUT_OF_RESOURCES)
+	EFI_STATUS_STR(NOT_FOUND)
+	EFI_STATUS_STR(ABORTED)
+	EFI_STATUS_STR(SECURITY_VIOLATION)
+	default:
+		pr_warn("Unknown efi status: 0x%lx", status);
+	}
+
+	return "Unknown efi status";
+}
+
 #endif /* _LINUX_EFI_H */
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 1/2] efi: add a function for transferring status to string
@ 2019-03-22 10:33 Lee, Chun-Yi
  0 siblings, 0 replies; 12+ messages in thread
From: Lee, Chun-Yi @ 2019-03-22 10:33 UTC (permalink / raw)
  To: Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells,
	Josh Boyer, Nayna Jain, Mimi Zohar
  Cc: linux-efi, linux-security-module, linux-kernel, Lee, Chun-Yi,
	Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck

This function can be used to transfer EFI status code to string
for printing out debug message. Using this function can improve
the readability of log.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Anton Vorontsov <anton@enomsg.org>
Cc: Colin Cross <ccross@android.com>
Cc: Tony Luck <tony.luck@intel.com>
Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com>
---
 include/linux/efi.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 54357a258b35..a43cb0dc37af 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -1768,4 +1768,32 @@ struct linux_efi_memreserve {
 #define EFI_MEMRESERVE_COUNT(size) (((size) - sizeof(struct linux_efi_memreserve)) \
 	/ sizeof(((struct linux_efi_memreserve *)0)->entry[0]))
 
+#define EFI_STATUS_STR(_status) \
+case EFI_##_status: \
+	return "EFI_" __stringify(_status);
+
+static inline char *
+efi_status_to_str(efi_status_t status)
+{
+	switch (status) {
+	EFI_STATUS_STR(SUCCESS)
+	EFI_STATUS_STR(LOAD_ERROR)
+	EFI_STATUS_STR(INVALID_PARAMETER)
+	EFI_STATUS_STR(UNSUPPORTED)
+	EFI_STATUS_STR(BAD_BUFFER_SIZE)
+	EFI_STATUS_STR(BUFFER_TOO_SMALL)
+	EFI_STATUS_STR(NOT_READY)
+	EFI_STATUS_STR(DEVICE_ERROR)
+	EFI_STATUS_STR(WRITE_PROTECTED)
+	EFI_STATUS_STR(OUT_OF_RESOURCES)
+	EFI_STATUS_STR(NOT_FOUND)
+	EFI_STATUS_STR(ABORTED)
+	EFI_STATUS_STR(SECURITY_VIOLATION)
+	default:
+		pr_warn("Unknown efi status: 0x%lx", status);
+	}
+
+	return "Unknown efi status";
+}
+
 #endif /* _LINUX_EFI_H */
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-12-12 14:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-12  9:38 [PATCH 0/2] efi: cosmetic patches for the error messages when loading certificates Lee, Chun-Yi
2019-12-12  9:38 ` [PATCH 1/2] efi: add a function for transferring status to string Lee, Chun-Yi
2019-12-12 11:20   ` Ard Biesheuvel
2019-12-12 14:24     ` Joey Lee
2019-12-12  9:38 ` [PATCH 2/2] efi: show error messages only when loading certificates is failed Lee, Chun-Yi
  -- strict thread matches above, loose matches on Subject: below --
2019-03-24  0:26 [PATCH 1/2] efi: add a function for transferring status to string Lee, Chun-Yi
2019-03-27 18:58 ` Ard Biesheuvel
2019-03-27 19:04   ` Mimi Zohar
2019-03-30  5:41     ` joeyli
2019-03-30  5:37   ` joeyli
2019-03-23  8:37 Lee, Chun-Yi
2019-03-22 10:33 Lee, Chun-Yi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).