On Thu, Mar 20, 2014 at 05:53:31PM +0100, Lukas Slebodnik wrote:
On (20/03/14 17:21), Jakub Hrozek wrote:
>On Thu, Mar 20, 2014 at 05:00:00PM +0100, Sumit Bose wrote:
>> On Thu, Mar 20, 2014 at 04:20:59PM +0100, Lukas Slebodnik wrote:
>> > ehlo,
>> >
>> > debug_prg_name is used in debug_fn and it was allocated under
>> > talloc context "kr". The variable "kr" was removed
before the last debug
>> > messages in function main. It is very little change that it will be
overridden.
>> >
>> > It is possible to see this issue with exported environment variable
>> > TALLOC_FREE_FILL=255
>>
>> I'm pretty sure the patch works as expected and fixes the isssue. But I
>> wonder if a different approach might be better? I think it does not make
>> sense to allocate debug_prg_name on a given talloc context but that it
>> would be better to just allocate it on NULL. This is e.g. done in the
>> ldap_child (here a talloc_free() is missing on exit but this would be a
>> different story). Then debug_prg_name can even be allocate before kr
>> and the debug messages for a failed allocation of kr can use the right
>> program name and not "sssd".
>
>I agree, because also given that krb5_child is a short lived process,
>we don't care too much about possible leaks.
No problem.
New version attached.
LS
This version works for me, do you want to also add a talloc_free() on
exit to be clean or do you also consider this not needed for short-lived
processes?