Re: read after free in notmuch new

Subject: Re: read after free in notmuch new

Date: Tue, 21 Feb 2017 21:44:29 -0400

To: Tomi Ollila,


From: David Bremner

David Bremner <> writes:

> Tomi Ollila <> writes:
>> To me it looks like replacing g_hash_table_insert() with 
>> g_hash_table_replace() would do the trick.
>> (or even g_hash_table_add()!)
>> One has to read the documentation a bit (and compare the docstrings of
>> these 2 functions to guess the missing pieces) to get some understanding to
>> this...
> Hi Tomi;
> Thanks for the suggestion. Unfortunately in my experiments it just
> shifts the invalid memory access to a different piece of memory. I think
> the problem is that a pointer to the previous copy of that key also
> leaked a reference via last_ref, so when we kill that via
> g_hash_table_replace it causes the same problem.

Thinking a bit more about it, at least in this case the pointer that was
just inserted remains valid after insertion, and can be
talloc_strdup'ed, i.e.

diff --git a/lib/ b/lib/
index f0bfe566..eddb780c 100644
--- a/lib/
+++ b/lib/
@@ -652,7 +652,7 @@ parse_references (void *ctx,
 	ref = _parse_message_id (ctx, refs, &refs);
 	if (ref && strcmp (ref, message_id)) {
-	    g_hash_table_insert (hash, ref, NULL);
+	    g_hash_table_add (hash, ref);
 	    last_ref = ref;
@@ -661,7 +661,7 @@ parse_references (void *ctx,
      * reference to the database.  We should avoid making a message
      * its own parent, thus the above check.
-    return last_ref;
+    return talloc_strdup(ctx, last_ref);

I'll let valgrind chew on that for a bit and see what it says.