: gact action: fix multiple bugs in init path

# This is a BitKeeper generated diff -Nru style patch. #
# ChangeSet
# 2005/01/10 02:06:29+01:00 [email protected]
# [PKT_SCHED]: gact action: fix multiple bugs in init path
#

#   - rta can be NULL
#   - Attribute sizes are not checked
#   - No locking when replacing an action
#   - The action is inserted into the hash before its parameters are set

#
# Also return proper error codes.
#
# Signed-off-by: Patrick McHardy <[email protected]>
#
#

# net/sched/gact.c
#   2005/01/10 02:06:22+01:00 [email protected] +34 -32
#   [PKT_SCHED]: gact action: fix multiple bugs in init path
#   
#   - rta can be NULL
#   - Attribute sizes are not checked
#   - No locking when replacing an action
#   - The action is inserted into the hash before its parameters are set
#   
#   Also return proper error codes.
#   
#   Signed-off-by: Patrick McHardy <[email protected]>
#    
# 
# include/net/pkt_act.h
#   2005/01/10 02:06:22+01:00 [email protected] +10 -17
#   [PKT_SCHED]: gact action: fix multiple bugs in init path
#   
#   - rta can be NULL
#   - Attribute sizes are not checked
#   - No locking when replacing an action
#   - The action is inserted into the hash before its parameters are set
#   
#   Also return proper error codes.
#   
#   Signed-off-by: Patrick McHardy <[email protected]>
#    
# 
diff -Nru a/include/net/pkt_act.h b/include/net/pkt_act.h
--- a/include/net/pkt_act.h	2005-01-10 06:22:29 +01:00
+++ b/include/net/pkt_act.h	2005-01-10 06:22:29 +01:00
@@ -218,10 +218,10 @@

#ifdef CONFIG_NET_ACT_INIT
static inline struct tcf_st *
-tcf_hash_check(struct tc_st *parm, struct tc_action *a, int ovr, int bind) +tcf_hash_check(u32 index, struct tc_action *a, int ovr, int bind)
{
struct tcf_st *p = NULL;
- if (parm->index && (p = tcf_hash_lookup(parm->index)) != NULL) {
+ if (index && (p = tcf_hash_lookup(index)) != NULL) {
spin_lock(&p->lock);
if (bind) {
p->bindcnt++;
@@ -234,9 +234,8 @@
}

static inline struct tcf_st *
-tcf_hash_create(struct tc_st *parm, struct rtattr *est, struct tc_action *a, int size, int ovr, int bind) +tcf_hash_create(u32 index, struct rtattr *est, struct tc_action *a, int size, int ovr, int bind)
{
- unsigned h;
struct tcf_st *p = NULL;

p = kmalloc(size, GFP_KERNEL);
@@ -252,31 +251,25 @@

spin_lock_init(&p->lock);
p->stats_lock = &p->lock;
- p->index = parm->index ? : tcf_hash_new_index();
+ p->index = index ? : tcf_hash_new_index();
p->tm.install = jiffies;
p->tm.lastuse = jiffies;
#ifdef CONFIG_NET_ESTIMATOR
if (est)
gen_new_estimator(&p->bstats, &p->rate_est, p->stats_lock, est);
#endif
- h = tcf_hash(p->index);
- write_lock_bh(&tcf_t_lock);
- p->next = tcf_ht[h];
- tcf_ht[h] = p;
- write_unlock_bh(&tcf_t_lock);
-
a->priv = (void *) p;
return p;
}

-static inline struct tcf_st *
-tcf_hash_init(struct tc_st *parm, struct rtattr *est, struct tc_action *a, int size, int ovr, int bind) +static inline void tcf_hash_insert(struct tcf_st *p)
{
- struct tcf_st *p = tcf_hash_check (parm,a,ovr,bind);
+ unsigned h = tcf_hash(p->index);

- if (!p)
- p = tcf_hash_create(parm, est, a, size, ovr, bind);
- return p;
+ write_lock_bh(&tcf_t_lock);
+ p->next = tcf_ht[h];
+ tcf_ht[h] = p;
+ write_unlock_bh(&tcf_t_lock);
}

#endif

diff -Nru a/net/sched/gact.c b/net/sched/gact.c
@@ -75,51 +75,53 @@
                          struct tc_action *a, int ovr, int bind)
 {
 	struct rtattr *tb[TCA_GACT_MAX];
-	struct tc_gact *parm = NULL;
-#ifdef CONFIG_GACT_PROB
-	struct tc_gact_p *p_parm = NULL;
-#endif
-	struct tcf_gact *p = NULL;
+	struct tc_gact *parm;
+	struct tcf_gact *p;
 	int ret = 0;
 
-	if (rtattr_parse(tb, TCA_GACT_MAX, RTA_DATA(rta), RTA_PAYLOAD(rta)) < 0)
-		return -1;
-
-	if (tb[TCA_GACT_PARMS - 1] == NULL) {
-		printk("BUG: tcf_gact_init called with NULL params\n");
-		return -1;
-	}
+	if (rta == NULL ||
+	    rtattr_parse(tb, TCA_GACT_MAX, RTA_DATA(rta), RTA_PAYLOAD(rta)) < 0)
+		return -EINVAL;
 
+	if (tb[TCA_GACT_PARMS - 1] == NULL ||
+	    RTA_PAYLOAD(tb[TCA_GACT_PARMS - 1]) < sizeof(*parm))
+		return -EINVAL;
 	parm = RTA_DATA(tb[TCA_GACT_PARMS - 1]);
+
+	if (tb[TCA_GACT_PROB-1] != NULL)
 #ifdef CONFIG_GACT_PROB
-	if (tb[TCA_GACT_PROB - 1] != NULL)
-		p_parm = RTA_DATA(tb[TCA_GACT_PROB - 1]);
+		if (RTA_PAYLOAD(tb[TCA_GACT_PROB-1]) < sizeof(struct tc_gact_p))
+			return -EINVAL;
+#else
+		return -EOPNOTSUPP;
 #endif
-	p = tcf_hash_check(parm, a, ovr, bind);
+
+	p = tcf_hash_check(parm->index, a, ovr, bind);
 	if (p == NULL) {
-		p = tcf_hash_create(parm, est, a, sizeof(*p), ovr, bind);
+		p = tcf_hash_create(parm->index, est, a, sizeof(*p), ovr, bind);
 		if (p == NULL)
-			return -1;
-		else {
-			p->refcnt = 1;
-			ret = 1;
-			goto override;
+			return -ENOMEM;
+		ret = ACT_P_CREATED;
+	} else {
+		if (!ovr) {
+			tcf_hash_release(p, bind);
+			return -EEXIST;
 		}
 	}
 
-	if (ovr) {
-override:
-		p->action = parm->action;
+	spin_lock_bh(&p->lock);
+	p->action = parm->action;
 #ifdef CONFIG_GACT_PROB
-		if (p_parm != NULL) {
-			p->paction = p_parm->paction;
-			p->pval = p_parm->pval;
-			p->ptype = p_parm->ptype;
-		} else {
-			p->paction = p->pval = p->ptype = 0;
-		}
-#endif
+	if (tb[TCA_GACT_PROB-1] != NULL) {
+		struct tc_gact_p *p_parm = RTA_DATA(tb[TCA_GACT_PROB-1]);
+		p->paction = p_parm->paction;
+		p->pval    = p_parm->pval;
+		p->ptype   = p_parm->ptype;
 	}
+#endif
+	spin_unlock_bh(&p->lock);
+	if (ret == ACT_P_CREATED)
+		tcf_hash_insert(p);
 	return ret;
 }