[QUICK] talloc bugs

Sam Liddicott sam at liddicott.com
Tue Jun 30 09:24:32 MDT 2009


We got results:

* Sam Liddicott wrote, On 30/06/09 14:31:
> * tridge wrote, On 30/06/09 05:57:
>   
>>  > > First off I'd propose that for a test patch we abort() if you
>>  > > talloc_free() a pointer with a reference. Then we can run 'make test'
>>  > > and see just how often this happens. I think it will be very rare, and
>>  > > we may even find it doesn't happen at all outside of the talloc test
>>  > > suite.
>>  > >   
>>  > I'll do that (just for you).
>>
>> thanks! I'll be very interested in the results.
>>
>>   
>>     
>
> The attached diff causes no tests to be run:
>
> PROVISIONING DC...
> A summary with detailed information can be found in:
> /summary
>
> ALL OK (0 tests in 0 testsuites)
>
>
> Maybe I made what should be an obvious mistake, but I can't find it.
> I'm still looking to see what is going on.
>   
pre-test provision is failing running this command:

NSS_WRAPPER_PASSWD="/home/projects/samba-git/source4/st/dc/etc/passwd"
NSS_WRAPPER_GROUP="/home/projects/samba-git/source4/st/dc/etc/group"
/usr/bin/python2.4 ./setup/provision
--configfile=/home/projects/samba-git/source4/st/dc/etc/smb.conf
--host-name=localdc1 --host-ip=127.0.0.1 --quiet --domain=SAMBADOMAIN
--realm=SAMBA.EXAMPLE.COM --adminpass=localdcpass
--krbtgtpass=krbtgtlocaldcpass --machinepass=machinelocaldcpass
--root=root --server-role="domain controller"

the gdb backtrace of python is:
#0  0xffffe410 in __kernel_vsyscall ()
#1  0x00129ba0 in raise () from /lib/libc.so.6
#2  0x0012b4b1 in abort () from /lib/libc.so.6
#3  0xb5a8506e in talloc_abort () from bin/python/samba/dcerpc/security.so
#4  0xb5a86596 in talloc_free () from bin/python/samba/dcerpc/security.so
#5  0xb563610b in py_random_sid (self=0x0) at ./librpc/ndr/py_security.c:382
#6  0x0423b73a in PyEval_EvalFrame () from /usr/lib/libpython2.4.so.1.0
#7  0x0423cc68 in PyEval_EvalCodeEx () from /usr/lib/libpython2.4.so.1.0
#8  0x0423b426 in PyEval_EvalFrame () from /usr/lib/libpython2.4.so.1.0
#9  0x0423cc68 in PyEval_EvalCodeEx () from /usr/lib/libpython2.4.so.1.0
#10 0x0423ccf3 in PyEval_EvalCode () from /usr/lib/libpython2.4.so.1.0
#11 0x04259998 in Py_CompileString () from /usr/lib/libpython2.4.so.1.0
#12 0x0425b0a8 in PyRun_SimpleFileExFlags () from
/usr/lib/libpython2.4.so.1.0
#13 0x0425b78a in PyRun_AnyFileExFlags () from /usr/lib/libpython2.4.so.1.0
#14 0x04262185 in Py_Main () from /usr/lib/libpython2.4.so.1.0
#15 0x08048582 in main ()

referring to talloc_free(sid) in:

static PyObject *py_random_sid(PyObject *self)
{
        struct dom_sid *sid;
        PyObject *ret;
        char *str = talloc_asprintf(NULL, "S-1-5-21-%u-%u-%u",
                        (unsigned)generate_random(),
                        (unsigned)generate_random(),
                        (unsigned)generate_random());

        sid = dom_sid_parse_talloc(NULL, str);
        talloc_free(str);
        ret = py_talloc_import(&dom_sid_Type, sid);
        talloc_free(sid);
        return ret;
}

because py_talloc_import calls talloc_reference on sid.

So at the first pre-test setup we find that the hope: talloc_free just
"isn't" called when there is a reference
is a vain hope.


I suppose that in this specific case the author "should have known" that
py_talloc_import would take a reference and have "known" not to call
talloc_free but talloc_unreference or some long-winded variant instead,
but putting this restriction on the author seems madness. And what if
py_talloc_import (or some other function) only "maybe" will take a
reference? And what a broken API it would leave, with the caller having
to work out what free function to use, when talloc could just as easily
work it out for him.

So...

does this mean we can get back to considering that _talloc_free shall
not promote reference to owner?
There is no internal benefit to it doing so, and so nothing lost by it
not doing so.

If you like we can insist that the talloc parent is ALSO a managed
reference, and that then the parent field in the tc is just a
convenience for the simpler talloc API, for when it isn't worth the
effort of carrying around the owner in conjunction with the pointer so
that talloc_free can be called. (e.g. programs that are ported to
libtalloc).

Sam


More information about the samba-technical mailing list