From: Jonathan McDowell <noodles@earth.li>
Date: Sun, 18 Aug 2019 18:30:06 +0000 (+0100)
Subject: Fix memory leak when updating keys
X-Git-Tag: onak-0.6.0~31
X-Git-Url: https://the.earth.li/gitweb/?a=commitdiff_plain;h=f869498b2b159bd3d363fb2f9d803b99c44de8bc;p=onak.git

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 1a88238..30a77ca 100644
--- 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 f4ece8f..4c319cd 100644
--- 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);