[syslog-ng] [PATCH 4/7] [affile]: Fix a possible double free

Gergely Nagy algernon at balabit.hu
Sat May 28 14:33:49 CEST 2011


>> From: Juhasz Viktor <jviktor at balabit.hu>
>> 
>> Duplicate the filename we insert into self->writer_hash, because
>> otherwise, we're likely to end up trying to free it twice.
>> 
>> Signed-off-by: Viktor Juhasz <jviktor at balabit.hu>
>> ---
>>  modules/affile/affile.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/modules/affile/affile.c b/modules/affile/affile.c
>> index c5e89f5..f6c70e9 100644
>> --- a/modules/affile/affile.c
>> +++ b/modules/affile/affile.c
>> @@ -1046,7 +1046,7 @@ affile_dd_open_writer(gpointer args[])
>>  	    {
>>  	      log_pipe_ref(&next->super);
>>  	      g_static_mutex_lock(&self->lock);
>> -              g_hash_table_insert(self->writer_hash, filename->str, next);
>> +              g_hash_table_insert(self->writer_hash, strdup(filename->str), next);
>>                g_static_mutex_unlock(&self->lock);
>>              }
>>  	}
>
> I don't understand. The writer_hash hashtable contains a borrowed key,
> nothing frees it there. Isn't it possible that the last patch (which
> changed AFFileDestWriter->filename allocation) fixed this issue too?

That's very possible, indeed. After having a closer look, I believe that
the other patch fixed this issue aswell.

> And even if the strdup was necessary, g_strdup() should have been used,
> as the g_malloc() style allocations may not map directly to malloc() on
> some platforms.

Yikes, my bad! I didn't spot it's not g_strdup().

-- 
|8]


More information about the syslog-ng mailing list