Re: [PATCH net 1/1] net sched actions: do not overwrite status of action creation.

[David Miller]

From: Roman Mashak 
Date: Fri, 24 Feb 2017 17:36:58 -0500

> nla_memdup_cookie was overwriting err value, declared at function
> scope and earlier initialized with result of ->init(). At success
> nla_memdup_cookie() returns 0, and thus module refcnt decremented,
> although the action was installed.
 ...
> Fixes: 1045ba77a ("net sched actions: Add support for user cookies")
> Signed-off-by: Roman Mashak 
> Signed-off-by: Jamal Hadi Salim 

Applied, thanks.

[PATCH net 1/1] net sched actions: do not overwrite status of action creation.

[Roman Mashak]

nla_memdup_cookie was overwriting err value, declared at function
scope and earlier initialized with result of ->init(). At success
nla_memdup_cookie() returns 0, and thus module refcnt decremented,
although the action was installed.

$ sudo tc actions add action pass index 1 cookie 1234
$ sudo tc actions ls action gact

action order 0: gact action pass
 random type none pass val 0
 index 1 ref 1 bind 0
$
$ lsmod
Module  Size  Used by
act_gact   16384  0
...
$
$ sudo rmmod act_gact
[   52.310283] [ cut here ]
[   52.312551] WARNING: CPU: 1 PID: 455 at kernel/module.c:1113
module_put+0x99/0xa0
[   52.316278] Modules linked in: act_gact(-) crct10dif_pclmul crc32_pclmul
ghash_clmulni_intel psmouse pcbc evbug aesni_intel aes_x86_64 crypto_simd
serio_raw glue_helper pcspkr cryptd
[   52.322285] CPU: 1 PID: 455 Comm: rmmod Not tainted 4.10.0+ #11
[   52.324261] Call Trace:
[   52.325132]  dump_stack+0x63/0x87
[   52.326236]  __warn+0xd1/0xf0
[   52.326260]  warn_slowpath_null+0x1d/0x20
[   52.326260]  module_put+0x99/0xa0
[   52.326260]  tcf_hashinfo_destroy+0x7f/0x90
[   52.326260]  gact_exit_net+0x27/0x40 [act_gact]
[   52.326260]  ops_exit_list.isra.6+0x38/0x60
[   52.326260]  unregister_pernet_operations+0x90/0xe0
[   52.326260]  unregister_pernet_subsys+0x21/0x30
[   52.326260]  tcf_unregister_action+0x68/0xa0
[   52.326260]  gact_cleanup_module+0x17/0xa0f [act_gact]
[   52.326260]  SyS_delete_module+0x1ba/0x220
[   52.326260]  entry_SYSCALL_64_fastpath+0x1e/0xad
[   52.326260] RIP: 0033:0x7f527ffae367
[   52.326260] RSP: 002b:7ffeb402a598 EFLAGS: 0202 ORIG_RAX:
00b0
[   52.326260] RAX: ffda RBX: 559b069912a0 RCX: 7f527ffae367
[   52.326260] RDX: 000a RSI: 0800 RDI: 559b06991308
[   52.326260] RBP: 0003 R08: 7f5280264420 R09: 7ffeb4029511
[   52.326260] R10: 087b R11: 0202 R12: 7ffeb4029580
[   52.326260] R13:  R14:  R15: 559b069912a0
[   52.354856] ---[ end trace 90d89401542b0db6 ]---
$

With the fix:

$ sudo modprobe act_gact
$ lsmod
Module  Size  Used by
act_gact   16384  0
...
$ sudo tc actions add action pass index 1 cookie 1234
$ sudo tc actions ls action gact

action order 0: gact action pass
 random type none pass val 0
 index 1 ref 1 bind 0
$
$ lsmod
Module  Size  Used by
act_gact   16384  1
...
$ sudo rmmod act_gact
rmmod: ERROR: Module act_gact is in use
$
$ sudo /home/mrv/bin/tc actions del action gact index 1
$ sudo rmmod act_gact
$ lsmod
Module  Size  Used by
$

Fixes: 1045ba77a ("net sched actions: Add support for user cookies")
Signed-off-by: Roman Mashak 
Signed-off-by: Jamal Hadi Salim 
---
 net/sched/act_api.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f219ff3..e59cb09 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -613,8 +613,8 @@ struct tc_action *tcf_action_init_1(struct net *net, struct 
nlattr *nla,
goto err_mod;
}
 
-   err = nla_memdup_cookie(a, tb);
-   if (err < 0) {
+   if (nla_memdup_cookie(a, tb) < 0) {
+   err = -ENOMEM;
tcf_hash_release(a, bind);
goto err_mod;
}
-- 
1.9.1