[SSSD] On talloc_new(NULL) (ab)use in SSSD

Jakub Hrozek jhrozek at redhat.com
Thu Jan 10 14:57:33 UTC 2013


On Thu, Jan 10, 2013 at 09:16:18AM -0500, Stephen Gallagher wrote:
> On 01/07/2013 02:31 PM, Stephen Gallagher wrote:
> >On Mon 07 Jan 2013 02:19:49 PM EST, Jakub Hrozek wrote:
> >>On Mon, Jan 07, 2013 at 02:09:02PM -0500, Stephen Gallagher wrote:
> >>>>One concern I expressed to Simo on IRC is that we'd have to be more
> >>>>careful with long-lived requests such as resolving deep nested group
> >>>>memberships, otherwise the parent context will grow out of control.
> >>>>
> >>>>If SSS_FREE_MEM_CTX was used rigorously, then it would be much less of
> >>>>an issue. Maybe we could write a small GCC plugin to issue a warning
> >>>>for
> >>>>functions that call SSS_MEM_CTX but not SSS_FREE_MEM_CTX.
> >>>
> >>>I'm not really familiar with GCC plugins, but can we model that to
> >>>detect whether a branch decision could allow it to exit without
> >>>calling SSS_FREE_MEM_CTX? I think some of the static analysis tools
> >>>like Coverity can do this, but I don't know if GCC itself can.
> >>
> >>There is a plugin that tracks reference counts in Python plugins written
> >>in C:
> >>https://gcc-python-plugin.readthedocs.org/en/latest/cpychecker.html#reference-count-checking
> >>
> >>
> >>I haven't seen its source code so I can't comment on how easy/hard it
> >>is, but it seems doable.
> >
> >Looks like it was written by Dave Malcolm. I'll talk with him and see
> >what his thoughts are on that. Thanks for the pointer.
> 
> 
> Following up on this, I had a conversation with David Malcolm the
> other day. We identified that his latest project (a plugin system
> for adding new glibc warnings and errors).
> 
> We identified that it will actually be very easy to write a checker
> for proper talloc_new()/talloc_free() pairings within a function.
> We're planning on hacking on it together at FUDCon next week.
> 
> With this in place, we won't need to switch over to Simo's proposed
> new macros (we still can, over time if we want), because we'll get a
> compile-time error if we're ever forgetting to clean up a temp
> context at any exit path.
> 

That would be really awesome, thank you!

One other thing that might be checked is that if there is a
tevent_req_done() or tevent_req_error call, the function end or call
return after that. I remember a couple of bugs where we called
tevent_req_error() but carried on with the request which resulted in
access-after-free bugs.



More information about the sssd-devel mailing list