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

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