| Message ID | 1518564832.3715.187.camel@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf-next] bpf: fix sock_map_alloc() error path | expand |
On Tue, 2018-02-13 at 15:33 -0800, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > In case user program provides silly parameters, we want > a map_alloc() handler to return an error, not a NULL pointer, > otherwise we crash later in find_and_alloc_map() > > Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: syzbot <syzkaller@googlegroups.com> > --- This would apply to net or bpf trees, not -next ones, sorry for the confusion in the [PATCH bpf-next] prefix. Bug was added in 4.14
Hi Eric, On 02/14/2018 12:43 AM, Eric Dumazet wrote: > On Tue, 2018-02-13 at 15:33 -0800, Eric Dumazet wrote: >> From: Eric Dumazet <edumazet@google.com> >> >> In case user program provides silly parameters, we want >> a map_alloc() handler to return an error, not a NULL pointer, >> otherwise we crash later in find_and_alloc_map() >> >> Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support") >> Signed-off-by: Eric Dumazet <edumazet@google.com> >> Reported-by: syzbot <syzkaller@googlegroups.com> >> --- > > This would apply to net or bpf trees, not -next ones, sorry for the > confusion in the [PATCH bpf-next] prefix. No problem, thanks for the fix! > Bug was added in 4.14 Fixes tag is actually slightly different, it would be: Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks") I can change it if you want, no need to resend. Thanks, Daniel
On 02/13/2018 05:19 PM, Daniel Borkmann wrote: > Hi Eric, > > On 02/14/2018 12:43 AM, Eric Dumazet wrote: >> On Tue, 2018-02-13 at 15:33 -0800, Eric Dumazet wrote: >>> From: Eric Dumazet <edumazet@google.com> >>> >>> In case user program provides silly parameters, we want >>> a map_alloc() handler to return an error, not a NULL pointer, >>> otherwise we crash later in find_and_alloc_map() >>> >>> Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support") >>> Signed-off-by: Eric Dumazet <edumazet@google.com> >>> Reported-by: syzbot <syzkaller@googlegroups.com> >>> --- Thanks Eric, also great to see syzkaller working on the sockmap types. Acked-by: John Fastabend <john.fastabend@gmail.com> >> >> This would apply to net or bpf trees, not -next ones, sorry for the >> confusion in the [PATCH bpf-next] prefix. > > No problem, thanks for the fix! > >> Bug was added in 4.14 > > Fixes tag is actually slightly different, it would be: > > Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks") > Yep. > I can change it if you want, no need to resend.> Thanks, > Daniel >
On Tue, Feb 13, 2018 at 06:53:36PM -0800, John Fastabend wrote: > On 02/13/2018 05:19 PM, Daniel Borkmann wrote: > > Hi Eric, > > > > On 02/14/2018 12:43 AM, Eric Dumazet wrote: > >> On Tue, 2018-02-13 at 15:33 -0800, Eric Dumazet wrote: > >>> From: Eric Dumazet <edumazet@google.com> > >>> > >>> In case user program provides silly parameters, we want > >>> a map_alloc() handler to return an error, not a NULL pointer, > >>> otherwise we crash later in find_and_alloc_map() > >>> > >>> Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support") > >>> Signed-off-by: Eric Dumazet <edumazet@google.com> > >>> Reported-by: syzbot <syzkaller@googlegroups.com> > >>> --- > > Thanks Eric, also great to see syzkaller working on the sockmap > types. > > Acked-by: John Fastabend <john.fastabend@gmail.com> > > >> > >> This would apply to net or bpf trees, not -next ones, sorry for the > >> confusion in the [PATCH bpf-next] prefix. > > > > No problem, thanks for the fix! > > > >> Bug was added in 4.14 > > > > Fixes tag is actually slightly different, it would be: > > > > Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks") > > > > Yep. Applied to bpf tree with corrected Fixes tag, Thanks everyone.
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c index 48c33417d13c0ad40154f25aeade0c9b4cafd96a..a927e89dad6e9591066c3a87afc497a196ebd887 100644 --- a/kernel/bpf/sockmap.c +++ b/kernel/bpf/sockmap.c @@ -521,8 +521,8 @@ static struct smap_psock *smap_init_psock(struct sock *sock, static struct bpf_map *sock_map_alloc(union bpf_attr *attr) { struct bpf_stab *stab; - int err = -EINVAL; u64 cost; + int err; if (!capable(CAP_NET_ADMIN)) return ERR_PTR(-EPERM); @@ -547,6 +547,7 @@ static struct bpf_map *sock_map_alloc(union bpf_attr *attr) /* make sure page count doesn't overflow */ cost = (u64) stab->map.max_entries * sizeof(struct sock *); + err = -EINVAL; if (cost >= U32_MAX - PAGE_SIZE) goto free_stab;