On Thu, Jul 12, 2012 at 09:26:30AM +0000, BRESTAN Rainer wrote:
> Using drbdadm with environment variable MALLOC_CHECK_ set to not zero result
> in segmentation fault.
>
> Reason is the handling of most free operations in drbdadm_main.c, notably in
> functions free_config, free_host_info and maybe some other more.
>
> Example: function free_host_info
> for_each_volume(vol, hi->volumes)
> free_volume(vol);
>
> Function free_volume call as last libc function free(vol), which
> instructs libc in case of MALLOC_CHECK_ not zero to initialize memory
> with 0xcc. The macro for_each_volume take vol->next (which is now
> 0xcccccccc) as next vol and calls free_volume again which try to free
> the memory and this cause the segmentation fault.
>
> Example of correct behaviour is function free_options, which temporary stores
> the next element before freeing it.
>
> There is currently no race condition in free before next, because this part
> is not multi threaded.
>
> Questions:
> Is there a specific reason for doing that (DRBD 8.3.11 works fine with
> MALLOC_CHECK_) in DRBD 8.4.1 ?
> Is it planned or a bug ?
We certainly do not plan to segfault ;-)
> Patch ?
Won't be necessary.
diff --git a/user/drbdadm.h b/user/drbdadm.h
index 454fcf9..c3554d8 100644
--- a/user/drbdadm.h
+++ b/user/drbdadm.h
@@ -334,6 +334,9 @@ extern void add_setup_option(bool explicit, char *option);
#define for_each_volume(v_,volumes_) \
for (v_ = volumes_; v_; v_ = v_->next)
+#define for_each_volume_safe(v_,tmp_,volumes_) \
+ for (v_ = volumes_; v_ && ({ tmp_ = v_->next; 1; }); v_ = tmp_)
+
#define APPEND(LIST,ITEM) ({ \
typeof((LIST)) _l = (LIST); \
typeof((ITEM)) _i = (ITEM); \
diff --git a/user/drbdadm_main.c b/user/drbdadm_main.c
index 33d9d28..ace37a7 100644
--- a/user/drbdadm_main.c
+++ b/user/drbdadm_main.c
@@ -1235,13 +1235,13 @@ static void free_volume(struct d_volume *vol)
static void free_host_info(struct d_host_info *hi)
{
- struct d_volume *vol;
+ struct d_volume *vol, *tmp;
if (!hi)
return;
free_names(hi->on_hosts);
- for_each_volume(vol, hi->volumes)
+ for_each_volume_safe(vol, tmp, hi->volumes)
free_volume(vol);
free(hi->address);
free(hi->address_family);
Any more segfaults to report?
Thanks,
--
: Lars Ellenberg
: LINBIT | Your Way to High Availability
: DRBD/HA support and consulting http://www.linbit.com
_______________________________________________
drbd-user mailing list
[email protected]
http://lists.linbit.com/mailman/listinfo/drbd-user
