teddy_bear
Joined: 04 Feb 2009 Posts: 4
|
Posted: Wed Sep 30, 2009 2:57 pm Post subject: miniupnpd 20090921: bug in calling iptc_free() |
|
|
There's a bug in the latest changes in version 20090921 that may cause miniupnpd to die or segfault when adding or deleting redirect rules.
The problem is that iptc_free() is called without checking the argument for NULL, but iptc_commit() also calls iptc_free() internally (at least in iptables 1.3.x - not sure about 1.4.x), so the handle could already be NULL.
Here's the proposed patch against 20090921. In addition to NULL checks in a few places, I moved iptc_free inside delete_rule_and_commit since after that call the handle can already be disposed anyway, so logically it should be there, and added one more missed iptc_free call to list_redirect_rule:
Code: | Date: Wed Sep 30 01:05:51 2009 -0400
Fix handle deallocation after iptc_commit() calls, clean-up
diff --git a/release/src/router/miniupnpd/netfilter/iptcrdr.c b/release/src/router/miniupnpd/netfilter/iptcrdr.c
index 3bf61d2..21e01a7 100644
--- a/release/src/router/miniupnpd/netfilter/iptcrdr.c
+++ b/release/src/router/miniupnpd/netfilter/iptcrdr.c
@@ -340,6 +340,12 @@ delete_rule_and_commit(unsigned int index, IPTC_HANDLE h,
logcaller, iptc_strerror(errno));
r = -1;
}
+ if (h)
+#ifdef IPTABLES_143
+ iptc_free(h);
+#else
+ iptc_free(&h);
+#endif
return r;
}
@@ -415,21 +421,10 @@ delete_redirect_and_filter_rules(unsigned short eport, int proto)
if(h)
{
r = delete_rule_and_commit(index, h, miniupnpd_nat_chain, "delete_redirect_rule");
-#ifdef IPTABLES_143
- iptc_free(h);
-#else
- iptc_free(&h);
-#endif
}
- h = iptc_init("filter");
- if(h && (r == 0))
+ if(r == 0 && (h = iptc_init("filter")))
{
r = delete_rule_and_commit(index, h, miniupnpd_forward_chain, "delete_filter_rule");
-#ifdef IPTABLES_143
- iptc_free(h);
-#else
- iptc_free(&h);
-#endif
}
}
del_redirect_desc(eport, proto);
@@ -534,6 +529,7 @@ iptc_init_verify_and_append(const char * table, const char * miniupnpd_chain, st
{
syslog(LOG_ERR, "%s : iptc_append_entry() error : %s\n",
logcaller, iptc_strerror(errno));
+ if (h)
#ifdef IPTABLES_143
iptc_free(h);
#else
@@ -549,6 +545,7 @@ iptc_init_verify_and_append(const char * table, const char * miniupnpd_chain, st
{
syslog(LOG_ERR, "%s : iptc_commit() error : %s\n",
logcaller, iptc_strerror(errno));
+ if (h)
#ifdef IPTABLES_143
iptc_free(h);
#else
@@ -556,6 +553,7 @@ iptc_init_verify_and_append(const char * table, const char * miniupnpd_chain, st
#endif
return -1;
}
+ if (h)
#ifdef IPTABLES_143
iptc_free(h);
#else
@@ -738,6 +736,11 @@ list_redirect_rule(const char * ifname)
if(!iptc_is_chain(miniupnpd_nat_chain, h))
{
printf("chain %s not found\n", miniupnpd_nat_chain);
+#ifdef IPTABLES_143
+ iptc_free(h);
+#else
+ iptc_free(&h);
+#endif
return -1;
}
#ifdef IPTABLES_143 |
|
|