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

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