diff options
author | Davide Caratti <dcaratti@redhat.com> | 2018-09-04 13:00:19 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2018-09-06 01:18:54 -0400 |
commit | ee28bb56ac5b4c0c08ef10d33cc7adb749bbf4c6 (patch) | |
tree | 2bc7adbb31b75d65653887014c6e26c6daa6ac1d | |
parent | 0a3b8b2b215f9e84b82ae97df71292ccfd92b1e7 (diff) |
net/sched: fix memory leak in act_tunnel_key_init()
If users try to install act_tunnel_key 'set' rules with duplicate values
of 'index', the tunnel metadata are allocated, but never released. Then,
kmemleak complains as follows:
# tc a a a tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 index 111
# echo clear > /sys/kernel/debug/kmemleak
# tc a a a tunnel_key set src_ip 1.1.1.1 dst_ip 2.2.2.2 id 42 index 111
Error: TC IDR already exists.
We have an error talking to the kernel
# echo scan > /sys/kernel/debug/kmemleak
# cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff8800574e6c80 (size 256):
comm "tc", pid 5617, jiffies 4298118009 (age 57.990s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 1c e8 b0 ff ff ff ff ................
81 24 c2 ad ff ff ff ff 00 00 00 00 00 00 00 00 .$..............
backtrace:
[<00000000b7afbf4e>] tunnel_key_init+0x8a5/0x1800 [act_tunnel_key]
[<000000007d98fccd>] tcf_action_init_1+0x698/0xac0
[<0000000099b8f7cc>] tcf_action_init+0x15c/0x590
[<00000000dc60eebe>] tc_ctl_action+0x336/0x5c2
[<000000002f5a2f7d>] rtnetlink_rcv_msg+0x357/0x8e0
[<000000000bfe7575>] netlink_rcv_skb+0x124/0x350
[<00000000edab656f>] netlink_unicast+0x40f/0x5d0
[<00000000b322cdcb>] netlink_sendmsg+0x6e8/0xba0
[<0000000063d9d490>] sock_sendmsg+0xb3/0xf0
[<00000000f0d3315a>] ___sys_sendmsg+0x654/0x960
[<00000000c06cbd42>] __sys_sendmsg+0xd3/0x170
[<00000000ce72e4b0>] do_syscall_64+0xa5/0x470
[<000000005caa2d97>] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[<00000000fac1b476>] 0xffffffffffffffff
This problem theoretically happens also in case users attempt to setup a
geneve rule having wrong configuration data, or when the kernel fails to
allocate 'params_new'. Ensure that tunnel_key_init() releases the tunnel
metadata also in the above conditions.
Addresses-Coverity-ID: 1373974 ("Resource leak")
Fixes: d0f6dd8a914f4 ("net/sched: Introduce act_tunnel_key")
Fixes: 0ed5269f9e41f ("net/sched: add tunnel option support to act_tunnel_key")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | net/sched/act_tunnel_key.c | 16 |
1 files changed, 10 insertions, 6 deletions
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c index 420759153d5f..28d58bbc953e 100644 --- a/net/sched/act_tunnel_key.c +++ b/net/sched/act_tunnel_key.c | |||
@@ -317,7 +317,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla, | |||
317 | &metadata->u.tun_info, | 317 | &metadata->u.tun_info, |
318 | opts_len, extack); | 318 | opts_len, extack); |
319 | if (ret < 0) | 319 | if (ret < 0) |
320 | goto err_out; | 320 | goto release_tun_meta; |
321 | } | 321 | } |
322 | 322 | ||
323 | metadata->u.tun_info.mode |= IP_TUNNEL_INFO_TX; | 323 | metadata->u.tun_info.mode |= IP_TUNNEL_INFO_TX; |
@@ -333,23 +333,24 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla, | |||
333 | &act_tunnel_key_ops, bind, true); | 333 | &act_tunnel_key_ops, bind, true); |
334 | if (ret) { | 334 | if (ret) { |
335 | NL_SET_ERR_MSG(extack, "Cannot create TC IDR"); | 335 | NL_SET_ERR_MSG(extack, "Cannot create TC IDR"); |
336 | goto err_out; | 336 | goto release_tun_meta; |
337 | } | 337 | } |
338 | 338 | ||
339 | ret = ACT_P_CREATED; | 339 | ret = ACT_P_CREATED; |
340 | } else if (!ovr) { | 340 | } else if (!ovr) { |
341 | tcf_idr_release(*a, bind); | ||
342 | NL_SET_ERR_MSG(extack, "TC IDR already exists"); | 341 | NL_SET_ERR_MSG(extack, "TC IDR already exists"); |
343 | return -EEXIST; | 342 | ret = -EEXIST; |
343 | goto release_tun_meta; | ||
344 | } | 344 | } |
345 | 345 | ||
346 | t = to_tunnel_key(*a); | 346 | t = to_tunnel_key(*a); |
347 | 347 | ||
348 | params_new = kzalloc(sizeof(*params_new), GFP_KERNEL); | 348 | params_new = kzalloc(sizeof(*params_new), GFP_KERNEL); |
349 | if (unlikely(!params_new)) { | 349 | if (unlikely(!params_new)) { |
350 | tcf_idr_release(*a, bind); | ||
351 | NL_SET_ERR_MSG(extack, "Cannot allocate tunnel key parameters"); | 350 | NL_SET_ERR_MSG(extack, "Cannot allocate tunnel key parameters"); |
352 | return -ENOMEM; | 351 | ret = -ENOMEM; |
352 | exists = true; | ||
353 | goto release_tun_meta; | ||
353 | } | 354 | } |
354 | params_new->tcft_action = parm->t_action; | 355 | params_new->tcft_action = parm->t_action; |
355 | params_new->tcft_enc_metadata = metadata; | 356 | params_new->tcft_enc_metadata = metadata; |
@@ -367,6 +368,9 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla, | |||
367 | 368 | ||
368 | return ret; | 369 | return ret; |
369 | 370 | ||
371 | release_tun_meta: | ||
372 | dst_release(&metadata->dst); | ||
373 | |||
370 | err_out: | 374 | err_out: |
371 | if (exists) | 375 | if (exists) |
372 | tcf_idr_release(*a, bind); | 376 | tcf_idr_release(*a, bind); |