147 lines
4.5 KiB
Diff
147 lines
4.5 KiB
Diff
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 5c913c2e2..d39846d8c 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 4a9d04b43..fb8be9cbb 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);
|
|
|
|
|