| Message ID | 20180123065351.2690530-1-yhs@fb.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf-next] bpf: fix incorrect kmalloc usage in lpm_trie MAP_GET_NEXT_KEY rcu region | expand |
On Mon, 2018-01-22 at 22:53 -0800, Yonghong Song wrote: > In commit b471f2f1de8b ("bpf: implement MAP_GET_NEXT_KEY command for LPM_TRIE map"), > the implemented MAP_GET_NEXT_KEY callback function is guarded with rcu read lock. > In the function body, "kmalloc(size, GFP_USER | __GFP_NOWARN)" is used which may > sleep and violate rcu read lock region requirements. This patch fixed the issue > by using GFP_ATOMIC instead to avoid blocking kmalloc. Tested with > CONFIG_DEBUG_ATOMIC_SLEEP=y as suggested by Eric Dumazet. > > Fixes: b471f2f1de8b ("bpf: implement MAP_GET_NEXT_KEY command for LPM_TRIE map") > Signed-off-by: Yonghong Song <yhs@fb.com> Reported-by: syzbot <syzkaller@googlegroups.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Thanks. > --- > kernel/bpf/lpm_trie.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c > index d7ea962..8f083ea 100644 > --- a/kernel/bpf/lpm_trie.c > +++ b/kernel/bpf/lpm_trie.c > @@ -624,7 +624,7 @@ static int trie_get_next_key(struct bpf_map *map, void *_key, void *_next_key) > } > > node_stack = kmalloc(trie->max_prefixlen * sizeof(struct lpm_trie_node *), > - GFP_USER | __GFP_NOWARN); > + GFP_ATOMIC | __GFP_NOWARN); > if (!node_stack) > return -ENOMEM; >
On 01/23/2018 04:50 PM, Eric Dumazet wrote: > On Mon, 2018-01-22 at 22:53 -0800, Yonghong Song wrote: >> In commit b471f2f1de8b ("bpf: implement MAP_GET_NEXT_KEY command for LPM_TRIE map"), >> the implemented MAP_GET_NEXT_KEY callback function is guarded with rcu read lock. >> In the function body, "kmalloc(size, GFP_USER | __GFP_NOWARN)" is used which may >> sleep and violate rcu read lock region requirements. This patch fixed the issue >> by using GFP_ATOMIC instead to avoid blocking kmalloc. Tested with >> CONFIG_DEBUG_ATOMIC_SLEEP=y as suggested by Eric Dumazet. >> >> Fixes: b471f2f1de8b ("bpf: implement MAP_GET_NEXT_KEY command for LPM_TRIE map") >> Signed-off-by: Yonghong Song <yhs@fb.com> > > Reported-by: syzbot <syzkaller@googlegroups.com> > Reviewed-by: Eric Dumazet <edumazet@google.com> Applied to bpf-next, thanks everyone!
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c index d7ea962..8f083ea 100644 --- a/kernel/bpf/lpm_trie.c +++ b/kernel/bpf/lpm_trie.c @@ -624,7 +624,7 @@ static int trie_get_next_key(struct bpf_map *map, void *_key, void *_next_key) } node_stack = kmalloc(trie->max_prefixlen * sizeof(struct lpm_trie_node *), - GFP_USER | __GFP_NOWARN); + GFP_ATOMIC | __GFP_NOWARN); if (!node_stack) return -ENOMEM;
In commit b471f2f1de8b ("bpf: implement MAP_GET_NEXT_KEY command for LPM_TRIE map"), the implemented MAP_GET_NEXT_KEY callback function is guarded with rcu read lock. In the function body, "kmalloc(size, GFP_USER | __GFP_NOWARN)" is used which may sleep and violate rcu read lock region requirements. This patch fixed the issue by using GFP_ATOMIC instead to avoid blocking kmalloc. Tested with CONFIG_DEBUG_ATOMIC_SLEEP=y as suggested by Eric Dumazet. Fixes: b471f2f1de8b ("bpf: implement MAP_GET_NEXT_KEY command for LPM_TRIE map") Signed-off-by: Yonghong Song <yhs@fb.com> --- kernel/bpf/lpm_trie.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)