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
| ||
|
Date: Wed, 21 Feb 2018 19:55:52 -0800 From: Yonghong Song <yhs@...com> To: Eric Dumazet <eric.dumazet@...il.com>, Alexei Starovoitov <alexei.starovoitov@...il.com> CC: <ast@...com>, <daniel@...earbox.net>, <netdev@...r.kernel.org>, <kernel-team@...com> Subject: Re: [PATCH bpf v2] bpf: fix memory leak in lpm_trie map_free callback function On 2/21/18 7:40 PM, Eric Dumazet wrote: > On Tue, 2018-02-13 at 19:17 -0800, Alexei Starovoitov wrote: >> On Tue, Feb 13, 2018 at 07:00:21PM -0800, Yonghong Song wrote: >>> There is a memory leak happening in lpm_trie map_free callback >>> function trie_free. The trie structure itself does not get freed. >>> >>> Also, trie_free function did not do synchronize_rcu before freeing >>> various data structures. This is incorrect as some rcu_read_lock >>> region(s) for lookup, update, delete or get_next_key may not complete yet. >>> The fix is to add synchronize_rcu in the beginning of trie_free. >>> The useless spin_lock is removed from this function as well. >>> >>> Fixes: b95a5c4db09b ("bpf: add a longest prefix match trie map implementation") >>> Reported-by: Mathieu Malaterre <malat@...ian.org> >>> Reported-by: Alexei Starovoitov <ast@...nel.org> >>> Tested-by: Mathieu Malaterre <malat@...ian.org> >>> Signed-off-by: Yonghong Song <yhs@...com> >>> --- >>> kernel/bpf/lpm_trie.c | 11 +++++++---- >>> 1 file changed, 7 insertions(+), 4 deletions(-) >>> >>> v1->v2: >>> Make comments more precise and make label name more appropriate, >>> as suggested by Daniel >> >> Applied to bpf tree, Thanks Yonghong. > > > This does not look good. > > LOCKDEP surely should complain to > > node = rcu_dereference_protected(*slot, lockdep_is_held(&trie->lock)); > > Since we no longer hold trie->lock Eric, Thanks for spotting this issue. Will fix this issue soon. Yonghong
Powered by blists - more mailing lists