Re: [Gluster-devel] [PATCH] [master] glusterd: fix segfault on volume status detail

[Lars Ellenberg]

On Wed, Mar 06, 2013 at 12:19:14PM +0100, Lars Ellenberg wrote:
> From: Lars Ellenberg <address@hidden>
> 
> If for some reason glusterd_get_brick_root() fails,
> it frees the gf_strdup'ed *mount_point in its own error path,
> and returns -1.
> 
> Unfortunately it already had assigned that pointer value
> to the output argument, the caller function
> glusterd_add_brick_detail() sees a non-NULL pointer,
> and free() again: segfault.
> 
> Could be fixed with a one-liner (*mount_point = NULL)
> in the error path, but I think glusterd_get_brick_root()
> should only assign to the output argument once all checks passed,
> so I use a local temporary pointer, which increases the patch a bit.
> 
> Signed-off-by: Lars Ellenberg <address@hidden>

https://bugzilla.redhat.com/show_bug.cgi?id=919352
http://review.gluster.org/#change,4646


You may want to consider to add a "double free" check,
you already have (at least two flavors of) instrumented free(),
depending on ->mem_acct_enable...
Below is what I mean.

You need to flesh out the
do_something_useful_log_loud_and_screaming_but_not_segfault()
part, still ;-)

Thanks,
        Lars

--- libglusterfs/src/mem-pool.h 2013-03-08 10:26:55.902715232 +0100
***************
*** 97,103 ****

  #define FREE(ptr)                               \
!         if (ptr != NULL) {                      \
                  free ((void *)ptr);             \
!                 ptr = (void *)0xeeeeeeee;       \
!         }

--- 97,108 ----

+ #define DANGLING_PTR_MAGIC    ((void*)0xeeeeeeee)
  #define FREE(ptr)                               \
! do {                                            \
!         if (ptr == DANGLING_PTR_MAGIC) {        \
!                 
do_something_useful_log_loud_and_screaming_but_not_segfault();  \
!         } else if (ptr != NULL) {               \
                  free ((void *)ptr);             \
!                 ptr = DANGLING_PTR_MAGIC;       \
!         }                                       \
! } while (0)