* [PATCH 1/2] efi: add a function for transferring status to string @ 2019-03-24 0:26 Lee, Chun-Yi 2019-03-24 0:26 ` [PATCH 2/2 v2] efi: print appropriate status message when loading certificates Lee, Chun-Yi 2019-03-27 18:58 ` [PATCH 1/2] efi: add a function for transferring status to string Ard Biesheuvel 0 siblings, 2 replies; 8+ 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] 8+ messages in thread
* [PATCH 2/2 v2] efi: print appropriate status message when loading certificates 2019-03-24 0:26 [PATCH 1/2] efi: add a function for transferring status to string Lee, Chun-Yi @ 2019-03-24 0:26 ` Lee, Chun-Yi 2019-03-27 19:23 ` Mimi Zohar 2019-03-27 18:58 ` [PATCH 1/2] efi: add a function for transferring status to string Ard Biesheuvel 1 sibling, 1 reply; 8+ 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 When loading certificates list from UEFI variable, the original error message direct shows the efi status code from UEFI firmware. 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 So, this patch shows the status string instead of status code. On the other hand, the "Couldn't get UEFI" message doesn't need to be exposed when db/dbx/mok variable do not exist. So, this patch set the message level to debug. v2. Setting the MODSIGN messagse level to debug. Link: https://forums.opensuse.org/showthread.php/535324-MODSIGN-Couldn-t-get-UEFI-db-list?p=2897516#post2897516 Cc: James Morris <jmorris@namei.org> Cc: Serge E. Hallyn" <serge@hallyn.com> Cc: David Howells <dhowells@redhat.com> Cc: Nayna Jain <nayna@linux.ibm.com> Cc: Josh Boyer <jwboyer@fedoraproject.org> Cc: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> --- security/integrity/platform_certs/load_uefi.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c index 81b19c52832b..e65244b31f04 100644 --- a/security/integrity/platform_certs/load_uefi.c +++ b/security/integrity/platform_certs/load_uefi.c @@ -48,7 +48,9 @@ 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); + if (status != EFI_NOT_FOUND) + pr_err("Couldn't get size: %s\n", + efi_status_to_str(status)); return NULL; } @@ -59,7 +61,8 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid, 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); + pr_err("Error reading db var: %s\n", + efi_status_to_str(status)); return NULL; } @@ -155,7 +158,7 @@ static int __init load_uefi_certs(void) 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"); + pr_debug("MODSIGN: Couldn't get UEFI db list\n"); } else { rc = parse_efi_signature_list("UEFI:db", db, dbsize, get_handler_for_db); @@ -168,7 +171,7 @@ 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"); + pr_debug("Couldn't get UEFI MokListRT\n"); } else { rc = parse_efi_signature_list("UEFI:MokListRT", mok, moksize, get_handler_for_db); @@ -179,7 +182,7 @@ static int __init load_uefi_certs(void) dbx = get_cert_list(L"dbx", &secure_var, &dbxsize); if (!dbx) { - pr_info("Couldn't get UEFI dbx list\n"); + pr_debug("Couldn't get UEFI dbx list\n"); } else { rc = parse_efi_signature_list("UEFI:dbx", dbx, dbxsize, -- 2.16.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2 v2] efi: print appropriate status message when loading certificates 2019-03-24 0:26 ` [PATCH 2/2 v2] efi: print appropriate status message when loading certificates Lee, Chun-Yi @ 2019-03-27 19:23 ` Mimi Zohar 2019-03-29 17:40 ` jlee 0 siblings, 1 reply; 8+ messages in thread From: Mimi Zohar @ 2019-03-27 19:23 UTC (permalink / raw) To: Lee, Chun-Yi, Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells, Josh Boyer, Nayna Jain Cc: linux-efi, linux-security-module, linux-kernel, Lee, Chun-Yi On Sun, 2019-03-24 at 08:26 +0800, Lee, Chun-Yi wrote: > When loading certificates list from UEFI variable, the original error > message direct shows the efi status code from UEFI firmware. 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 > > So, this patch shows the status string instead of status code. > > On the other hand, the "Couldn't get UEFI" message doesn't need > to be exposed when db/dbx/mok variable do not exist. So, this > patch set the message level to debug. > > v2. > Setting the MODSIGN messagse level to debug. > > Link: https://forums.opensuse.org/showthread.php/535324-MODSIGN-Couldn-t-get-UEFI-db-list?p=2897516#post2897516 > Cc: James Morris <jmorris@namei.org> > Cc: Serge E. Hallyn" <serge@hallyn.com> > Cc: David Howells <dhowells@redhat.com> > Cc: Nayna Jain <nayna@linux.ibm.com> > Cc: Josh Boyer <jwboyer@fedoraproject.org> > Cc: Mimi Zohar <zohar@linux.ibm.com> > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> > --- > security/integrity/platform_certs/load_uefi.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c > index 81b19c52832b..e65244b31f04 100644 > --- a/security/integrity/platform_certs/load_uefi.c > +++ b/security/integrity/platform_certs/load_uefi.c > @@ -48,7 +48,9 @@ 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); > + if (status != EFI_NOT_FOUND) > + pr_err("Couldn't get size: %s\n", > + efi_status_to_str(status)); > return NULL; > } > > @@ -59,7 +61,8 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid, > 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); > + pr_err("Error reading db var: %s\n", > + efi_status_to_str(status)); > return NULL; > } > > @@ -155,7 +158,7 @@ static int __init load_uefi_certs(void) > 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"); > + pr_debug("MODSIGN: Couldn't get UEFI db list\n"); Sure, this is fine. > } else { > rc = parse_efi_signature_list("UEFI:db", > db, dbsize, get_handler_for_db); > @@ -168,7 +171,7 @@ 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"); > + pr_debug("Couldn't get UEFI MokListRT\n"); This is fine too. > } else { > rc = parse_efi_signature_list("UEFI:MokListRT", > mok, moksize, get_handler_for_db); > @@ -179,7 +182,7 @@ static int __init load_uefi_certs(void) > > dbx = get_cert_list(L"dbx", &secure_var, &dbxsize); > if (!dbx) { > - pr_info("Couldn't get UEFI dbx list\n"); > + pr_debug("Couldn't get UEFI dbx list\n"); If there isn't a db or moklist, then this is fine. My concern is not having an indication that the dbx wasn't installed, when it should have been. Perhaps similar to the "Loading compiled-in X.509 certificates" informational message there should informational messages for db, mok, and dbx as well. Mimi > } else { > rc = parse_efi_signature_list("UEFI:dbx", > dbx, dbxsize, ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2 v2] efi: print appropriate status message when loading certificates 2019-03-27 19:23 ` Mimi Zohar @ 2019-03-29 17:40 ` jlee 0 siblings, 0 replies; 8+ messages in thread From: jlee @ 2019-03-29 17:40 UTC (permalink / raw) To: Mimi Zohar Cc: Lee, Chun-Yi, Ard Biesheuvel, James Morris, Serge E . Hallyn, David Howells, Josh Boyer, Nayna Jain, linux-efi, linux-security-module, linux-kernel On Wed, Mar 27, 2019 at 03:23:55PM -0400, Mimi Zohar wrote: > On Sun, 2019-03-24 at 08:26 +0800, Lee, Chun-Yi wrote: > > When loading certificates list from UEFI variable, the original error > > message direct shows the efi status code from UEFI firmware. 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 > > > > So, this patch shows the status string instead of status code. > > > > On the other hand, the "Couldn't get UEFI" message doesn't need > > to be exposed when db/dbx/mok variable do not exist. So, this > > patch set the message level to debug. > > > > v2. > > Setting the MODSIGN messagse level to debug. > > > > Link: https://forums.opensuse.org/showthread.php/535324-MODSIGN-Couldn-t-get-UEFI-db-list?p=2897516#post2897516 > > Cc: James Morris <jmorris@namei.org> > > Cc: Serge E. Hallyn" <serge@hallyn.com> > > Cc: David Howells <dhowells@redhat.com> > > Cc: Nayna Jain <nayna@linux.ibm.com> > > Cc: Josh Boyer <jwboyer@fedoraproject.org> > > Cc: Mimi Zohar <zohar@linux.ibm.com> > > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> > > --- > > security/integrity/platform_certs/load_uefi.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c > > index 81b19c52832b..e65244b31f04 100644 > > --- a/security/integrity/platform_certs/load_uefi.c > > +++ b/security/integrity/platform_certs/load_uefi.c > > @@ -48,7 +48,9 @@ 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); > > + if (status != EFI_NOT_FOUND) > > + pr_err("Couldn't get size: %s\n", > > + efi_status_to_str(status)); > > return NULL; > > } > > > > @@ -59,7 +61,8 @@ static __init void *get_cert_list(efi_char16_t *name, efi_guid_t *guid, > > 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); > > + pr_err("Error reading db var: %s\n", > > + efi_status_to_str(status)); > > return NULL; > > } > > > > @@ -155,7 +158,7 @@ static int __init load_uefi_certs(void) > > 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"); > > + pr_debug("MODSIGN: Couldn't get UEFI db list\n"); > > Sure, this is fine. > > > } else { > > rc = parse_efi_signature_list("UEFI:db", > > db, dbsize, get_handler_for_db); > > @@ -168,7 +171,7 @@ 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"); > > + pr_debug("Couldn't get UEFI MokListRT\n"); > > This is fine too. > > > } else { > > rc = parse_efi_signature_list("UEFI:MokListRT", > > mok, moksize, get_handler_for_db); > > @@ -179,7 +182,7 @@ static int __init load_uefi_certs(void) > > > > dbx = get_cert_list(L"dbx", &secure_var, &dbxsize); > > if (!dbx) { > > - pr_info("Couldn't get UEFI dbx list\n"); > > + pr_debug("Couldn't get UEFI dbx list\n"); > > If there isn't a db or moklist, then this is fine. My concern is not > having an indication that the dbx wasn't installed, when it should > have been. > > Perhaps similar to the "Loading compiled-in X.509 certificates" > informational message there should informational messages for db, mok, > and dbx as well. > OK. I will add message when kernel found db, dbx and mok. It will just like the informaton message for ACPI S0,S3,S4 support. Thanks Joey Lee ^ permalink raw reply [flat|nested] 8+ 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-24 0:26 ` [PATCH 2/2 v2] efi: print appropriate status message when loading certificates Lee, Chun-Yi @ 2019-03-27 18:58 ` Ard Biesheuvel 2019-03-27 19:04 ` Mimi Zohar 2019-03-30 5:37 ` joeyli 1 sibling, 2 replies; 8+ 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] 8+ messages in thread
* Re: [PATCH 1/2] efi: add a function for transferring status to string 2019-03-27 18:58 ` [PATCH 1/2] efi: add a function for transferring status to string 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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread
* Re: [PATCH 1/2] efi: add a function for transferring status to string 2019-03-27 18:58 ` [PATCH 1/2] efi: add a function for transferring status to string Ard Biesheuvel 2019-03-27 19:04 ` Mimi Zohar @ 2019-03-30 5:37 ` joeyli 1 sibling, 0 replies; 8+ 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] 8+ messages in thread
end of thread, other threads:[~2019-03-30 5:42 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-24 0:26 [PATCH 1/2] efi: add a function for transferring status to string Lee, Chun-Yi 2019-03-24 0:26 ` [PATCH 2/2 v2] efi: print appropriate status message when loading certificates Lee, Chun-Yi 2019-03-27 19:23 ` Mimi Zohar 2019-03-29 17:40 ` jlee 2019-03-27 18:58 ` [PATCH 1/2] efi: add a function for transferring status to string Ard Biesheuvel 2019-03-27 19:04 ` Mimi Zohar 2019-03-30 5:41 ` joeyli 2019-03-30 5:37 ` joeyli
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.