pull-request: bpf 2023-11-30

pull-request: bpf 2023-11-30

[Daniel Borkmann]

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(-)

Re: pull-request: bpf 2023-11-30

[patchwork-bot+netdevbpf]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: pull-request: bpf 2023-11-30

[Eric Dumazet]

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(-)

Re: pull-request: bpf 2023-11-30

[Daniel Borkmann]

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

Re: pull-request: bpf 2023-11-30

[John Fastabend]

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

Re: pull-request: bpf 2023-11-30

[Eric Dumazet]

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.

Re: pull-request: bpf 2023-11-30

[Eric Dumazet]

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.

Re: pull-request: bpf 2023-11-30

[John Fastabend]

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.