From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001

From: Michael Chang <mchang@suse.com>

Date: Tue, 20 Nov 2018 19:15:37 +0800

Subject: [PATCH] verifiers: fix double close on pgp's sig file descriptor

An error emerged as when I was testing the verifiers branch, so instead

of putting it in pgp prefix, the verifiers is used to reflect what the

patch is based on.

While running verify_detached, grub aborts with error.

verify_detached /@/.snapshots/1/snapshot/boot/grub/grub.cfg

/@/.snapshots/1/snapshot/boot/grub/grub.cfg.sig

alloc magic is broken at 0x7beea660: 0

Aborted. Press any key to exit.

The error is caused by sig file descriptor been closed twice, first time

in grub_verify_signature() to which it is passed as parameter. Second in

grub_cmd_verify_signature() or in whichever opens the sig file

descriptor. The second close is not consider as bug to me either, as in

common rule of what opens a file has to close it to avoid file

descriptor leakage.

After all the design of grub_verify_signature() makes it difficult to keep

a good trace on opened file descriptor from it's caller. Let's refine

the application interface to accept file path rather than descriptor, in

this way the caller doesn't have to care about closing the descriptor by

delegating it to grub_verify_signature() with full tracing to opened

file descriptor by itself.

Also making it clear that sig descriptor is not referenced in error

returning path of grub_verify_signature_init(), so it can be closed

directly by it's caller. This also makes delegating it to

grub_pubkey_close() infeasible to help in relieving file descriptor

leakage as it has to depend on uncertainty of ctxt fields in error

returning path.

Signed-off-by: Michael Chang <mchang@suse.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

---

 grub-core/commands/pgp.c | 35 +++++++++++++++++------------------

 include/grub/pubkey.h    |  2 +-

 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/grub-core/commands/pgp.c b/grub-core/commands/pgp.c

index 5c913c2e2fe..d39846d8cfe 100644

--- a/grub-core/commands/pgp.c

+++ b/grub-core/commands/pgp.c

@@ -495,13 +495,12 @@ grub_verify_signature_init (struct grub_pubkey_context *ctxt, grub_file_t sig)

   grub_dprintf ("crypt", "alive\n");

-  ctxt->sig = sig;

-

   ctxt->hash_context = grub_zalloc (ctxt->hash->contextsize);

   if (!ctxt->hash_context)

     return grub_errno;

   ctxt->hash->init (ctxt->hash_context);

+  ctxt->sig = sig;

   return GRUB_ERR_NONE;

 }

@@ -684,16 +683,26 @@ grub_pubkey_close (void *ctxt)

 }

 grub_err_t

-grub_verify_signature (grub_file_t f, grub_file_t sig,

+grub_verify_signature (grub_file_t f, const char *fsig,

 		       struct grub_public_key *pkey)

 {

+  grub_file_t sig;

   grub_err_t err;

   struct grub_pubkey_context ctxt;

   grub_uint8_t *readbuf = NULL;

+  sig = grub_file_open (fsig,

+			GRUB_FILE_TYPE_SIGNATURE

+			| GRUB_FILE_TYPE_NO_DECOMPRESS);

+  if (!sig)

+    return grub_errno;

+

   err = grub_verify_signature_init (&ctxt, sig);

   if (err)

-    return err;

+    {

+      grub_file_close (sig);

+      return err;

+    }

   readbuf = grub_zalloc (READBUF_SIZE);

   if (!readbuf)

@@ -807,7 +816,7 @@ static grub_err_t

 grub_cmd_verify_signature (grub_extcmd_context_t ctxt,

 			   int argc, char **args)

 {

-  grub_file_t f = NULL, sig = NULL;

+  grub_file_t f = NULL;

   grub_err_t err = GRUB_ERR_NONE;

   struct grub_public_key *pk = NULL;

@@ -845,19 +854,8 @@ grub_cmd_verify_signature (grub_extcmd_context_t ctxt,

       goto fail;

     }

-  sig = grub_file_open (args[1],

-			GRUB_FILE_TYPE_SIGNATURE

-			| GRUB_FILE_TYPE_NO_DECOMPRESS);

-  if (!sig)

-    {

-      err = grub_errno;

-      goto fail;

-    }

-

-  err = grub_verify_signature (f, sig, pk);

+  err = grub_verify_signature (f, args[1], pk);

  fail:

-  if (sig)

-    grub_file_close (sig);

   if (f)

     grub_file_close (f);

   if (pk)

@@ -902,7 +900,8 @@ grub_pubkey_init (grub_file_t io, enum grub_file_type type __attribute__ ((unuse

   err = grub_verify_signature_init (ctxt, sig);

   if (err)

     {

-      grub_pubkey_close (ctxt);

+      grub_free (ctxt);

+      grub_file_close (sig);

       return err;

     }

   *context = ctxt;

diff --git a/include/grub/pubkey.h b/include/grub/pubkey.h

index 4a9d04b4305..fb8be9cbb73 100644

--- a/include/grub/pubkey.h

+++ b/include/grub/pubkey.h

@@ -25,7 +25,7 @@ struct grub_public_key *

 grub_load_public_key (grub_file_t f);

 grub_err_t

-grub_verify_signature (grub_file_t f, grub_file_t sig,

+grub_verify_signature (grub_file_t f, const char *fsig,

 		       struct grub_public_key *pk);