[PATCH] lib: fix g_hash_table related read-after-free bug
Tomi Ollila
tomi.ollila at iki.fi
Wed Feb 22 03:25:26 PST 2017
On Wed, Feb 22 2017, David Bremner <david at tethera.net> wrote:
> The two g_hash_table functions (insert, add) have different behaviour
> with respect to existing keys. g_hash_table_insert frees the new key,
> while g_hash_table_add (which is really g_hash_table_replace in
> disguise) frees the existing key. With this change 'ref' is live until
> the end of the function (assuming single-threaded access to
> 'hash'). We can't guarantee it will continue to be live in the
> future (i.e. there may be a future key duplication) so we copy it with
> the allocation context passed to parse_references (in practice this is
> the notmuch_message_t object whose parents we are finding).
>
> Thanks to Tomi for the simpler approach to the problem based on
> reading the fine glib manual.
> ---
Great work! LGTM.
Tomi
PS: tried to look whethet there were talloc_ref() (the glib way)...
there is "refcounting" but a bit different (unsuitable) way...
>
> this at least passes the --medium memory test. I'll run the full one
> but it probably needs a day or so to complete.
>
> lib/database.cc | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index f0bfe566..eddb780c 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -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);
> }
>
> notmuch_status_t
> --
> 2.11.0
More information about the notmuch
mailing list