Re: [PATCH 3/5] efi_loader: image_loader: replace EFI_PRINT with log macros

On 7/5/22 07:48, AKASHI Takahiro wrote:

Now We are migrating from EFI_PRINT() to log macro's.

I don't understand your logic:

log_err("Parsing image's signature failed\n"); log_debug("Signature was rejected by "dbx"\n");

Shouldn't everything leading to a signature being rejected be treated the same (i.e. use log_err())?

Best regards

Heinrich

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

lib/efi_loader/efi_image_loader.c | 54 +++++++++++++++---------------- 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 961139888504..fe8e4a89082c 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -238,7 +238,7 @@ efi_status_t efi_image_region_add(struct efi_image_regions *regs, int i, j;

if (regs->num >= regs->max) {

• EFI_PRINT("%s: no more room for regions\n", __func__);
  

• log_err("%s: no more room for regions\n", __func__);
  

  return EFI_OUT_OF_RESOURCES; }

@@ -263,7 +263,7 @@ efi_status_t efi_image_region_add(struct efi_image_regions *regs, }

/* new data overlapping registered region */

• EFI_PRINT("%s: new region already part of another\n", __func__);
  

• log_err("%s: new region already part of another\n", __func__);
  

  return EFI_INVALID_PARAMETER; }

@@ -434,8 +434,8 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, bytes_hashed = opt->SizeOfHeaders; align = opt->FileAlignment; } else {

• EFI_PRINT("%s: Invalid optional header magic %x\n", __func__,
  

• 	  nt->OptionalHeader.Magic);
  

• log_err("%s: Invalid optional header magic %x\n", __func__,
  

• 	nt->OptionalHeader.Magic);
  

  goto err; }

@@ -445,7 +445,7 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, nt->FileHeader.SizeOfOptionalHeader); sorted = calloc(sizeof(IMAGE_SECTION_HEADER *), num_sections); if (!sorted) {

• EFI_PRINT("%s: Out of memory\n", __func__);
  

• log_err("%s: Out of memory\n", __func__);
  

  goto err; }

@@ -464,7 +464,7 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, efi_image_region_add(regs, efi + sorted[i]->PointerToRawData, efi + sorted[i]->PointerToRawData + size, 0);

