iptables: Fix setting policy

[platform/upstream/connman.git] / src / iptables.c

diff --git a/src/iptables.c b/src/iptables.c
index c5776b1..158dcf3 100644 (file)
--- a/src/iptables.c
+++ b/src/iptables.c
@@ -182,48 +182,65 @@ typedef int (*iterate_entries_cb_t)(struct ipt_entry *entry, int builtin,
                                        unsigned int hook,size_t size,
                                        unsigned int offset, void *user_data);
 
+static unsigned int next_hook_entry_index(unsigned int *valid_hooks)
+{
+       unsigned int h;
+
+       if (*valid_hooks == 0)
+               return NF_INET_NUMHOOKS;
+
+       h = __builtin_ffs(*valid_hooks) - 1;
+       *valid_hooks ^= (1 << h);
+
+       return h;
+}
+
 static int iterate_entries(struct ipt_entry *entries,
                                unsigned int valid_hooks,
                                unsigned int *hook_entry,
+                               unsigned int *underflow,
                                size_t size, iterate_entries_cb_t cb,
                                void *user_data)
 {
-       unsigned int i, h;
+       unsigned int offset, h, hook;
        int builtin, err;
        struct ipt_entry *entry;
 
-       if (valid_hooks != 0)
-               h = __builtin_ffs(valid_hooks) - 1;
-       else
-               h = NF_INET_NUMHOOKS;
+       h = next_hook_entry_index(&valid_hooks);
+       hook = h;
 
-       for (i = 0, entry = entries; i < size;
-                       i += entry->next_offset) {
+       for (offset = 0, entry = entries; offset < size;
+                       offset += entry->next_offset) {
                builtin = -1;
-               entry = (void *)entries + i;
+               entry = (void *)entries + offset;
 
                /*
-                * Find next valid hook which offset is higher
-                * or equal with the current offset.
+                * Updating builtin, hook and h is very tricky.
+                * The rules are:
+                * - builtin is only set to the current hook number
+                *   if the current entry is the hook entry (aka chain
+                *   head). And only for builtin chains, never for
+                *   the user chains.
+                * - hook is the current hook number. If we
+                *   look at user chains it needs to be NF_INET_NETNUMHOOKS.
+                * - h is the next hook entry. Thous we need to be carefully
+                *   not to access the table when h is NF_INET_NETNUMHOOKS.
                 */
-               if (h < NF_INET_NUMHOOKS) {
-                       if (hook_entry[h] < i) {
-                               valid_hooks ^= (1 << h);
-
-                               if (valid_hooks != 0)
-                                       h = __builtin_ffs(valid_hooks) - 1;
-                               else
-                                       h = NF_INET_NUMHOOKS;
-                       }
+               if (h < NF_INET_NUMHOOKS && hook_entry[h] == offset) {
+                       builtin = h;
+                       hook = h;
+               }
 
-                       if (hook_entry[h] == i)
-                               builtin = h;
+               if (h == NF_INET_NUMHOOKS)
+                       hook = h;
+
+               if (h < NF_INET_NUMHOOKS && underflow[h] <= offset) {
+                       h = next_hook_entry_index(&valid_hooks);
                }
 
-               err = cb(entry, builtin, h, size, i, user_data);
+               err = cb(entry, builtin, hook, size, offset, user_data);
                if (err < 0)
                        return err;
-
        }
 
        return 0;
@@ -301,9 +318,13 @@ static gboolean is_fallthrough(struct connman_iptables_entry *e)
        struct xt_entry_target *target;
 
        target = ipt_get_target(e->entry);
-       if (!strcmp(target->u.user.name, ""))
-               return true;
+       if (!g_strcmp0(target->u.user.name, IPT_STANDARD_TARGET)) {
+               struct xt_standard_target *t;
 
+               t = (struct xt_standard_target *)target;
+               if (t->verdict == 0)
+                       return true;
+       }
        return false;
 }
 
@@ -1033,7 +1054,7 @@ static int iptables_delete_rule(struct connman_iptables *table,
 static int iptables_change_policy(struct connman_iptables *table,
                                const char *chain_name, const char *policy)
 {
-       GList *chain_head;
+       GList *chain_head, *chain_tail;
        struct connman_iptables_entry *entry;
        struct xt_entry_target *target;
        struct xt_standard_target *t;
@@ -1051,6 +1072,11 @@ static int iptables_change_policy(struct connman_iptables *table,
        if (entry->builtin < 0)
                return -EINVAL;
 
+       chain_tail = find_chain_tail(table, chain_name);
+       if (chain_tail == NULL)
+               return -EINVAL;
+
+       entry = chain_tail->prev->data;
        target = ipt_get_target(entry->entry);
 
        t = (struct xt_standard_target *)target;
@@ -1269,6 +1295,7 @@ static void dump_table(struct connman_iptables *table)
        iterate_entries(table->blob_entries->entrytable,
                        table->info->valid_hooks,
                        table->info->hook_entry,
+                       table->info->underflow,
                        table->blob_entries->size,
                        print_entry, dump_entry);
 }
@@ -1293,7 +1320,8 @@ static void dump_ipt_replace(struct ipt_replace *repl)
                repl->underflow[NF_IP_POST_ROUTING]);
 
        iterate_entries(repl->entries, repl->valid_hooks,
-                       repl->hook_entry, repl->size, print_entry, dump_entry);
+                       repl->hook_entry, repl->underflow,
+                       repl->size, print_entry, dump_entry);
 }
 
 static int iptables_get_entries(struct connman_iptables *table)
@@ -1422,7 +1450,8 @@ static struct connman_iptables *iptables_init(const char *table_name)
 
        iterate_entries(table->blob_entries->entrytable,
                        table->info->valid_hooks, table->info->hook_entry,
-                       table->blob_entries->size, add_entry, table);
+                       table->info->underflow, table->blob_entries->size,
+                       add_entry, table);
 
        g_hash_table_insert(table_hash, g_strdup(table_name), table);
 
@@ -1576,9 +1605,6 @@ static struct xtables_match *prepare_matches(struct connman_iptables *table,
        if (xt_m->init != NULL)
                xt_m->init(xt_m->m);
 
-       if (xt_m == xt_m->next)
-               goto done;
-
 #if XTABLES_VERSION_CODE > 5
        if (xt_m->x6_options != NULL)
                iptables_globals.opts =
@@ -1603,7 +1629,6 @@ static struct xtables_match *prepare_matches(struct connman_iptables *table,
                xt_m = NULL;
        }
 
-done:
        return xt_m;
 }
 
@@ -2282,9 +2307,18 @@ void flush_table(const char *name)
        iterate_entries(table->blob_entries->entrytable,
                        table->info->valid_hooks,
                        table->info->hook_entry,
+                       table->info->underflow,
                        table->blob_entries->size,
                        flush_table_cb, &chains);
 
+
+       /*
+        * The offset update code is fragile and it works
+        * only safe if we remove elements and move forwards
+        * in the table.
+        */
+       chains = g_slist_reverse(chains);
+
        for (list = chains; list != NULL; list = list->next) {
                char *chain = list->data;