| Message ID | 20231129234916.16128-1-daniel@iogearbox.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0d47fa5cc91b9c8a0c90833bf1705048b2295714 |
| Headers | show |
| Series | pull-request: bpf 2023-11-30 | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Pull request for net |
| netdev/build_32bit | success | Errors and warnings before: 1200 this patch: 1200 |
| netdev/build_clang | success | Errors and warnings before: 1143 this patch: 1143 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1230 this patch: 1230 |
| netdev/build_clang_rust | success | No Rust files in patch. Skipping build |
Hello: This pull request was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Thu, 30 Nov 2023 00:49:16 +0100 you wrote: > Hi David, hi Jakub, hi Paolo, hi Eric, > > The following pull-request contains BPF updates for your *net* tree. > > We've added 5 non-merge commits during the last 7 day(s) which contain > a total of 10 files changed, 66 insertions(+), 15 deletions(-). > > [...] Here is the summary with links: - pull-request: bpf 2023-11-30 https://git.kernel.org/netdev/net/c/0d47fa5cc91b You are awesome, thank you!
On Thu, Nov 30, 2023 at 12:49 AM Daniel Borkmann <daniel@iogearbox.net> wrote: > > Hi David, hi Jakub, hi Paolo, hi Eric, > > The following pull-request contains BPF updates for your *net* tree. > > We've added 5 non-merge commits during the last 7 day(s) which contain > a total of 10 files changed, 66 insertions(+), 15 deletions(-). > > The main changes are: > > 1) Fix AF_UNIX splat from use after free in BPF sockmap, from John Fastabend. syzbot is not happy with this patch. Would the following fix make sense? diff --git a/net/unix/unix_bpf.c b/net/unix/unix_bpf.c index 7ea7c3a0d0d06224f49ad5f073bf772b9528a30a..58e89361059fbf9d5942c6dd268dd80ac4b57098 100644 --- a/net/unix/unix_bpf.c +++ b/net/unix/unix_bpf.c @@ -168,7 +168,8 @@ int unix_stream_bpf_update_proto(struct sock *sk, struct sk_psock *psock, bool r } sk_pair = unix_peer(sk); - sock_hold(sk_pair); + if (sk_pair) + sock_hold(sk_pair); psock->sk_pair = sk_pair; unix_stream_bpf_check_needs_rebuild(psock->sk_proto); sock_replace_proto(sk, &unix_stream_bpf_prot); > > 2) Fix a syzkaller splat in netdevsim by properly handling offloaded programs (and > not device-bound ones), from Stanislav Fomichev. > > 3) Fix bpf_mem_cache_alloc_flags() to initialize the allocation hint, from Hou Tao. > > 4) Fix netkit by rejecting IFLA_NETKIT_PEER_INFO in changelink, from Daniel Borkmann. > > Please consider pulling these changes from: > > git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git tags/for-netdev > > Thanks a lot! > > Also thanks to reporters, reviewers and testers of commits in this pull-request: > > Jakub Kicinski, Jakub Sitnicki, Nikolay Aleksandrov, Yonghong Song > > ---------------------------------------------------------------- > > The following changes since commit d3fa86b1a7b4cdc4367acacea16b72e0a200b3d7: > > Merge tag 'net-6.7-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net (2023-11-23 10:40:13 -0800) > > are available in the Git repository at: > > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git tags/for-netdev > > for you to fetch changes up to 51354f700d400e55b329361e1386b04695e6e5c1: > > bpf, sockmap: Add af_unix test with both sockets in map (2023-11-30 00:25:25 +0100) > > ---------------------------------------------------------------- > bpf-for-netdev > > ---------------------------------------------------------------- > Daniel Borkmann (1): > netkit: Reject IFLA_NETKIT_PEER_INFO in netkit_change_link > > Hou Tao (1): > bpf: Add missed allocation hint for bpf_mem_cache_alloc_flags() > > John Fastabend (2): > bpf, sockmap: af_unix stream sockets need to hold ref for pair sock > bpf, sockmap: Add af_unix test with both sockets in map > > Stanislav Fomichev (1): > netdevsim: Don't accept device bound programs > > drivers/net/netdevsim/bpf.c | 4 +- > drivers/net/netkit.c | 6 +++ > include/linux/skmsg.h | 1 + > include/net/af_unix.h | 1 + > kernel/bpf/memalloc.c | 2 + > net/core/skmsg.c | 2 + > net/unix/af_unix.c | 2 - > net/unix/unix_bpf.c | 5 +++ > .../selftests/bpf/prog_tests/sockmap_listen.c | 51 +++++++++++++++++----- > .../selftests/bpf/progs/test_sockmap_listen.c | 7 +++ > 10 files changed, 66 insertions(+), 15 deletions(-)
On 11/30/23 3:53 PM, Eric Dumazet wrote: > On Thu, Nov 30, 2023 at 12:49 AM Daniel Borkmann <daniel@iogearbox.net> wrote: >> >> Hi David, hi Jakub, hi Paolo, hi Eric, >> >> The following pull-request contains BPF updates for your *net* tree. >> >> We've added 5 non-merge commits during the last 7 day(s) which contain >> a total of 10 files changed, 66 insertions(+), 15 deletions(-). >> >> The main changes are: >> >> 1) Fix AF_UNIX splat from use after free in BPF sockmap, from John Fastabend. > > syzbot is not happy with this patch. > > Would the following fix make sense? > > diff --git a/net/unix/unix_bpf.c b/net/unix/unix_bpf.c > index 7ea7c3a0d0d06224f49ad5f073bf772b9528a30a..58e89361059fbf9d5942c6dd268dd80ac4b57098 > 100644 > --- a/net/unix/unix_bpf.c > +++ b/net/unix/unix_bpf.c > @@ -168,7 +168,8 @@ int unix_stream_bpf_update_proto(struct sock *sk, > struct sk_psock *psock, bool r > } > > sk_pair = unix_peer(sk); > - sock_hold(sk_pair); > + if (sk_pair) > + sock_hold(sk_pair); > psock->sk_pair = sk_pair; > unix_stream_bpf_check_needs_rebuild(psock->sk_proto); > sock_replace_proto(sk, &unix_stream_bpf_prot); > Oh well :/ Above looks reasonable to me, thanks, but I'll defer to John & Jakub (both Cc'ed) for a final look. Thanks, Daniel
Daniel Borkmann wrote: > On 11/30/23 3:53 PM, Eric Dumazet wrote: > > On Thu, Nov 30, 2023 at 12:49 AM Daniel Borkmann <daniel@iogearbox.net> wrote: > >> > >> Hi David, hi Jakub, hi Paolo, hi Eric, > >> > >> The following pull-request contains BPF updates for your *net* tree. > >> > >> We've added 5 non-merge commits during the last 7 day(s) which contain > >> a total of 10 files changed, 66 insertions(+), 15 deletions(-). > >> > >> The main changes are: > >> > >> 1) Fix AF_UNIX splat from use after free in BPF sockmap, from John Fastabend. > > > > syzbot is not happy with this patch. > > > > Would the following fix make sense? > > > > diff --git a/net/unix/unix_bpf.c b/net/unix/unix_bpf.c > > index 7ea7c3a0d0d06224f49ad5f073bf772b9528a30a..58e89361059fbf9d5942c6dd268dd80ac4b57098 > > 100644 > > --- a/net/unix/unix_bpf.c > > +++ b/net/unix/unix_bpf.c > > @@ -168,7 +168,8 @@ int unix_stream_bpf_update_proto(struct sock *sk, > > struct sk_psock *psock, bool r > > } > > > > sk_pair = unix_peer(sk); > > - sock_hold(sk_pair); > > + if (sk_pair) > > + sock_hold(sk_pair); > > psock->sk_pair = sk_pair; > > unix_stream_bpf_check_needs_rebuild(psock->sk_proto); > > sock_replace_proto(sk, &unix_stream_bpf_prot); > > > > Oh well :/ Above looks reasonable to me, thanks, but I'll defer to John & Jakub (both Cc'ed) > for a final look. > > Thanks, > Daniel Is that sk in LISTEN state by any chance? I can't think why we even allow such a thing for af_unix sockets. Another possible fix would be to block adding these to sockmap at all. But, above should be fine as well so I would just go with that. Eric or Daniel would you like to submit a patch or I can if needed. Thanks, John
On Thu, Nov 30, 2023 at 4:54 PM John Fastabend <john.fastabend@gmail.com> wrote: > > Daniel Borkmann wrote: > > On 11/30/23 3:53 PM, Eric Dumazet wrote: > > > On Thu, Nov 30, 2023 at 12:49 AM Daniel Borkmann <daniel@iogearbox.net> wrote: > > >> > > >> Hi David, hi Jakub, hi Paolo, hi Eric, > > >> > > >> The following pull-request contains BPF updates for your *net* tree. > > >> > > >> We've added 5 non-merge commits during the last 7 day(s) which contain > > >> a total of 10 files changed, 66 insertions(+), 15 deletions(-). > > >> > > >> The main changes are: > > >> > > >> 1) Fix AF_UNIX splat from use after free in BPF sockmap, from John Fastabend. > > > > > > syzbot is not happy with this patch. > > > > > > Would the following fix make sense? > > > > > > diff --git a/net/unix/unix_bpf.c b/net/unix/unix_bpf.c > > > index 7ea7c3a0d0d06224f49ad5f073bf772b9528a30a..58e89361059fbf9d5942c6dd268dd80ac4b57098 > > > 100644 > > > --- a/net/unix/unix_bpf.c > > > +++ b/net/unix/unix_bpf.c > > > @@ -168,7 +168,8 @@ int unix_stream_bpf_update_proto(struct sock *sk, > > > struct sk_psock *psock, bool r > > > } > > > > > > sk_pair = unix_peer(sk); > > > - sock_hold(sk_pair); > > > + if (sk_pair) > > > + sock_hold(sk_pair); > > > psock->sk_pair = sk_pair; > > > unix_stream_bpf_check_needs_rebuild(psock->sk_proto); > > > sock_replace_proto(sk, &unix_stream_bpf_prot); > > > > > > > Oh well :/ Above looks reasonable to me, thanks, but I'll defer to John & Jakub (both Cc'ed) > > for a final look. > > > > Thanks, > > Daniel > > Is that sk in LISTEN state by any chance? I can't think why we even allow such a > thing for af_unix sockets. Another possible fix would be to block adding these > to sockmap at all. > > But, above should be fine as well so I would just go with that. Eric or Daniel > would you like to submit a patch or I can if needed. Here is the repro: # See https://goo.gl/kgGztJ for information about syzkaller reproducers. #{"procs":1,"slowdown":1,"sandbox":"","sandbox_arg":0,"close_fds":false} r0 = socket(0x1, 0x1, 0x0) r1 = bpf$MAP_CREATE(0x0, &(0x7f0000000200)=@base={0xf, 0x4, 0x4, 0x12}, 0x48) bpf$MAP_UPDATE_ELEM(0x2, &(0x7f0000000140)={r1, &(0x7f0000000000), &(0x7f0000000100)=@tcp6=r0}, 0x20) I will release the syzbot report, and send the patch, thanks.
On Thu, Nov 30, 2023 at 5:04 PM Eric Dumazet <edumazet@google.com> wrote: > > Here is the repro: > > # See https://goo.gl/kgGztJ for information about syzkaller reproducers. > #{"procs":1,"slowdown":1,"sandbox":"","sandbox_arg":0,"close_fds":false} > r0 = socket(0x1, 0x1, 0x0) > r1 = bpf$MAP_CREATE(0x0, &(0x7f0000000200)=@base={0xf, 0x4, 0x4, 0x12}, 0x48) > bpf$MAP_UPDATE_ELEM(0x2, &(0x7f0000000140)={r1, &(0x7f0000000000), > &(0x7f0000000100)=@tcp6=r0}, 0x20) > > I will release the syzbot report, and send the patch, thanks. Actually I will release the syzbot report, and let you work on a fix, perhaps as you pointed out we could be more restrictive.
Eric Dumazet wrote: > On Thu, Nov 30, 2023 at 5:04 PM Eric Dumazet <edumazet@google.com> wrote: > > > > > Here is the repro: > > > > # See https://goo.gl/kgGztJ for information about syzkaller reproducers. > > #{"procs":1,"slowdown":1,"sandbox":"","sandbox_arg":0,"close_fds":false} > > r0 = socket(0x1, 0x1, 0x0) > > r1 = bpf$MAP_CREATE(0x0, &(0x7f0000000200)=@base={0xf, 0x4, 0x4, 0x12}, 0x48) > > bpf$MAP_UPDATE_ELEM(0x2, &(0x7f0000000140)={r1, &(0x7f0000000000), > > &(0x7f0000000100)=@tcp6=r0}, 0x20) > > > > I will release the syzbot report, and send the patch, thanks. > > Actually I will release the syzbot report, and let you work on a fix, > perhaps as you pointed out we could be more restrictive. Thanks, I think just fixing the null ptr deref is probably not enough because that socket could be connected() after that and then we get back to the original issue where we don't hold a ref on the peer sock. I'll just block adding non established af_unix socks to the map and if someone wants to support unconnected sockets they can add support for it then.
Hi David, hi Jakub, hi Paolo, hi Eric, The following pull-request contains BPF updates for your *net* tree. We've added 5 non-merge commits during the last 7 day(s) which contain a total of 10 files changed, 66 insertions(+), 15 deletions(-). The main changes are: 1) Fix AF_UNIX splat from use after free in BPF sockmap, from John Fastabend. 2) Fix a syzkaller splat in netdevsim by properly handling offloaded programs (and not device-bound ones), from Stanislav Fomichev. 3) Fix bpf_mem_cache_alloc_flags() to initialize the allocation hint, from Hou Tao. 4) Fix netkit by rejecting IFLA_NETKIT_PEER_INFO in changelink, from Daniel Borkmann. Please consider pulling these changes from: git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git tags/for-netdev Thanks a lot! Also thanks to reporters, reviewers and testers of commits in this pull-request: Jakub Kicinski, Jakub Sitnicki, Nikolay Aleksandrov, Yonghong Song ---------------------------------------------------------------- The following changes since commit d3fa86b1a7b4cdc4367acacea16b72e0a200b3d7: Merge tag 'net-6.7-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net (2023-11-23 10:40:13 -0800) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git tags/for-netdev for you to fetch changes up to 51354f700d400e55b329361e1386b04695e6e5c1: bpf, sockmap: Add af_unix test with both sockets in map (2023-11-30 00:25:25 +0100) ---------------------------------------------------------------- bpf-for-netdev ---------------------------------------------------------------- Daniel Borkmann (1): netkit: Reject IFLA_NETKIT_PEER_INFO in netkit_change_link Hou Tao (1): bpf: Add missed allocation hint for bpf_mem_cache_alloc_flags() John Fastabend (2): bpf, sockmap: af_unix stream sockets need to hold ref for pair sock bpf, sockmap: Add af_unix test with both sockets in map Stanislav Fomichev (1): netdevsim: Don't accept device bound programs drivers/net/netdevsim/bpf.c | 4 +- drivers/net/netkit.c | 6 +++ include/linux/skmsg.h | 1 + include/net/af_unix.h | 1 + kernel/bpf/memalloc.c | 2 + net/core/skmsg.c | 2 + net/unix/af_unix.c | 2 - net/unix/unix_bpf.c | 5 +++ .../selftests/bpf/prog_tests/sockmap_listen.c | 51 +++++++++++++++++----- .../selftests/bpf/progs/test_sockmap_listen.c | 7 +++ 10 files changed, 66 insertions(+), 15 deletions(-)