On 03/12/2013 10:45 AM, Daniel Wagner wrote:
From: Daniel Wagner <daniel.wag...@bmw-carit.de>
After removing one or more rules the builtin hooks need to be updated
accordingly. iptables_flush_chain() and iptables_delete_rule()
share a common code part.
---
src/iptables.c | 66 ++++++++++++++++++++++++----------------------------------
1 file changed, 27 insertions(+), 39 deletions(-)
diff --git a/src/iptables.c b/src/iptables.c
index 5e24efb..96a575c 100644
--- a/src/iptables.c
+++ b/src/iptables.c
@@ -515,6 +515,29 @@ static int remove_table_entry(struct connman_iptables
*table,
return removed;
}
+static void delete_update_hooks(struct connman_iptables *table,
+ int builtin, GList *chain_head,
+ int removed)
+{
+ struct connman_iptables_entry *e;
+ GList *list;
+
+ e = chain_head->data;
+ e->builtin = builtin;
+
+ table->underflow[builtin] -= removed;
+
+ for (list = chain_head->next; list; list = list->next) {
+ e = list->data;
+
+ if (e->builtin < 0)
+ continue;
+
+ table->hook_entry[e->builtin] -= removed;
+ table->underflow[e->builtin] -= removed;
+ }
+}
+
static int iptables_flush_chain(struct connman_iptables *table,
const char *name)
{
@@ -552,26 +575,8 @@ static int iptables_flush_chain(struct connman_iptables
*table,
list = next;
}
- if (builtin >= 0) {
- struct connman_iptables_entry *e;
-
- entry = list->data;
-
- entry->builtin = builtin;
-
- table->underflow[builtin] -= removed;
-
- for (list = chain_tail; list; list = list->next) {
- e = list->data;
-
- builtin = e->builtin;
- if (builtin < 0)
- continue;
-
- table->hook_entry[builtin] -= removed;
- table->underflow[builtin] -= removed;
- }
- }
+ if (builtin >= 0)
+ delete_update_hooks(table, builtin, chain_head, removed);
update_offsets(table);
@@ -1034,25 +1039,8 @@ static int iptables_delete_rule(struct connman_iptables
*table,
removed += remove_table_entry(table, entry);
- if (builtin >= 0) {
- list = list->next;
- if (list) {
- entry = list->data;
- entry->builtin = builtin;
- }
We have here hidden bug. If we remove the first rule in the table then
list == chain_head, and we just removed that entry with
remove_table_entry(). That means the pointer 'list' has been modified in
remove_table_entry.
-
- table->underflow[builtin] -= removed;
- for (list = chain_tail; list; list = list->next) {
- entry = list->data;
-
- builtin = entry->builtin;
- if (builtin < 0)
- continue;
-
- table->hook_entry[builtin] -= removed;
- table->underflow[builtin] -= removed;
- }
- }
+ if (builtin >= 0)
+ delete_update_hooks(table, builtin, chain_head, removed);
And here I do the same mistake be assuming chain_head is still valid. An
updated version is on the way.
_______________________________________________
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
