netdev - [PATCH RFC 5/5] net/sched: act_gact: validate the control action inside init()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <469e1ba2395d64b15e8d1b8bf08fcd281a3572e7.1550271080.git.dcaratti@redhat.com>
Date:   Sat, 16 Feb 2019 00:06:31 +0100
From:   Davide Caratti <dcaratti@...hat.com>
To:     Jamal Hadi Salim <jhs@...atatu.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Vlad Buslov <vladbu@...lanox.com>,
        Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Subject: [PATCH RFC 5/5] net/sched: act_gact: validate the control action inside init()

Don't overwrite act_gact data if the control control action is not valid,
to prevent loosing the previous configuration in case validation failed.
Not doing that caused NULL dereference in the data path if 'goto chain'
is used.

Tested with:
 # ./tdc.py -c gact

Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain")
Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values")
Signed-off-by: Davide Caratti <dcaratti@...hat.com>
---
 net/sched/act_gact.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 727bbca9534b..530e8bb8f94d 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -20,6 +20,7 @@
 #include <linux/init.h>
 #include <net/netlink.h>
 #include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
 #include <linux/tc_act/tc_gact.h>
 #include <net/tc_act/tc_gact.h>
 
@@ -61,6 +62,7 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
 {
 	struct tc_action_net *tn = net_generic(net, gact_net_id);
 	struct nlattr *tb[TCA_GACT_MAX + 1];
+	struct tcf_chain *newchain = NULL;
 	struct tc_gact *parm;
 	struct tcf_gact *gact;
 	int ret = 0;
@@ -116,10 +118,15 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
 		return err;
 	}
 
+	err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack);
+	if (unlikely(err)) {
+		ret = err;
+		goto error;
+	}
 	gact = to_gact(*a);
 
 	spin_lock_bh(&gact->tcf_lock);
-	gact->tcf_action = parm->action;
+	tcf_action_set_ctrlact(*a, parm->action, newchain);
 #ifdef CONFIG_GACT_PROB
 	if (p_parm) {
 		gact->tcfg_paction = p_parm->paction;
@@ -135,7 +142,13 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
 
 	if (ret == ACT_P_CREATED)
 		tcf_idr_insert(tn, *a);
+end:
 	return ret;
+error:
+	if (newchain)
+		tcf_chain_put_by_act(newchain);
+	tcf_idr_release(*a, bind);
+	goto end;
 }
 
 static int tcf_gact_act(struct sk_buff *skb, const struct tc_action *a,
-- 
2.20.1