Page 1 of 1

[PATCH] xodtemplate: Fix SIGABRT from invalid pointer to free()

Posted: Thu Oct 18, 2007 10:53 am
by Guest
This is a multi-part message in MIME format.
--------------050506030001080505060907
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit

Andreas Ericsson wrote:
> Andreas Ericsson wrote:
>> Ethan Galstad wrote:
>>> Andreas Ericsson wrote:
>>>> Ahoy.
>>>>
>>>> I was digging around trying to fix a bunch of compiler-warnings, and
>>>> noticed that pretty much every invocation of my_free() resulted in
>>>> about a million warnings, such as this:
>>>>
>>>> ../xdata/xrddefault.c:190: warning: dereferencing type-punned
>>>> pointer will break strict-aliasing rules
>>>>
>>>>
>>>> Considering that not a single invocation of my_free() evaluates the
>>>> return code of the function, a macro such as
>>>>
>>>> #define my_free(ptr) { if(ptr) { free(ptr); ptr = NULL; } }
>>>>
>>>> would do the trick, and also provide a slight performance improvement.
>>>>
>>>> Since a patch would be fairly huge, and I've kinda filled my quota
>>>> for huge patches for today, the following sed-script will take care
>>>> of the call-sites (requires sed 4.0.9 or later):
>>>>
>>>> sed -i 's/my_free[^&]*&\([^)]*\).*/my_free(\1);/' */*.c
>>>>
>>> Definitely a good idea. I've committed this to CVS after some manual
>>> massaging post-sed.
>>>
>> Thanks.
>>
>>> Don't know why, but I get a SIGABRT if I don't comment out two my_free()
>>> statements in xdata/xodtemplate.c in the
>>> xodtemplate_get_inherited_string() function.
>>>
>> I'll look into it.
>>
>
> Hmm. I can't reproduce it, as I don't have a decent config with template
> inheritance using + concat stuff. Have you got anything readily made to
> share? I'll hack some up otherwise, but I'm not sure I'd get one that
> could trigger it.
>
> I'm fairly certain what's happening though. The caller passes the string
> by reference, but we're free()'ing it as if it wasn't, so we're basically
> calling free() on a pointer to the caller's stack frame. Bad thing.
> Change them from
>
> my_free(this_value);
>
> to
>
> my_free(*this_value);
>
> and things might turn out ok. Otherwise, tar up your config and send me
> what you've got and I'll see what I can do.
>

Got the config, verified the crash, and here's the patch.

--%
Date: Thu, 18 Oct 2007 20:48:12 +0200
Subject: [PATCH] xodtemplate: Fix SIGABRT from invalid pointer to free()

Signed-off-by: Andreas Ericsson
---
xdata/xodtemplate.c | 10 ++--------
1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/xdata/xodtemplate.c b/xdata/xodtemplate.c
index 94ef4cd..6594e4e 100644
--- a/xdata/xodtemplate.c
+++ b/xdata/xodtemplate.c
@@ -13390,10 +13390,7 @@ int xodtemplate_get_inherited_string(int *have_template_value, char **template_v
strcpy(buf,*template_value);
strcat(buf,",");
strcat(buf,*this_value+1);
-#ifdef WHY_DOES_THIS_CAUSE_A_SIGABRT
- /**** POTENTIAL MEMORY LEAK ****/
- my_free(this_value);
-#endif
+ my_free(*this_value);
*this_value=buf;
}
}
@@ -13410,10 +13407,7 @@ int xodtemplate_get_inherited_string(int *have_template_value, char **template_v
/* remove the additive symbol if present */
if(*this_value!=NULL && *this_value[0]=='+'){
buf=(char *)strdup(*this_value+1);
-#ifdef WHY_DOES_THIS_CAUSE_A_SIGABRT
- /**** POTENTIAL MEMORY LEAK ****/
- my_free(this_value);
-#endif
+ my_free(*this_value);
*this_value=buf;
}

--
1.5.3.4.1207.g6d77b


--
Andreas Ericsson [email protected]
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231

--------------050506030001080505060907
Content-Type: text/x-patch;
name="0001-xodtemplate-Fix-SIGABRT-from-invalid-pointer-to-fre.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename*0="0001-xodtemplate-Fix-SIGABRT-from-invalid-pointer-to-fre.pat";
filename*1="ch"

From 2586c7341b5f7c224ddf6dc81b148122c2b859dc Mon Sep 17 00:00:00 2001
From: Andreas Ericsson
Date: Thu, 18 Oct 2007 20:48:12 +0200
Subject: [PATCH] xodtemplate: Fix SIGABRT from invalid pointer to free()

Signed-off-by: Andreas Eri

...[email truncated]...


This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]