After fixing the gpgme extract-keys crash, I put it on my list to take a closer look at the gpgme code.
Attached is a patch cleaning up some memory leaks in the code. I'll push it in a couple days, but wanted to give people a chance to look and comment if they see anything wrong. Thanks, -Kevin
# HG changeset patch
# User Kevin McCarthy <ke...@8t8.us>
# Date 1421534053 28800
# Sat Jan 17 14:34:13 2015 -0800
# Node ID 879741a49a02fd47529c89fcf4ca9db563c9fb52
# Parent d732298789f254b738701893c84446a02c181725
Fix some context, data, and key memory leaks in gpgme.
The context and data cleanup just involved adding missing
gpgme_release() and gpgme_data_release() calls in a few places.
The key usage was a little more involved:
* Fix crypt_free_key(). It wasn't freeing the key passed in, and
didn't work properly if multiple keys were passed in. It also was
missing a call to gpgme_key_unref().
* Add free_recipient_set() to properly unref all the keys before
freeing the array.
* Fix get_candidates() to ref keys added to the crypt_key_t list, and
unref the keys returned by gpgme_op_keylist_next().
* Change usages of gpgme_key_release() to gpgme_key_unref(). The former
was deprecated in gpgme version 0.4.1 (2003-06-06).
diff --git a/crypt-gpgme.c b/crypt-gpgme.c
--- a/crypt-gpgme.c
+++ b/crypt-gpgme.c
@@ -243,22 +243,29 @@
return k;
}
/* Release all the keys at the address of KEYLIST and set the address
to NULL. */
static void crypt_free_key (crypt_key_t **keylist)
{
+ crypt_key_t *k;
+
+ if (!keylist)
+ return;
+
while (*keylist)
- {
- crypt_key_t *k = (*keylist)->next;
- FREE (&k);
- *keylist = k;
- }
+ {
+ k = *keylist;
+ *keylist = (*keylist)->next;
+
+ gpgme_key_unref (k->kobj);
+ FREE (&k);
+ }
}
/* Return trute when key K is valid. */
static int crypt_key_is_valid (crypt_key_t *k)
{
if (k->flags & KEYFLAG_CANTUSE)
return 0;
return 1;
@@ -551,16 +558,38 @@
return NULL;
}
if (ret_fp)
*ret_fp = fp;
return safe_strdup (tempfile);
}
+static void free_recipient_set (gpgme_key_t **p_rset)
+{
+ gpgme_key_t *rset, k;
+
+ if (!p_rset)
+ return;
+
+ rset = *p_rset;
+ if (!rset)
+ return;
+
+ while (*rset)
+ {
+ k = *rset;
+ gpgme_key_unref (k);
+ rset++;
+ }
+
+ FREE (p_rset);
+}
+
+
/* Create a GpgmeRecipientSet from the keys in the string KEYLIST.
The keys must be space delimited. */
static gpgme_key_t *create_recipient_set (const char *keylist,
gpgme_protocol_t protocol)
{
int err;
const char *s;
char buf[100];
@@ -603,17 +632,19 @@
{
safe_realloc (&rset, sizeof (*rset) * (rset_n + 1));
rset[rset_n++] = key;
}
else
{
mutt_error (_("error adding recipient `%s': %s\n"),
buf, gpgme_strerror (err));
- FREE (&rset);
+ rset[rset_n] = NULL;
+ free_recipient_set (&rset);
+ gpgme_release (context);
return NULL;
}
}
} while (*s);
}
/* NULL terminate. */
safe_realloc (&rset, sizeof (*rset) * (rset_n + 1));
@@ -646,29 +677,29 @@
gpgme_release (listctx);
mutt_error (_("secret key `%s' not found: %s\n"),
signid, gpgme_strerror (err));
return -1;
}
err = gpgme_op_keylist_next (listctx, &key2);
if (!err)
{
- gpgme_key_release (key);
- gpgme_key_release (key2);
+ gpgme_key_unref (key);
+ gpgme_key_unref (key2);
gpgme_release (listctx);
mutt_error (_("ambiguous specification of secret key `%s'\n"),
signid);
return -1;
}
gpgme_op_keylist_end (listctx);
gpgme_release (listctx);
gpgme_signers_clear (ctx);
err = gpgme_signers_add (ctx, key);
- gpgme_key_release (key);
+ gpgme_key_unref (key);
if (err)
{
mutt_error (_("error setting secret key `%s': %s\n"),
signid, gpgme_strerror (err));
return -1;
}
return 0;
}
@@ -829,16 +860,17 @@
ctx = create_gpgme_context (use_smime);
if (!use_smime)
gpgme_set_armor (ctx, 1);
if (set_signer (ctx, use_smime))
{
gpgme_data_release (signature);
+ gpgme_data_release (message);
gpgme_release (ctx);
return NULL;
}
if (option (OPTCRYPTUSEPKA))
{
err = set_pka_sig_notation (ctx);
if (err)
@@ -957,23 +989,23 @@
if (!rset)
return NULL;
if (sign)
convert_to_7bit (a);
plaintext = body_to_data_object (a, 0);
if (!plaintext)
{
- FREE (&rset);
+ free_recipient_set (&rset);
return NULL;
}
outfile = encrypt_gpgme_object (plaintext, rset, 0, sign);
gpgme_data_release (plaintext);
- FREE (&rset);
+ free_recipient_set (&rset);
if (!outfile)
return NULL;
t = mutt_new_body ();
t->type = TYPEMULTIPART;
t->subtype = safe_strdup ("encrypted");
t->encoding = ENC7BIT;
t->use_disp = 0;
@@ -1016,23 +1048,23 @@
rset = create_recipient_set (keylist, GPGME_PROTOCOL_CMS);
if (!rset)
return NULL;
plaintext = body_to_data_object (a, 0);
if (!plaintext)
{
- FREE (&rset);
+ free_recipient_set (&rset);
return NULL;
}
outfile = encrypt_gpgme_object (plaintext, rset, 1, 0);
gpgme_data_release (plaintext);
- FREE (&rset);
+ free_recipient_set (&rset);
if (!outfile)
return NULL;
t = mutt_new_body ();
t->type = TYPEAPPLICATION;
t->subtype = safe_strdup ("pkcs7-mime");
mutt_set_parameter ("name", "smime.p7m", &t->parameter);
mutt_set_parameter ("smime-type", "enveloped-data", &t->parameter);
@@ -1338,17 +1370,17 @@
for (i = 0, sig = result->signatures; sig && (i < idx);
i++, sig = sig->next)
;
if (! sig)
return -1; /* Signature not found. */
if (signature_key)
{
- gpgme_key_release (signature_key);
+ gpgme_key_unref (signature_key);
signature_key = NULL;
}
fpr = sig->fpr;
sum = sig->summary;
if (gpg_err_code (sig->status) != GPG_ERR_NO_ERROR)
anybad = 1;
@@ -1416,17 +1448,17 @@
print_time (sig->exp_timestamp, s);
state_attach_puts ("\n", s);
}
show_sig_summary (sum, ctx, key, idx, s, sig);
anywarn = 1;
}
if (key != signature_key)
- gpgme_key_release (key);
+ gpgme_key_unref (key);
}
return anybad ? 1 : anywarn ? 2 : 0;
}
/* Do the actual verification step. With IS_SMIME set to true we
assume S/MIME (surprise!) */
static int verify_one (BODY *sigbdy, STATE *s,
@@ -1457,16 +1489,19 @@
ctx = create_gpgme_context (is_smime);
/* Note: We don't need a current time output because GPGME avoids
such an attack by separating the meta information from the
data. */
state_attach_puts (_("[-- Begin signature information --]\n"), s);
err = gpgme_op_verify (ctx, signature, message, NULL);
+ gpgme_data_release (message);
+ gpgme_data_release (signature);
+
mutt_need_hard_redraw ();
if (err)
{
char buf[200];
snprintf (buf, sizeof(buf)-1,
_("Error: verification failed: %s\n"),
gpgme_strerror (err));
@@ -1474,17 +1509,17 @@
}
else
{ /* Verification succeeded, see what the result is. */
int res, idx;
int anybad = 0;
if (signature_key)
{
- gpgme_key_release (signature_key);
+ gpgme_key_unref (signature_key);
signature_key = NULL;
}
for(idx=0; (res = show_one_sig_status (ctx, idx, s)) != -1; idx++)
{
if (res == 1)
anybad = 1;
else if (res == 2)
@@ -1962,17 +1997,17 @@
shortid, date, uid->uid);
else
fprintf (*fp, "%s %5.5s %d/%8s %s\n", more ? "sub" : "pub",
gpgme_pubkey_algo_name (subkey->pubkey_algo), subkey->length,
shortid, date);
subkey = subkey->next;
more = 1;
}
- gpgme_key_release (key);
+ gpgme_key_unref (key);
}
if (gpg_err_code (err) != GPG_ERR_EOF)
{
dprint (1, (debugfile, "Error listing keys\n"));
goto err_fp;
}
rc = 0;
@@ -2349,16 +2384,17 @@
state_attach_puts (_("Error: copy data failed\n"), s);
}
else
{
unlink (tmpfname);
FREE (&tmpfname);
}
}
+ gpgme_data_release (plaintext);
gpgme_release (ctx);
}
/*
* Now, copy cleartext to the screen. NOTE - we expect that PGP
* outputs utf-8 cleartext. This may not always be true, but it
* seems to be a reasonable guess.
*/
@@ -2400,16 +2436,17 @@
if (needpass)
state_attach_puts (_("[-- END PGP MESSAGE --]\n"), s);
else if (pgp_keyblock)
state_attach_puts (_("[-- END PGP PUBLIC KEY BLOCK --]\n"), s);
else
state_attach_puts (_("[-- END PGP SIGNED MESSAGE --]\n"), s);
}
+ gpgme_data_release (armored_data);
if (pgpout)
{
safe_fclose (&pgpout);
}
}
else
{
/* A traditional PGP part may mix signed and unsigned content */
@@ -3510,17 +3547,17 @@
gpgme_set_protocol (listctx, GPGME_PROTOCOL_CMS);
k = key->kobj;
gpgme_key_ref (k);
while ((s = k->chain_id) && k->subkeys && strcmp (s, k->subkeys->fpr) )
{
putc ('\n', fp);
err = gpgme_op_keylist_start (listctx, s, 0);
- gpgme_key_release (k);
+ gpgme_key_unref (k);
k = NULL;
if (!err)
err = gpgme_op_keylist_next (listctx, &k);
if (err)
{
fprintf (fp, _("Error finding issuer key: %s\n"),
gpgme_strerror (err));
goto leave;
@@ -3533,17 +3570,17 @@
putc ('\n', fp);
fputs (_("Error: certification chain to long - stopping here\n"),
fp);
break;
}
}
leave:
- gpgme_key_release (k);
+ gpgme_key_unref (k);
gpgme_release (listctx);
safe_fclose (&fp);
mutt_clear_error ();
snprintf (cmd, sizeof (cmd), _("Key ID: 0x%s"), crypt_keyid (key));
mutt_do_pager (cmd, tempfile, 0, NULL);
}
/*
@@ -3701,22 +3738,24 @@
}
}
#endif /* DISABLED code */
for (idx = 0, uid = key->uids; uid; idx++, uid = uid->next)
{
k = safe_calloc (1, sizeof *k);
k->kobj = key;
+ gpgme_key_ref (k->kobj);
k->idx = idx;
k->uid = uid->uid;
k->flags = flags;
*kend = k;
kend = &k->next;
}
+ gpgme_key_unref (key);
}
if (gpg_err_code (err) != GPG_ERR_EOF)
mutt_error (_("gpgme_op_keylist_next failed: %s"), gpgme_strerror
(err));
gpgme_op_keylist_end (ctx);
no_pgphints:
;
}
@@ -3742,22 +3781,24 @@
flags |= KEYFLAG_CANENCRYPT;
if (key_check_cap (key, KEY_CAP_CAN_SIGN))
flags |= KEYFLAG_CANSIGN;
for (idx = 0, uid = key->uids; uid; idx++, uid = uid->next)
{
k = safe_calloc (1, sizeof *k);
k->kobj = key;
+ gpgme_key_ref (k->kobj);
k->idx = idx;
k->uid = uid->uid;
k->flags = flags;
*kend = k;
kend = &k->next;
}
+ gpgme_key_unref (key);
}
if (gpg_err_code (err) != GPG_ERR_EOF)
mutt_error (_("gpgme_op_keylist_next failed: %s"), gpgme_strerror
(err));
gpgme_op_keylist_end (ctx);
}
gpgme_release (ctx);
FREE (&pattern);
@@ -4591,17 +4632,17 @@
else
mutt_any_key_to_continue (_("Failed to verify sender"));
}
else
mutt_any_key_to_continue (_("Failed to figure out sender"));
if (signature_key)
{
- gpgme_key_release (signature_key);
+ gpgme_key_unref (signature_key);
signature_key = NULL;
}
return ret;
}
int smime_gpgme_verify_sender (HEADER *h)
{
signature.asc
Description: PGP signature
