netdev - [PATCH net] net/sched: act_police: fix memory leak in case of invalid control action

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-next>] [day] [month] [year] [list]

Date:   Wed, 28 Nov 2018 18:43:42 +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>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Cc:     Dan Carpenter <dan.carpenter@...cle.com>
Subject: [PATCH net] net/sched: act_police: fix memory leak in case of invalid control action

when users set an invalid control action, kmemleak complains as follows:

 # echo clear >/sys/kernel/debug/kmemleak
 # ./tdc.py -e b48b
 Test b48b: Add police action with exceed goto chain control action
 All test results:

 1..1
 ok 1 - b48b # Add police action with exceed goto chain control action
 about to flush the tap output if tests need to be skipped
 done flushing skipped test tap output
 # echo scan >/sys/kernel/debug/kmemleak
 # cat /sys/kernel/debug/kmemleak
 unreferenced object 0xffffa0fafbc3dde0 (size 96):
  comm "tc", pid 2358, jiffies 4294922738 (age 17.022s)
  hex dump (first 32 bytes):
    2a 00 00 20 00 00 00 00 00 00 7d 00 00 00 00 00  *.. ......}.....
    f8 07 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000648803d2>] tcf_action_init_1+0x384/0x4c0
    [<00000000cb69382e>] tcf_action_init+0x12b/0x1a0
    [<00000000847ef0d4>] tcf_action_add+0x73/0x170
    [<0000000093656e14>] tc_ctl_action+0x122/0x160
    [<0000000023c98e32>] rtnetlink_rcv_msg+0x263/0x2d0
    [<000000003493ae9c>] netlink_rcv_skb+0x4d/0x130
    [<00000000de63f8ba>] netlink_unicast+0x209/0x2d0
    [<00000000c3da0ebe>] netlink_sendmsg+0x2c1/0x3c0
    [<000000007a9e0753>] sock_sendmsg+0x33/0x40
    [<00000000457c6d2e>] ___sys_sendmsg+0x2a0/0x2f0
    [<00000000c5c6a086>] __sys_sendmsg+0x5e/0xa0
    [<00000000446eafce>] do_syscall_64+0x5b/0x180
    [<000000004aa871f2>] entry_SYSCALL_64_after_hwframe+0x44/0xa9
    [<00000000450c38ef>] 0xffffffffffffffff

change tcf_police_init() to avoid leaking 'new' in case TCA_POLICE_RESULT
contains TC_ACT_GOTO_CHAIN extended action.

Fixes: c08f5ed5d625 ("net/sched: act_police: disallow 'goto chain' on fallback control action")
Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
Signed-off-by: Davide Caratti <dcaratti@...hat.com>
---
 net/sched/act_police.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 37c9b8f0e10f..ec8ec55e0fe8 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -85,7 +85,7 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 			       int ovr, int bind, bool rtnl_held,
 			       struct netlink_ext_ack *extack)
 {
-	int ret = 0, err;
+	int ret = 0, tcfp_result = TC_ACT_OK, err, size;
 	struct nlattr *tb[TCA_POLICE_MAX + 1];
 	struct tc_police *parm;
 	struct tcf_police *police;
@@ -93,7 +93,6 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 	struct tc_action_net *tn = net_generic(net, police_net_id);
 	struct tcf_police_params *new;
 	bool exists = false;
-	int size;
 
 	if (nla == NULL)
 		return -EINVAL;
@@ -160,6 +159,16 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 		goto failure;
 	}
 
+	if (tb[TCA_POLICE_RESULT]) {
+		tcfp_result = nla_get_u32(tb[TCA_POLICE_RESULT]);
+		if (TC_ACT_EXT_CMP(tcfp_result, TC_ACT_GOTO_CHAIN)) {
+			NL_SET_ERR_MSG(extack,
+				       "goto chain not allowed on fallback");
+			err = -EINVAL;
+			goto failure;
+		}
+	}
+
 	new = kzalloc(sizeof(*new), GFP_KERNEL);
 	if (unlikely(!new)) {
 		err = -ENOMEM;
@@ -167,6 +176,7 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 	}
 
 	/* No failure allowed after this point */
+	new->tcfp_result = tcfp_result;
 	new->tcfp_mtu = parm->mtu;
 	if (!new->tcfp_mtu) {
 		new->tcfp_mtu = ~0;
@@ -196,16 +206,6 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 	if (tb[TCA_POLICE_AVRATE])
 		new->tcfp_ewma_rate = nla_get_u32(tb[TCA_POLICE_AVRATE]);
 
-	if (tb[TCA_POLICE_RESULT]) {
-		new->tcfp_result = nla_get_u32(tb[TCA_POLICE_RESULT]);
-		if (TC_ACT_EXT_CMP(new->tcfp_result, TC_ACT_GOTO_CHAIN)) {
-			NL_SET_ERR_MSG(extack,
-				       "goto chain not allowed on fallback");
-			err = -EINVAL;
-			goto failure;
-		}
-	}
-
 	spin_lock_bh(&police->tcf_lock);
 	spin_lock_bh(&police->tcfp_lock);
 	police->tcfp_t_c = ktime_get_ns();
-- 
2.19.1