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.
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.
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.
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.
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.
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.
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
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.
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.
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Stephen Gallagher