Fix memory leak when updating keys

If the first new key had no new components we'd remove it from the list
but not actually free it. Fix things up so we free it properly. Found
with gcc's -fsanitize=leak option.

diff --git a/keydb.c b/keydb.c
index 1a882386bf25933c4ea4ae5fa47853301ce5bd6b..30a77caf61f912ddf5142aff5669b8d1be2844cc 100644 (file)
--- a/keydb.c
+++ b/keydb.c
@@ -170,16 +170,18 @@ struct ll *generic_cached_getkeysigs(struct onak_dbctx *dbctx, uint64_t keyid)
 int generic_update_keys(struct onak_dbctx *dbctx,
                struct openpgp_publickey **keys, bool sendsync)
 {
-       struct openpgp_publickey *curkey = NULL;
+       struct openpgp_publickey **curkey, *tmp = NULL;
        struct openpgp_publickey *oldkey = NULL;
-       struct openpgp_publickey *prev = NULL;
        struct openpgp_fingerprint fp;
        int newkeys = 0;
        bool intrans;
 
-       for (curkey = *keys; curkey != NULL; curkey = curkey->next) {
+       curkey = keys;
+       while (*curkey != NULL) {
+               get_fingerprint((*curkey)->publickey, &fp);
+
                intrans = dbctx->starttrans(dbctx);
-               get_fingerprint(curkey->publickey, &fp);
+
                logthing(LOGTHING_INFO,
                        "Fetching key, result: %d",
                        dbctx->fetch_key_fp(dbctx, &fp, &oldkey,
@@ -192,37 +194,34 @@ int generic_update_keys(struct onak_dbctx *dbctx,
                 * one that we send out.
                 */
                if (oldkey != NULL) {
-                       merge_keys(oldkey, curkey);
-                       if (curkey->sigs == NULL &&
-                                       curkey->uids == NULL &&
-                                       curkey->subkeys == NULL) {
-                               if (prev == NULL) {
-                                       *keys = curkey->next;
-                               } else {
-                                       prev->next = curkey->next;
-                                       curkey->next = NULL;
-                                       free_publickey(curkey);
-                                       curkey = prev;
-                               }
+                       merge_keys(oldkey, *curkey);
+                       if ((*curkey)->sigs == NULL &&
+                                       (*curkey)->uids == NULL &&
+                                       (*curkey)->subkeys == NULL) {
+                               tmp = *curkey;
+                               *curkey = (*curkey)->next;
+                               tmp->next = NULL;
+                               free_publickey(tmp);
                        } else {
-                               prev = curkey;
                                logthing(LOGTHING_INFO,
                                        "Merged key; storing updated key.");
                                dbctx->store_key(dbctx, oldkey, intrans,
                                        true);
+                               curkey = &(*curkey)->next;
                        }
                        free_publickey(oldkey);
                        oldkey = NULL;
                } else {
                        logthing(LOGTHING_INFO,
                                "Storing completely new key.");
-                       dbctx->store_key(dbctx, curkey, intrans, false);
+                       dbctx->store_key(dbctx, *curkey, intrans, false);
                        newkeys++;
+                       curkey = &(*curkey)->next;
                }
                dbctx->endtrans(dbctx);
        }
 
-       if (sendsync && keys != NULL) {
+       if (sendsync && keys != NULL && *keys != NULL) {
                sendkeysync(*keys);
        }
 

diff --git a/onak.c b/onak.c
index f4ece8f2150b2eb9f0795a271e7edf98ffc8c55f..4c319cd194c68ed6137f3f6715485347a642f25f 100644 (file)
--- a/onak.c
+++ b/onak.c
@@ -394,6 +394,8 @@ int main(int argc, char *argv[])
                                                        &fingerprint);
                                        dbctx->delete_key(dbctx, &fingerprint,
                                                        false);
+                                       free_publickey(keys);
+                                       keys = NULL;
                                }
                        } else
                                dbctx->delete_key(dbctx, &fingerprint, false);