Dmitry Mishin wrote:
> On Saturday 02 June 2007 16:54, Patrick McHardy wrote:
> 
>>Here it is, could you please test whether it fixes the crash by
>>backing out the hashlimit compat patch and triggering the size
>>error again? Thanks.
> 
> Patrick, 
> it looks like translate_compat_table() should be fixed also.


You're right, thanks for catching this. This patch should be
better.

[NETFILTER]: ip_tables: fix compat related crash

check_compat_entry_size_and_hooks iterates over the matches and calls
compat_check_calc_match, which loads the match and calculates the
compat offsets, but unlike the non-compat version, doesn't call
->checkentry yet. On error however it calls cleanup_matches, which in
turn calls ->destroy, which can result in crashes if the destroy
function (validly) expects to only get called after the checkentry
function.

Add a compat_release_match function that only drops the module reference
on error and rename compat_check_calc_match to compat_find_calc_match to
reflect the fact that it doesn't call the checkentry function.

Reported by Jan Engelhardt <[EMAIL PROTECTED]>

Signed-off-by: Patrick McHardy <[EMAIL PROTECTED]>

---
commit b2b15a77343e2baadee22a5e64f691732874b10b
tree 7d82cf0a9fd3cf77933205a7a1834e958293e293
parent e7d815ef75f70dcdf55001f1f88ae7ae8827a7ba
author Patrick McHardy <[EMAIL PROTECTED]> Sun, 03 Jun 2007 18:57:04 +0200
committer Patrick McHardy <[EMAIL PROTECTED]> Sun, 03 Jun 2007 18:57:04 +0200

 net/ipv4/netfilter/ip_tables.c |   41 ++++++++++++++++++++++++++++++++--------
 1 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index e3f83bf..0a35639 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -1425,7 +1425,7 @@ out:
 }
 
 static inline int
-compat_check_calc_match(struct ipt_entry_match *m,
+compat_find_calc_match(struct ipt_entry_match *m,
            const char *name,
            const struct ipt_ip *ip,
            unsigned int hookmask,
@@ -1449,6 +1449,31 @@ compat_check_calc_match(struct ipt_entry_match *m,
 }
 
 static inline int
+compat_release_match(struct ipt_entry_match *m, unsigned int *i)
+{
+       if (i && (*i)-- == 0)
+               return 1;
+
+       module_put(m->u.kernel.match->me);
+       return 0;
+}
+
+static inline int
+compat_release_entry(struct ipt_entry *e, unsigned int *i)
+{
+       struct ipt_entry_target *t;
+
+       if (i && (*i)-- == 0)
+               return 1;
+
+       /* Cleanup all matches */
+       IPT_MATCH_ITERATE(e, compat_release_match, NULL);
+       t = ipt_get_target(e);
+       module_put(t->u.kernel.target->me);
+       return 0;
+}
+
+static inline int
 check_compat_entry_size_and_hooks(struct ipt_entry *e,
                           struct xt_table_info *newinfo,
                           unsigned int *size,
@@ -1485,10 +1510,10 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
        off = 0;
        entry_offset = (void *)e - (void *)base;
        j = 0;
-       ret = IPT_MATCH_ITERATE(e, compat_check_calc_match, name, &e->ip,
+       ret = IPT_MATCH_ITERATE(e, compat_find_calc_match, name, &e->ip,
                        e->comefrom, &off, &j);
        if (ret != 0)
-               goto cleanup_matches;
+               goto release_matches;
 
        t = ipt_get_target(e);
        target = try_then_request_module(xt_find_target(AF_INET,
@@ -1499,7 +1524,7 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
                duprintf("check_compat_entry_size_and_hooks: `%s' not found\n",
                                                        t->u.user.name);
                ret = target ? PTR_ERR(target) : -ENOENT;
-               goto cleanup_matches;
+               goto release_matches;
        }
        t->u.kernel.target = target;
 
@@ -1526,8 +1551,8 @@ check_compat_entry_size_and_hooks(struct ipt_entry *e,
 
 out:
        module_put(t->u.kernel.target->me);
-cleanup_matches:
-       IPT_MATCH_ITERATE(e, cleanup_match, &j);
+release_matches:
+       IPT_MATCH_ITERATE(e, compat_release_match, &j);
        return ret;
 }
 
@@ -1690,13 +1715,13 @@ translate_compat_table(const char *name,
 
 free_newinfo:
        xt_free_table_info(newinfo);
-out:
        IPT_ENTRY_ITERATE(entry0, total_size, cleanup_entry, &j);
        return ret;
 out_unlock:
        compat_flush_offsets();
        xt_compat_unlock(AF_INET);
-       goto out;
+       IPT_ENTRY_ITERATE(entry0, total_size, compat_release_entry, &j);
+       return ret;
 }
 
 static int

Reply via email to