• EFI_PRINT("section[%d](%s): raw: 0x%x-0x%x, virt: %x-%x\n",
  

• log_debug("section[%d](%s): raw: 0x%x-0x%x, virt: %x-%x\n",
    i, sorted[i]->Name,
    sorted[i]->PointerToRawData,
    sorted[i]->PointerToRawData + size,
  

@@ -478,7 +478,7 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp,

/* 3. Extra data excluding Certificates Table */ if (bytes_hashed + authsz < len) {

• EFI_PRINT("extra data for hash: %zu\n",
  

• log_debug("extra data for hash: %zu\n",
    len - (bytes_hashed + authsz));
  

  efi_image_region_add(regs, efi + bytes_hashed, efi + len - authsz, 0);

@@ -487,18 +487,18 @@ bool efi_image_parse(void *efi, size_t len, struct efi_image_regions **regp, /* Return Certificates Table */ if (authsz) { if (len < authoff + authsz) {

• 	EFI_PRINT("%s: Size for auth too large: %u >= %zu\n",
  

• 		  __func__, authsz, len - authoff);
  

• 	log_err("%s: Size for auth too large: %u >= %zu\n",
  

• 		__func__, authsz, len - authoff);
  goto err;
  

  } if (authsz < sizeof(*auth)) {

• 	EFI_PRINT("%s: Size for auth too small: %u < %zu\n",
  

• 		  __func__, authsz, sizeof(*auth));
  

• 	log_err("%s: Size for auth too small: %u < %zu\n",
  

• 		__func__, authsz, sizeof(*auth));
  goto err;
  

  } *auth = efi + authoff; *auth_len = authsz;

• EFI_PRINT("WIN_CERTIFICATE: 0x%x, size: 0x%x\n", authoff,
  

• log_debug("WIN_CERTIFICATE: 0x%x, size: 0x%x\n", authoff,
    authsz);
  

  } else { *auth = NULL;

@@ -549,7 +549,7 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) size_t auth_size; bool ret = false;

• EFI_PRINT("%s: Enter, %d\n", __func__, ret);

• log_debug("%s: Enter, %d\n", __func__, ret);

  if (!efi_secure_boot_enabled()) return true;

@@ -560,7 +560,7 @@ static bool efi_image_authenticate(void *efi, size_t efi_size)

if (!efi_image_parse(new_efi, efi_size, &regs, &wincerts, &wincerts_len)) {

• EFI_PRINT("Parsing PE executable image failed\n");
  

• log_err("Parsing PE executable image failed\n");
  

  goto out; }

@@ -569,18 +569,18 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) */ db = efi_sigstore_parse_sigdb(u"db"); if (!db) {

• EFI_PRINT("Getting signature database(db) failed\n");
  

• log_err("Getting signature database(db) failed\n");
  

  goto out; }

  dbx = efi_sigstore_parse_sigdb(u"dbx"); if (!dbx) {

• EFI_PRINT("Getting signature database(dbx) failed\n");
  

• log_err("Getting signature database(dbx) failed\n");
  

  goto out; }

  if (efi_signature_lookup_digest(regs, dbx, true)) {

• EFI_PRINT("Image's digest was found in \"dbx\"\n");
  

• log_debug("Image's digest was found in \"dbx\"\n");
  

  goto out; }

@@ -602,12 +602,12 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) break;

if (wincert->dwLength <= sizeof(*wincert)) {

• 	EFI_PRINT("dwLength too small: %u < %zu\n",
  

• 	log_debug("dwLength too small: %u < %zu\n",
  	  wincert->dwLength, sizeof(*wincert));
  continue;
  

  }

• EFI_PRINT("WIN_CERTIFICATE_TYPE: 0x%x\n",
  

• log_debug("WIN_CERTIFICATE_TYPE: 0x%x\n",
    wincert->wCertificateType);
  
  

  auth = (u8 *)wincert + sizeof(*wincert);

@@ -617,12 +617,12 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) break;

	if (auth_size <= sizeof(efi_guid_t)) {

• 		EFI_PRINT("dwLength too small: %u < %zu\n",
  

• 		log_debug("dwLength too small: %u < %zu\n",
  		  wincert->dwLength, sizeof(*wincert));
  	continue;
  }
  if (guidcmp(auth, &efi_guid_cert_type_pkcs7)) {
  

• 		EFI_PRINT("Certificate type not supported: %pUs\n",
  

• 		log_debug("Certificate type not supported: %pUs\n",
  		  auth);
  	ret = false;
  	goto out;
  

@@ -632,14 +632,14 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) auth_size -= sizeof(efi_guid_t); } else if (wincert->wCertificateType != WIN_CERT_TYPE_PKCS_SIGNED_DATA) {

• 	EFI_PRINT("Certificate type not supported\n");
  

• 	log_debug("Certificate type not supported\n");
  ret = false;
  goto out;
  

  }

  msg = pkcs7_parse_message(auth, auth_size); if (IS_ERR(msg)) {

• 	EFI_PRINT("Parsing image's signature failed\n");
  

• 	log_err("Parsing image's signature failed\n");
  msg = NULL;
  continue;
  

  }

@@ -666,13 +666,13 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) /* try black-list first */ if (efi_signature_verify_one(regs, msg, dbx)) { ret = false;

• 	EFI_PRINT("Signature was rejected by \"dbx\"\n");
  

• 	log_debug("Signature was rejected by \"dbx\"\n");
  goto out;
  

  }

  if (!efi_signature_check_signers(msg, dbx)) { ret = false;

• 	EFI_PRINT("Signer(s) in \"dbx\"\n");
  

• 	log_debug("Signer(s) in \"dbx\"\n");
  goto out;
  

  }

@@ -682,7 +682,7 @@ static bool efi_image_authenticate(void *efi, size_t efi_size) continue; }

• EFI_PRINT("Signature was not verified by \"db\"\n");
  

• log_debug("Signature was not verified by \"db\"\n");
  

  }

@@ -698,7 +698,7 @@ out: if (new_efi != efi) free(new_efi);

• EFI_PRINT("%s: Exit, %d\n", __func__, ret);

• log_debug("%s: Exit, %d\n", __func__, ret); return ret; } #else