On 07.09.2011 14:22, Jiri Denemark wrote:
> On Wed, Sep 07, 2011 at 14:11:14 +0200, Michal Privoznik wrote:
>> Although we were initializing worker threads during pool creating,
>> we missed this during virThreadPoolSendJob. This bug led to segmenation
>> fault as worker thread free() given argument.
>> ---
>> src/util/threadpool.c | 13 ++++++++++++-
>> 1 files changed, 12 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/util/threadpool.c b/src/util/threadpool.c
>> index 70a75c0..6210b00 100644
>> --- a/src/util/threadpool.c
>> +++ b/src/util/threadpool.c
>> @@ -286,6 +286,7 @@ int virThreadPoolSendJob(virThreadPoolPtr pool,
>> void *jobData)
>> {
>> virThreadPoolJobPtr job;
>> + struct virThreadPoolWorkerData *data = NULL;
>>
>> virMutexLock(&pool->mutex);
>> if (pool->quit)
>> @@ -298,10 +299,19 @@ int virThreadPoolSendJob(virThreadPoolPtr pool,
>> goto error;
>> }
>>
>> + if (VIR_ALLOC(data) < 0) {
>> + pool->nWorkers--;
>> + virReportOOMError();
>> + goto error;
>> + }
>> +
>> + data->pool = pool;
>> + data->cond = &pool->cond;
>> +
>> if (virThreadCreate(&pool->workers[pool->nWorkers - 1],
>> true,
>> virThreadPoolWorker,
>> - pool) < 0) {
>> + data) < 0) {
>> pool->nWorkers--;
>
> You need to VIR_FREE(data) here...
>
>> goto error;
>> }
>> @@ -336,6 +346,7 @@ int virThreadPoolSendJob(virThreadPoolPtr pool,
>> return 0;
>>
>> error:
>> + VIR_FREE(data);
>
> ...instead of here because you can get to error label after the worker thread
> was successfully created (in case allocating job fails) so either data would
> be freed twice or the worker would read from memory that was already freed.
>
>> virMutexUnlock(&pool->mutex);
>> return -1;
>> }
>
> ACK with this fixed.
>
> Jirka
Thanks, pushed with that fixed.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list