s4:torture:smb2: fix a nasty double free error.

Stefan (metze) Metzmacher metze at samba.org
Fri Oct 28 00:40:32 MDT 2011


Am 28.10.2011 06:44, schrieb Andrew Bartlett:
> On Fri, 2011-10-28 at 02:38 +0200, Michael Adam wrote:
>> --- a/source4/torture/smb2/smb2.c
>> +++ b/source4/torture/smb2/smb2.c
>> @@ -30,17 +30,25 @@ static bool wrap_simple_1smb2_test(struct
>> torture_context *torture_ctx,
>>  {
>>         bool (*fn) (struct torture_context *, struct smb2_tree *);
>>         bool ret;
>> -
>>         struct smb2_tree *tree1;
>> +       TALLOC_CTX *mem_ctx = talloc_new(torture_ctx);
>>  
>>         if (!torture_smb2_connection(torture_ctx, &tree1))
>>                 return false;
>>  
>> +       /*
>> +        * This is a trick:
>> +        * The test might close the connection. If we steal the tree
>> context
>> +        * before that and free the parent instead of tree directly,
>> we avoid
>> +        * a double free error.
>> +        */
>> +       talloc_steal(mem_ctx, tree1);
>> +
>>         fn = test->fn;
>>  
>>         ret = fn(torture_ctx, tree1);
>>  
>> -       talloc_free(tree1);
>> +       talloc_free(mem_ctx);
>>  
>>         return ret;
>>  }
> 
> The other way to do this would be to initialise tree1 to:
> 
> talloc_unlink(torture_ctx, tree1)
> 
> That way, we only unlink tree1 that is a child of torture_ctx.  This is
> essentially what your patch does, as internally talloc always calls
> talloc_unlink(parent, child).

I don't understand that, if tree1 points to invalid memory,
we would still have problem. And with the parent free we can avoid the
impact
of talloc_reference(), also this code can't assume torture_ctx is the direct
parent of tree1.

metze

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20111028/85575b85/attachment.pgp>


More information about the samba-technical mailing list