5:47 a.m.
The attached patch implements a fast in-memory cache for serving sudo
requests.
Most of the code was written by Pavel, so I retained the attribution
even though I made a couple of fixes and wrote the man page section.
https://fedorahosted.org/sssd/ticket/1111
8:06 p.m.
On Thu, 2012-02-02 at 12:47 +0100, Jakub Hrozek wrote:
The attached patch implements a fast in-memory cache for serving
sudo
requests.
Most of the code was written by Pavel, so I retained the attribution
even though I made a couple of fixes and wrote the man page section.
https://fedorahosted.org/sssd/ticket/1111
Nack.
You have a memory violation in sudosrv_cache_remove(). You're calling
talloc_free(cache_entry), but you're currently in the handler for an
event that's a child of cache_entry. You need to decouple the two
somehow (either by freeing cache_entry in a tevent_immediate handler
after this concludes or by talloc_stealing the timer event onto NULL
before freeing cache_entry. Sanity is not guaranteed otherwise!
Otherwise this patch looks good.
4:34 p.m.
On Thu, Feb 02, 2012 at 09:06:13PM -0500, Stephen Gallagher wrote:
On Thu, 2012-02-02 at 12:47 +0100, Jakub Hrozek wrote:
> The attached patch implements a fast in-memory cache for serving sudo
> requests.
>
> Most of the code was written by Pavel, so I retained the attribution
> even though I made a couple of fixes and wrote the man page section.
>
> https://fedorahosted.org/sssd/ticket/1111
Nack.
You have a memory violation in sudosrv_cache_remove(). You're calling
talloc_free(cache_entry), but you're currently in the handler for an
event that's a child of cache_entry. You need to decouple the two
somehow (either by freeing cache_entry in a tevent_immediate handler
after this concludes or by talloc_stealing the timer event onto NULL
before freeing cache_entry. Sanity is not guaranteed otherwise!
Otherwise this patch looks good.
Thank you, valgrind didn't catch this.
6:56 a.m.
On Fri, 2012-02-03 at 23:34 +0100, Jakub Hrozek wrote:
On Thu, Feb 02, 2012 at 09:06:13PM -0500, Stephen Gallagher wrote:
> On Thu, 2012-02-02 at 12:47 +0100, Jakub Hrozek wrote:
> > The attached patch implements a fast in-memory cache for serving sudo
> > requests.
> >
> > Most of the code was written by Pavel, so I retained the attribution
> > even though I made a couple of fixes and wrote the man page section.
> >
> > https://fedorahosted.org/sssd/ticket/1111
>
>
> Nack.
>
> You have a memory violation in sudosrv_cache_remove(). You're calling
> talloc_free(cache_entry), but you're currently in the handler for an
> event that's a child of cache_entry. You need to decouple the two
> somehow (either by freeing cache_entry in a tevent_immediate handler
> after this concludes or by talloc_stealing the timer event onto NULL
> before freeing cache_entry. Sanity is not guaranteed otherwise!
>
> Otherwise this patch looks good.
Thank you, valgrind didn't catch this.
Ack
7:33 a.m.
On Sat, 2012-02-04 at 07:56 -0500, Stephen Gallagher wrote:
On Fri, 2012-02-03 at 23:34 +0100, Jakub Hrozek wrote:
> On Thu, Feb 02, 2012 at 09:06:13PM -0500, Stephen Gallagher wrote:
> > On Thu, 2012-02-02 at 12:47 +0100, Jakub Hrozek wrote:
> > > The attached patch implements a fast in-memory cache for serving sudo
> > > requests.
> > >
> > > Most of the code was written by Pavel, so I retained the attribution
> > > even though I made a couple of fixes and wrote the man page section.
> > >
> > > https://fedorahosted.org/sssd/ticket/1111
> >
> >
> > Nack.
> >
> > You have a memory violation in sudosrv_cache_remove(). You're calling
> > talloc_free(cache_entry), but you're currently in the handler for an
> > event that's a child of cache_entry. You need to decouple the two
> > somehow (either by freeing cache_entry in a tevent_immediate handler
> > after this concludes or by talloc_stealing the timer event onto NULL
> > before freeing cache_entry. Sanity is not guaranteed otherwise!
> >
> > Otherwise this patch looks good.
>
> Thank you, valgrind didn't catch this.
Ack
Pushed to master.
4525
days inactive
4527
days old
sssd-devel@lists.fedorahosted.org
4 comments
2 participants
participants (2)
-
Jakub Hrozek -
Stephen Gallagher