| Message ID | 20230425144725.669262-1-pctammela@mojatatu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1b483d9f5805c7e3d628d4995e97f4311fcb82eb |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net/sched: act_pedit: free pedit keys on bail from offset check | expand |
On Tue, Apr 25, 2023 at 11:47:25AM -0300, Pedro Tammela wrote: > Ido Schimmel reports a memleak on a syzkaller instance: > BUG: memory leak > unreferenced object 0xffff88803d45e400 (size 1024): > comm "syz-executor292", pid 563, jiffies 4295025223 (age 51.781s) > hex dump (first 32 bytes): > 28 bd 70 00 fb db df 25 02 00 14 1f ff 02 00 02 (.p....%........ > 00 32 00 00 1f 00 00 00 ac 14 14 3e 08 00 07 00 .2.........>.... > backtrace: > [<ffffffff81bd0f2c>] kmemleak_alloc_recursive include/linux/kmemleak.h:42 [inline] > [<ffffffff81bd0f2c>] slab_post_alloc_hook mm/slab.h:772 [inline] > [<ffffffff81bd0f2c>] slab_alloc_node mm/slub.c:3452 [inline] > [<ffffffff81bd0f2c>] __kmem_cache_alloc_node+0x25c/0x320 mm/slub.c:3491 > [<ffffffff81a865d9>] __do_kmalloc_node mm/slab_common.c:966 [inline] > [<ffffffff81a865d9>] __kmalloc+0x59/0x1a0 mm/slab_common.c:980 > [<ffffffff83aa85c3>] kmalloc include/linux/slab.h:584 [inline] > [<ffffffff83aa85c3>] tcf_pedit_init+0x793/0x1ae0 net/sched/act_pedit.c:245 > [<ffffffff83a90623>] tcf_action_init_1+0x453/0x6e0 net/sched/act_api.c:1394 > [<ffffffff83a90e58>] tcf_action_init+0x5a8/0x950 net/sched/act_api.c:1459 > [<ffffffff83a96258>] tcf_action_add+0x118/0x4e0 net/sched/act_api.c:1985 > [<ffffffff83a96997>] tc_ctl_action+0x377/0x490 net/sched/act_api.c:2044 > [<ffffffff83920a8d>] rtnetlink_rcv_msg+0x46d/0xd70 net/core/rtnetlink.c:6395 > [<ffffffff83b24305>] netlink_rcv_skb+0x185/0x490 net/netlink/af_netlink.c:2575 > [<ffffffff83901806>] rtnetlink_rcv+0x26/0x30 net/core/rtnetlink.c:6413 > [<ffffffff83b21cae>] netlink_unicast_kernel net/netlink/af_netlink.c:1339 [inline] > [<ffffffff83b21cae>] netlink_unicast+0x5be/0x8a0 net/netlink/af_netlink.c:1365 > [<ffffffff83b2293f>] netlink_sendmsg+0x9af/0xed0 net/netlink/af_netlink.c:1942 > [<ffffffff8380c39f>] sock_sendmsg_nosec net/socket.c:724 [inline] > [<ffffffff8380c39f>] sock_sendmsg net/socket.c:747 [inline] > [<ffffffff8380c39f>] ____sys_sendmsg+0x3ef/0xaa0 net/socket.c:2503 > [<ffffffff838156d2>] ___sys_sendmsg+0x122/0x1c0 net/socket.c:2557 > [<ffffffff8381594f>] __sys_sendmsg+0x11f/0x200 net/socket.c:2586 > [<ffffffff83815ab0>] __do_sys_sendmsg net/socket.c:2595 [inline] > [<ffffffff83815ab0>] __se_sys_sendmsg net/socket.c:2593 [inline] > [<ffffffff83815ab0>] __x64_sys_sendmsg+0x80/0xc0 net/socket.c:2593 > > The recently added static offset check missed a free to the key buffer when > bailing out on error. > > Fixes: e1201bc781c2 ("net/sched: act_pedit: check static offsets a priori") > Reported-by: Ido Schimmel <idosch@idosch.org> > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Reviewed-by: Ido Schimmel <idosch@nvidia.com> Tested-by: Ido Schimmel <idosch@nvidia.com>
Hello: This patch was applied to netdev/net.git (main) by Paolo Abeni <pabeni@redhat.com>: On Tue, 25 Apr 2023 11:47:25 -0300 you wrote: > Ido Schimmel reports a memleak on a syzkaller instance: > BUG: memory leak > unreferenced object 0xffff88803d45e400 (size 1024): > comm "syz-executor292", pid 563, jiffies 4295025223 (age 51.781s) > hex dump (first 32 bytes): > 28 bd 70 00 fb db df 25 02 00 14 1f ff 02 00 02 (.p....%........ > 00 32 00 00 1f 00 00 00 ac 14 14 3e 08 00 07 00 .2.........>.... > backtrace: > [<ffffffff81bd0f2c>] kmemleak_alloc_recursive include/linux/kmemleak.h:42 [inline] > [<ffffffff81bd0f2c>] slab_post_alloc_hook mm/slab.h:772 [inline] > [<ffffffff81bd0f2c>] slab_alloc_node mm/slub.c:3452 [inline] > [<ffffffff81bd0f2c>] __kmem_cache_alloc_node+0x25c/0x320 mm/slub.c:3491 > [<ffffffff81a865d9>] __do_kmalloc_node mm/slab_common.c:966 [inline] > [<ffffffff81a865d9>] __kmalloc+0x59/0x1a0 mm/slab_common.c:980 > [<ffffffff83aa85c3>] kmalloc include/linux/slab.h:584 [inline] > [<ffffffff83aa85c3>] tcf_pedit_init+0x793/0x1ae0 net/sched/act_pedit.c:245 > [<ffffffff83a90623>] tcf_action_init_1+0x453/0x6e0 net/sched/act_api.c:1394 > [<ffffffff83a90e58>] tcf_action_init+0x5a8/0x950 net/sched/act_api.c:1459 > [<ffffffff83a96258>] tcf_action_add+0x118/0x4e0 net/sched/act_api.c:1985 > [<ffffffff83a96997>] tc_ctl_action+0x377/0x490 net/sched/act_api.c:2044 > [<ffffffff83920a8d>] rtnetlink_rcv_msg+0x46d/0xd70 net/core/rtnetlink.c:6395 > [<ffffffff83b24305>] netlink_rcv_skb+0x185/0x490 net/netlink/af_netlink.c:2575 > [<ffffffff83901806>] rtnetlink_rcv+0x26/0x30 net/core/rtnetlink.c:6413 > [<ffffffff83b21cae>] netlink_unicast_kernel net/netlink/af_netlink.c:1339 [inline] > [<ffffffff83b21cae>] netlink_unicast+0x5be/0x8a0 net/netlink/af_netlink.c:1365 > [<ffffffff83b2293f>] netlink_sendmsg+0x9af/0xed0 net/netlink/af_netlink.c:1942 > [<ffffffff8380c39f>] sock_sendmsg_nosec net/socket.c:724 [inline] > [<ffffffff8380c39f>] sock_sendmsg net/socket.c:747 [inline] > [<ffffffff8380c39f>] ____sys_sendmsg+0x3ef/0xaa0 net/socket.c:2503 > [<ffffffff838156d2>] ___sys_sendmsg+0x122/0x1c0 net/socket.c:2557 > [<ffffffff8381594f>] __sys_sendmsg+0x11f/0x200 net/socket.c:2586 > [<ffffffff83815ab0>] __do_sys_sendmsg net/socket.c:2595 [inline] > [<ffffffff83815ab0>] __se_sys_sendmsg net/socket.c:2593 [inline] > [<ffffffff83815ab0>] __x64_sys_sendmsg+0x80/0xc0 net/socket.c:2593 > > [...] Here is the summary with links: - [net-next] net/sched: act_pedit: free pedit keys on bail from offset check https://git.kernel.org/netdev/net/c/1b483d9f5805 You are awesome, thank you!
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index fb93d4c1faca..fc945c7e4123 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -258,7 +258,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla, if (!offmask && cur % 4) { NL_SET_ERR_MSG_MOD(extack, "Offsets must be on 32bit boundaries"); ret = -EINVAL; - goto put_chain; + goto out_free_keys; } /* sanitize the shift value for any later use */ @@ -291,6 +291,8 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla, return ret; +out_free_keys: + kfree(nparms->tcfp_keys); put_chain: if (goto_ch) tcf_chain_put_by_act(goto_ch);
Ido Schimmel reports a memleak on a syzkaller instance: BUG: memory leak unreferenced object 0xffff88803d45e400 (size 1024): comm "syz-executor292", pid 563, jiffies 4295025223 (age 51.781s) hex dump (first 32 bytes): 28 bd 70 00 fb db df 25 02 00 14 1f ff 02 00 02 (.p....%........ 00 32 00 00 1f 00 00 00 ac 14 14 3e 08 00 07 00 .2.........>.... backtrace: [<ffffffff81bd0f2c>] kmemleak_alloc_recursive include/linux/kmemleak.h:42 [inline] [<ffffffff81bd0f2c>] slab_post_alloc_hook mm/slab.h:772 [inline] [<ffffffff81bd0f2c>] slab_alloc_node mm/slub.c:3452 [inline] [<ffffffff81bd0f2c>] __kmem_cache_alloc_node+0x25c/0x320 mm/slub.c:3491 [<ffffffff81a865d9>] __do_kmalloc_node mm/slab_common.c:966 [inline] [<ffffffff81a865d9>] __kmalloc+0x59/0x1a0 mm/slab_common.c:980 [<ffffffff83aa85c3>] kmalloc include/linux/slab.h:584 [inline] [<ffffffff83aa85c3>] tcf_pedit_init+0x793/0x1ae0 net/sched/act_pedit.c:245 [<ffffffff83a90623>] tcf_action_init_1+0x453/0x6e0 net/sched/act_api.c:1394 [<ffffffff83a90e58>] tcf_action_init+0x5a8/0x950 net/sched/act_api.c:1459 [<ffffffff83a96258>] tcf_action_add+0x118/0x4e0 net/sched/act_api.c:1985 [<ffffffff83a96997>] tc_ctl_action+0x377/0x490 net/sched/act_api.c:2044 [<ffffffff83920a8d>] rtnetlink_rcv_msg+0x46d/0xd70 net/core/rtnetlink.c:6395 [<ffffffff83b24305>] netlink_rcv_skb+0x185/0x490 net/netlink/af_netlink.c:2575 [<ffffffff83901806>] rtnetlink_rcv+0x26/0x30 net/core/rtnetlink.c:6413 [<ffffffff83b21cae>] netlink_unicast_kernel net/netlink/af_netlink.c:1339 [inline] [<ffffffff83b21cae>] netlink_unicast+0x5be/0x8a0 net/netlink/af_netlink.c:1365 [<ffffffff83b2293f>] netlink_sendmsg+0x9af/0xed0 net/netlink/af_netlink.c:1942 [<ffffffff8380c39f>] sock_sendmsg_nosec net/socket.c:724 [inline] [<ffffffff8380c39f>] sock_sendmsg net/socket.c:747 [inline] [<ffffffff8380c39f>] ____sys_sendmsg+0x3ef/0xaa0 net/socket.c:2503 [<ffffffff838156d2>] ___sys_sendmsg+0x122/0x1c0 net/socket.c:2557 [<ffffffff8381594f>] __sys_sendmsg+0x11f/0x200 net/socket.c:2586 [<ffffffff83815ab0>] __do_sys_sendmsg net/socket.c:2595 [inline] [<ffffffff83815ab0>] __se_sys_sendmsg net/socket.c:2593 [inline] [<ffffffff83815ab0>] __x64_sys_sendmsg+0x80/0xc0 net/socket.c:2593 The recently added static offset check missed a free to the key buffer when bailing out on error. Fixes: e1201bc781c2 ("net/sched: act_pedit: check static offsets a priori") Reported-by: Ido Schimmel <idosch@idosch.org> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> --- net/sched/act_pedit.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)