[PATCH PKT_SCHED 13/17]: Clean up tcf_ipt_init

[Patrick McHardy <kaber@xxxxxxxxx>]

Clean up tcf_ipt_init:

- Return proper error codes
- Check size of TCA_IPT_INDEX attribute
- Remove useless cast
- Rip out totally broken override bits
- Consolidate error path

The override part leaks memory, does not uninit the old
iptables target, needs GFP_ATOMIC allocations because a
lock is held and fails anyway. It think sharing code with
normal initialization obfuscates both parts, so I ripped
it out for now.

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/12/30 03:29:42+01:00 kaber@xxxxxxxxxxxx 
#   [PKT_SCHED]: Clean up tcf_ipt_init
#   
#   - Return proper error codes
#   - Check size of TCA_IPT_INDEX attribute
#   - Remove useless cast
#   - Rip out totally broken override bits
#   - Consolidate error path
#   
#   Signed-off-by: Patrick McHardy <kaber@xxxxxxxxx>
# 
# net/sched/ipt.c
#   2004/12/30 03:29:35+01:00 kaber@xxxxxxxxxxxx +30 -56
#   [PKT_SCHED]: Clean up tcf_ipt_init
#   
#   - Return proper error codes
#   - Check size of TCA_IPT_INDEX attribute
#   - Remove useless cast
#   - Rip out totally broken override bits
#   - Consolidate error path
#   
#   Signed-off-by: Patrick McHardy <kaber@xxxxxxxxx>
# 
diff -Nru a/net/sched/ipt.c b/net/sched/ipt.c
--- a/net/sched/ipt.c   2004-12-30 04:01:51 +01:00
+++ b/net/sched/ipt.c   2004-12-30 04:01:51 +01:00
@@ -60,12 +60,10 @@
        struct ipt_target *target;
        int ret = 0;
        struct ipt_entry_target *t = p->t;
-       target = __ipt_find_target_lock(t->u.user.name, &ret);
 
-       if (!target) {
-               printk("init_targ: Failed to find %s\n", t->u.user.name);
-               return -1;
-       }
+       target = __ipt_find_target_lock(t->u.user.name, &ret);
+       if (!target)
+               return -ENOENT;
 
        DPRINTK("init_targ: found %s\n", target->name);
        /* we really need proper ref counting
@@ -105,33 +103,31 @@
        u32 hook = 0;
 
        if (rtattr_parse(tb, TCA_IPT_MAX, RTA_DATA(rta), RTA_PAYLOAD(rta)) < 0)
-               return -1;
+               return -EINVAL;
 
-       if (tb[TCA_IPT_INDEX - 1]) {
+       if (tb[TCA_IPT_INDEX - 1] &&
+           RTA_PAYLOAD(tb[TCA_IPT_INDEX - 1]) >= sizeof(index))
                index = *(u32 *) RTA_DATA(tb[TCA_IPT_INDEX - 1]);
-               DPRINTK("ipt index %d\n", index);
-       }
 
        if (index && (p = tcf_hash_lookup(index)) != NULL) {
-               a->priv = (void *) p;
+               a->priv = p;
                spin_lock(&p->lock);
                if (bind) {
                        p->bindcnt += 1;
                        p->refcnt += 1;
                }
-               if (ovr) {
-                       goto override;
-               }
+               if (ovr)
+                       ret = -EOPNOTSUPP;
                spin_unlock(&p->lock);
                return ret;
        }
 
        if (tb[TCA_IPT_TARG - 1] == NULL || tb[TCA_IPT_HOOK - 1] == NULL)
-               return -1;
+               return -EINVAL;
 
        p = kmalloc(sizeof(*p), GFP_KERNEL);
        if (p == NULL)
-               return -1;
+               return -ENOMEM;
        memset(p, 0, sizeof(*p));
        p->refcnt = 1;
        ret = 1;
@@ -140,45 +136,30 @@
        if (bind)
                p->bindcnt = 1;
 
-override:
+ /* override: */
        hook = *(u32 *) RTA_DATA(tb[TCA_IPT_HOOK - 1]);
 
        t = (struct ipt_entry_target *) RTA_DATA(tb[TCA_IPT_TARG - 1]);
 
+       ret = -ENOMEM;
        p->t = kmalloc(t->u.target_size, GFP_KERNEL);
-       if (p->t == NULL) {
-               if (ovr) {
-                       printk("ipt policy messed up \n");
-                       spin_unlock(&p->lock);
-                       return -1;
-               }
-               kfree(p);
-               return -1;
-       }
-
+       if (p->t == NULL)
+               goto err1;
        memcpy(p->t, RTA_DATA(tb[TCA_IPT_TARG - 1]), t->u.target_size);
+
        DPRINTK(" target NAME %s size %d data[0] %x data[1] %x\n",
                t->u.user.name, t->u.target_size, t->data[0], t->data[1]);
 
        p->tname = kmalloc(IFNAMSIZ, GFP_KERNEL);
-
-       if (p->tname == NULL) {
-               if (ovr) {
-                       printk("ipt policy messed up 2 \n");
-                       spin_unlock(&p->lock);
-                       return -1;
-               }
-               kfree(p->t);
-               kfree(p);
-               return -1;
-       } else {
+       if (p->tname == NULL)
+               goto err2;
+       else {
                int csize = IFNAMSIZ - 1;
 
                memset(p->tname, 0, IFNAMSIZ);
                if (tb[TCA_IPT_TABLE - 1]) {
-                       if (strlen((char *) RTA_DATA(tb[TCA_IPT_TABLE - 1])) <
-                           csize)
-                               csize = strlen(RTA_DATA(tb[TCA_IPT_TABLE - 1]));
+                       if (RTA_PAYLOAD(tb[TCA_IPT_TABLE - 1]) < csize)
+                               csize = RTA_PAYLOAD(tb[TCA_IPT_TABLE - 1]);
                        strncpy(p->tname, RTA_DATA(tb[TCA_IPT_TABLE - 1]),
                                csize);
                        DPRINTK("table name %s\n", p->tname);
@@ -187,22 +168,8 @@
                }
        }
 
-       if (init_targ(p) < 0) {
-               if (ovr) {
-                       printk("ipt policy messed up 2 \n");
-                       spin_unlock(&p->lock);
-                       return -1;
-               }
-               kfree(p->tname);
-               kfree(p->t);
-               kfree(p);
-               return -1;
-       }
-
-       if (ovr) {
-               spin_unlock(&p->lock);
-               return -1;
-       }
+       if ((ret = init_targ(p)) < 0)
+               goto err3;
 
        p->index = index ? : tcf_hash_new_index();
 
@@ -223,6 +190,13 @@
        a->priv = p;
        return ret;
 
+err3:
+       kfree(p->tname);
+err2:
+       kfree(p->t);
+err1:
+       kfree(p);
+       return -1;
 }
 
 static int