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]
Date:   Sat, 16 Feb 2019 00:06:30 +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 4/5] net/sched: act_csum: validate the control action inside init()

Don't overwrite act_csum 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 csum

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_csum.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 1ae120c9ab02..bf0940156886 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -33,6 +33,7 @@
 #include <net/sctp/checksum.h>
 
 #include <net/act_api.h>
+#include <net/pkt_cls.h>
 
 #include <linux/tc_act/tc_csum.h>
 #include <net/tc_act/tc_csum.h>
@@ -52,6 +53,7 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
 	struct tc_action_net *tn = net_generic(net, csum_net_id);
 	struct tcf_csum_params *params_new;
 	struct nlattr *tb[TCA_CSUM_MAX + 1];
+	struct tcf_chain *newchain = NULL;
 	struct tc_csum *parm;
 	struct tcf_csum *p;
 	int ret = 0, err;
@@ -87,17 +89,23 @@ static int tcf_csum_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;
+	}
+
 	p = to_tcf_csum(*a);
 
 	params_new = kzalloc(sizeof(*params_new), GFP_KERNEL);
 	if (unlikely(!params_new)) {
-		tcf_idr_release(*a, bind);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto error;
 	}
 	params_new->update_flags = parm->update_flags;
 
 	spin_lock_bh(&p->tcf_lock);
-	p->tcf_action = parm->action;
+	tcf_action_set_ctrlact(*a, parm->action, newchain);
 	rcu_swap_protected(p->params, params_new,
 			   lockdep_is_held(&p->tcf_lock));
 	spin_unlock_bh(&p->tcf_lock);
@@ -108,7 +116,13 @@ static int tcf_csum_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;
 }
 
 /**
-- 
2.20.1

Powered by blists - more mailing lists