Blame SOURCES/0340-verifiers-fix-double-close-on-pgp-s-sig-file-descrip.patch

80913e
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
80913e
From: Michael Chang <mchang@suse.com>
80913e
Date: Tue, 20 Nov 2018 19:15:37 +0800
80913e
Subject: [PATCH] verifiers: fix double close on pgp's sig file descriptor
80913e
80913e
An error emerged as when I was testing the verifiers branch, so instead
80913e
of putting it in pgp prefix, the verifiers is used to reflect what the
80913e
patch is based on.
80913e
80913e
While running verify_detached, grub aborts with error.
80913e
80913e
verify_detached /@/.snapshots/1/snapshot/boot/grub/grub.cfg
80913e
/@/.snapshots/1/snapshot/boot/grub/grub.cfg.sig
80913e
80913e
alloc magic is broken at 0x7beea660: 0
80913e
Aborted. Press any key to exit.
80913e
80913e
The error is caused by sig file descriptor been closed twice, first time
80913e
in grub_verify_signature() to which it is passed as parameter. Second in
80913e
grub_cmd_verify_signature() or in whichever opens the sig file
80913e
descriptor. The second close is not consider as bug to me either, as in
80913e
common rule of what opens a file has to close it to avoid file
80913e
descriptor leakage.
80913e
80913e
After all the design of grub_verify_signature() makes it difficult to keep
80913e
a good trace on opened file descriptor from it's caller. Let's refine
80913e
the application interface to accept file path rather than descriptor, in
80913e
this way the caller doesn't have to care about closing the descriptor by
80913e
delegating it to grub_verify_signature() with full tracing to opened
80913e
file descriptor by itself.
80913e
80913e
Also making it clear that sig descriptor is not referenced in error
80913e
returning path of grub_verify_signature_init(), so it can be closed
80913e
directly by it's caller. This also makes delegating it to
80913e
grub_pubkey_close() infeasible to help in relieving file descriptor
80913e
leakage as it has to depend on uncertainty of ctxt fields in error
80913e
returning path.
80913e
80913e
Signed-off-by: Michael Chang <mchang@suse.com>
80913e
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
80913e
---
80913e
 grub-core/commands/pgp.c | 35 +++++++++++++++++------------------
80913e
 include/grub/pubkey.h    |  2 +-
80913e
 2 files changed, 18 insertions(+), 19 deletions(-)
80913e
80913e
diff --git a/grub-core/commands/pgp.c b/grub-core/commands/pgp.c
b32e65
index 5c913c2e2..d39846d8c 100644
80913e
--- a/grub-core/commands/pgp.c
80913e
+++ b/grub-core/commands/pgp.c
80913e
@@ -495,13 +495,12 @@ grub_verify_signature_init (struct grub_pubkey_context *ctxt, grub_file_t sig)
80913e
 
80913e
   grub_dprintf ("crypt", "alive\n");
80913e
 
80913e
-  ctxt->sig = sig;
80913e
-
80913e
   ctxt->hash_context = grub_zalloc (ctxt->hash->contextsize);
80913e
   if (!ctxt->hash_context)
80913e
     return grub_errno;
80913e
 
80913e
   ctxt->hash->init (ctxt->hash_context);
80913e
+  ctxt->sig = sig;
80913e
 
80913e
   return GRUB_ERR_NONE;
80913e
 }
80913e
@@ -684,16 +683,26 @@ grub_pubkey_close (void *ctxt)
80913e
 }
80913e
 
80913e
 grub_err_t
80913e
-grub_verify_signature (grub_file_t f, grub_file_t sig,
80913e
+grub_verify_signature (grub_file_t f, const char *fsig,
80913e
 		       struct grub_public_key *pkey)
80913e
 {
80913e
+  grub_file_t sig;
80913e
   grub_err_t err;
80913e
   struct grub_pubkey_context ctxt;
80913e
   grub_uint8_t *readbuf = NULL;
80913e
 
80913e
+  sig = grub_file_open (fsig,
80913e
+			GRUB_FILE_TYPE_SIGNATURE
80913e
+			| GRUB_FILE_TYPE_NO_DECOMPRESS);
80913e
+  if (!sig)
80913e
+    return grub_errno;
80913e
+
80913e
   err = grub_verify_signature_init (&ctxt, sig);
80913e
   if (err)
80913e
-    return err;
80913e
+    {
80913e
+      grub_file_close (sig);
80913e
+      return err;
80913e
+    }
80913e
 
80913e
   readbuf = grub_zalloc (READBUF_SIZE);
80913e
   if (!readbuf)
80913e
@@ -807,7 +816,7 @@ static grub_err_t
80913e
 grub_cmd_verify_signature (grub_extcmd_context_t ctxt,
80913e
 			   int argc, char **args)
80913e
 {
80913e
-  grub_file_t f = NULL, sig = NULL;
80913e
+  grub_file_t f = NULL;
80913e
   grub_err_t err = GRUB_ERR_NONE;
80913e
   struct grub_public_key *pk = NULL;
80913e
 
80913e
@@ -845,19 +854,8 @@ grub_cmd_verify_signature (grub_extcmd_context_t ctxt,
80913e
       goto fail;
80913e
     }
80913e
 
80913e
-  sig = grub_file_open (args[1],
80913e
-			GRUB_FILE_TYPE_SIGNATURE
80913e
-			| GRUB_FILE_TYPE_NO_DECOMPRESS);
80913e
-  if (!sig)
80913e
-    {
80913e
-      err = grub_errno;
80913e
-      goto fail;
80913e
-    }
80913e
-
80913e
-  err = grub_verify_signature (f, sig, pk);
80913e
+  err = grub_verify_signature (f, args[1], pk);
80913e
  fail:
80913e
-  if (sig)
80913e
-    grub_file_close (sig);
80913e
   if (f)
80913e
     grub_file_close (f);
80913e
   if (pk)
80913e
@@ -902,7 +900,8 @@ grub_pubkey_init (grub_file_t io, enum grub_file_type type __attribute__ ((unuse
80913e
   err = grub_verify_signature_init (ctxt, sig);
80913e
   if (err)
80913e
     {
80913e
-      grub_pubkey_close (ctxt);
80913e
+      grub_free (ctxt);
80913e
+      grub_file_close (sig);
80913e
       return err;
80913e
     }
80913e
   *context = ctxt;
80913e
diff --git a/include/grub/pubkey.h b/include/grub/pubkey.h
b32e65
index 4a9d04b43..fb8be9cbb 100644
80913e
--- a/include/grub/pubkey.h
80913e
+++ b/include/grub/pubkey.h
80913e
@@ -25,7 +25,7 @@ struct grub_public_key *
80913e
 grub_load_public_key (grub_file_t f);
80913e
 
80913e
 grub_err_t
80913e
-grub_verify_signature (grub_file_t f, grub_file_t sig,
80913e
+grub_verify_signature (grub_file_t f, const char *fsig,
80913e
 		       struct grub_public_key *pk);
80913e
 
80913e