diff mbox series

[bpf-next,v3,03/10] bpf: Allow read access to addr_len from cgroup sockaddr programs

Message ID 20230421162718.440230-4-daan.j.demeyer@gmail.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series Add cgroup sockaddr hooks for unix sockets | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1350 this patch: 1350
netdev/cc_maintainers warning 17 maintainers not CCed: davem@davemloft.net andrii@kernel.org pabeni@redhat.com dsahern@kernel.org song@kernel.org sdf@google.com willemdebruijn.kernel@gmail.com yhs@fb.com haoluo@google.com kuba@kernel.org edumazet@google.com daniel@iogearbox.net john.fastabend@gmail.com kpsingh@kernel.org jolsa@kernel.org netdev@vger.kernel.org ast@kernel.org
netdev/build_clang success Errors and warnings before: 147 this patch: 147
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1362 this patch: 1362
netdev/checkpatch warning CHECK: Macro argument 'bind_flags' may be better as '(bind_flags)' to avoid precedence issues CHECK: No space is necessary after a cast WARNING: line length of 81 exceeds 80 columns WARNING: line length of 82 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: line length of 84 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 12 this patch: 12
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-7 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-8 fail Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 fail Logs for test_maps on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-10 fail Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-11 fail Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-12 fail Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-13 fail Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 fail Logs for test_progs on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-15 fail Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 fail Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 fail Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-18 fail Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 fail Logs for test_progs_no_alu32 on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-20 fail Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-21 fail Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 fail Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_progs_no_alu32_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for test_progs_parallel on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-29 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-31 fail Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-32 fail Logs for test_verifier on aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-33 fail Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-34 fail Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-35 fail Logs for test_verifier on x86_64 with llvm-16

Commit Message

Daan De Meyer April 21, 2023, 4:27 p.m. UTC 
As prep for adding unix socket support to the cgroup sockaddr hooks,
let's expose the sockaddr addrlen in bpf_sock_addr_kern. While not
important for AF_INET or AF_INET6, the sockaddr length is important
when working with AF_UNIX sockaddrs as the size of the sockaddr cannot
be determined just from the address family or the sockaddr's contents.

__cgroup_bpf_run_filter_sock_addr() is modified to return the addr_len
in preparation for adding unix socket support for which we'll need to
return the modified address length.

Signed-off-by: Daan De Meyer <daan.j.demeyer@gmail.com>
---
 include/linux/bpf-cgroup.h | 73 +++++++++++++++++++-------------------
 include/linux/filter.h     |  1 +
 kernel/bpf/cgroup.c        | 16 +++++++--
 net/ipv4/af_inet.c         |  8 ++---
 net/ipv4/ping.c            |  8 ++++-
 net/ipv4/tcp_ipv4.c        |  8 ++++-
 net/ipv4/udp.c             | 17 ++++++---
 net/ipv6/af_inet6.c        |  8 ++---
 net/ipv6/ping.c            |  8 ++++-
 net/ipv6/tcp_ipv6.c        |  8 ++++-
 net/ipv6/udp.c             | 14 ++++++--
 11 files changed, 111 insertions(+), 58 deletions(-)

Comments

Alexei Starovoitov April 21, 2023, 8:55 p.m. UTC | #1 
On Fri, Apr 21, 2023 at 06:27:11PM +0200, Daan De Meyer wrote:
>   *
>   * This function will return %-EPERM if an attached program is found and
> - * returned value != 1 during execution. In all other cases, 0 is returned.
> + * returned value != 1 during execution. In all other cases, the new address
> + * length of the sockaddr is returned.
>   */
>  int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
>  				      struct sockaddr *uaddr,
> +				      u32 uaddrlen,
>  				      enum cgroup_bpf_attach_type atype,
>  				      void *t_ctx,
>  				      u32 *flags)
> @@ -1469,9 +1472,11 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
>  		.sk = sk,
>  		.uaddr = uaddr,
>  		.t_ctx = t_ctx,
> +		.uaddrlen = uaddrlen,
>  	};
>  	struct sockaddr_storage unspec;
>  	struct cgroup *cgrp;
> +	int ret;
>  
>  	/* Check socket family since not all sockets represent network
>  	 * endpoint (e.g. AF_UNIX).
> @@ -1482,11 +1487,16 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
>  	if (!ctx.uaddr) {
>  		memset(&unspec, 0, sizeof(unspec));
>  		ctx.uaddr = (struct sockaddr *)&unspec;
> +		ctx.uaddrlen = sizeof(unspec);
>  	}
>  
>  	cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
> -	return bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
> -				     0, flags);
> +	ret = bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
> +				    0, flags);
> +	if (ret)
> +		return ret;
> +
> +	return (int) ctx.uaddrlen;

But that is big behavioral change..
instead of 0 or 1 now it will be sizeof(unspec) or 1?
That will surely break some of the __cgroup_bpf_run_filter_sock_addr callers.
Daan De Meyer April 24, 2023, 1:58 p.m. UTC | #2 
> On Fri, Apr 21, 2023 at 06:27:11PM +0200, Daan De Meyer wrote:
> >   *
> >   * This function will return %-EPERM if an attached program is found and
> > - * returned value != 1 during execution. In all other cases, 0 is returned.
> > + * returned value != 1 during execution. In all other cases, the new address
> > + * length of the sockaddr is returned.
> >   */
> >  int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> >                                     struct sockaddr *uaddr,
> > +                                   u32 uaddrlen,
> >                                     enum cgroup_bpf_attach_type atype,
> >                                     void *t_ctx,
> >                                     u32 *flags)
> > @@ -1469,9 +1472,11 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> >               .sk = sk,
> >               .uaddr = uaddr,
> >               .t_ctx = t_ctx,
> > +             .uaddrlen = uaddrlen,
> >       };
> >       struct sockaddr_storage unspec;
> >       struct cgroup *cgrp;
> > +     int ret;
> >
> >       /* Check socket family since not all sockets represent network
> >        * endpoint (e.g. AF_UNIX).
> > @@ -1482,11 +1487,16 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> >       if (!ctx.uaddr) {
> >               memset(&unspec, 0, sizeof(unspec));
> >               ctx.uaddr = (struct sockaddr *)&unspec;
> > +             ctx.uaddrlen = sizeof(unspec);
> >       }
> >
> >       cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
> > -     return bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
> > -                                  0, flags);
> > +     ret = bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
> > +                                 0, flags);
> > +     if (ret)
> > +             return ret;
> > +
> > +     return (int) ctx.uaddrlen;
>
> But that is big behavioral change..
> instead of 0 or 1 now it will be sizeof(unspec) or 1?
> That will surely break some of the __cgroup_bpf_run_filter_sock_addr callers.

It will now always return the size of the addrlen as set by the bpf
program or the original addrlen if the bpf program did not change it.
I modified all the callers of __cgroup_bpf_run_filter_sock_addr() to
ignore the returned addrlen so as to not introduce any breakages. Only
when unix socket support is introduced in the following patch do we
actually start making use of the addrlen returned by
__cgroup_bpf_run_filter_sock_addr(). Alternatively, I can pass in an
optional pointer to __cgroup_bpf_run_filter_sock_addr() which is set
to the modified addrlen if it is provided and then only make use of
this in af_unix.c, but I figure since we're already returning an int,
we can use that to propagate the modified address length as well.

bpf_prog_run_array_cg() will return either 0 or -EPERM so we'll either
return an error if an error occurs or the modified address length if
no error occurs.

For the default size of the address length if none is provided, I used
sizeof(unspec) since that's the size of the memory we provide to the
BPF program, but I suppose that zero could also make sense here, to
indicate that we're providing an empty address. Let me know which one
is preferred.


On Fri, 21 Apr 2023 at 22:55, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Apr 21, 2023 at 06:27:11PM +0200, Daan De Meyer wrote:
> >   *
> >   * This function will return %-EPERM if an attached program is found and
> > - * returned value != 1 during execution. In all other cases, 0 is returned.
> > + * returned value != 1 during execution. In all other cases, the new address
> > + * length of the sockaddr is returned.
> >   */
> >  int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> >                                     struct sockaddr *uaddr,
> > +                                   u32 uaddrlen,
> >                                     enum cgroup_bpf_attach_type atype,
> >                                     void *t_ctx,
> >                                     u32 *flags)
> > @@ -1469,9 +1472,11 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> >               .sk = sk,
> >               .uaddr = uaddr,
> >               .t_ctx = t_ctx,
> > +             .uaddrlen = uaddrlen,
> >       };
> >       struct sockaddr_storage unspec;
> >       struct cgroup *cgrp;
> > +     int ret;
> >
> >       /* Check socket family since not all sockets represent network
> >        * endpoint (e.g. AF_UNIX).
> > @@ -1482,11 +1487,16 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> >       if (!ctx.uaddr) {
> >               memset(&unspec, 0, sizeof(unspec));
> >               ctx.uaddr = (struct sockaddr *)&unspec;
> > +             ctx.uaddrlen = sizeof(unspec);
> >       }
> >
> >       cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
> > -     return bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
> > -                                  0, flags);
> > +     ret = bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
> > +                                 0, flags);
> > +     if (ret)
> > +             return ret;
> > +
> > +     return (int) ctx.uaddrlen;
>
> But that is big behavioral change..
> instead of 0 or 1 now it will be sizeof(unspec) or 1?
> That will surely break some of the __cgroup_bpf_run_filter_sock_addr callers.
Alexei Starovoitov April 26, 2023, 12:05 a.m. UTC | #3 
On Mon, Apr 24, 2023 at 03:58:24PM +0200, Daan De Meyer wrote:
> > On Fri, Apr 21, 2023 at 06:27:11PM +0200, Daan De Meyer wrote:
> > >   *
> > >   * This function will return %-EPERM if an attached program is found and
> > > - * returned value != 1 during execution. In all other cases, 0 is returned.
> > > + * returned value != 1 during execution. In all other cases, the new address
> > > + * length of the sockaddr is returned.
> > >   */
> > >  int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> > >                                     struct sockaddr *uaddr,
> > > +                                   u32 uaddrlen,
> > >                                     enum cgroup_bpf_attach_type atype,
> > >                                     void *t_ctx,
> > >                                     u32 *flags)
> > > @@ -1469,9 +1472,11 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> > >               .sk = sk,
> > >               .uaddr = uaddr,
> > >               .t_ctx = t_ctx,
> > > +             .uaddrlen = uaddrlen,
> > >       };
> > >       struct sockaddr_storage unspec;
> > >       struct cgroup *cgrp;
> > > +     int ret;
> > >
> > >       /* Check socket family since not all sockets represent network
> > >        * endpoint (e.g. AF_UNIX).
> > > @@ -1482,11 +1487,16 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> > >       if (!ctx.uaddr) {
> > >               memset(&unspec, 0, sizeof(unspec));
> > >               ctx.uaddr = (struct sockaddr *)&unspec;
> > > +             ctx.uaddrlen = sizeof(unspec);
> > >       }
> > >
> > >       cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
> > > -     return bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
> > > -                                  0, flags);
> > > +     ret = bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
> > > +                                 0, flags);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     return (int) ctx.uaddrlen;
> >
> > But that is big behavioral change..
> > instead of 0 or 1 now it will be sizeof(unspec) or 1?
> > That will surely break some of the __cgroup_bpf_run_filter_sock_addr callers.
> 
> It will now always return the size of the addrlen as set by the bpf
> program or the original addrlen if the bpf program did not change it.
> I modified all the callers of __cgroup_bpf_run_filter_sock_addr() to
> ignore the returned addrlen so as to not introduce any breakages. Only
> when unix socket support is introduced in the following patch do we
> actually start making use of the addrlen returned by
> __cgroup_bpf_run_filter_sock_addr(). Alternatively, I can pass in an
> optional pointer to __cgroup_bpf_run_filter_sock_addr() which is set
> to the modified addrlen if it is provided and then only make use of
> this in af_unix.c, but I figure since we're already returning an int,
> we can use that to propagate the modified address length as well.
> 
> bpf_prog_run_array_cg() will return either 0 or -EPERM so we'll either
> return an error if an error occurs or the modified address length if
> no error occurs.
> 
> For the default size of the address length if none is provided, I used
> sizeof(unspec) since that's the size of the memory we provide to the
> BPF program, but I suppose that zero could also make sense here, to
> indicate that we're providing an empty address. Let me know which one
> is preferred.

I still don't understand how it's possible to modify the callers to
have correct behavior.

- return bpf_prog_run_array_cg();
+ ret = bpf_prog_run_array_cg();
+       if (ret)
+               return ret;
+
+       return (int) ctx.uaddrlen;

It used to return 0 or 1.
Now 1 is indistinguishable between 1 from prog and 0 from prog, but uaddrlen == 1.
I don't see how callers can deal with that.
Daan De Meyer April 26, 2023, 1:46 p.m. UTC | #4 
> On Mon, Apr 24, 2023 at 03:58:24PM +0200, Daan De Meyer wrote:
> > > On Fri, Apr 21, 2023 at 06:27:11PM +0200, Daan De Meyer wrote:
> > > >   *
> > > >   * This function will return %-EPERM if an attached program is found and
> > > > - * returned value != 1 during execution. In all other cases, 0 is returned.
> > > > + * returned value != 1 during execution. In all other cases, the new address
> > > > + * length of the sockaddr is returned.
> > > >   */
> > > >  int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> > > >                                     struct sockaddr *uaddr,
> > > > +                                   u32 uaddrlen,
> > > >                                     enum cgroup_bpf_attach_type atype,
> > > >                                     void *t_ctx,
> > > >                                     u32 *flags)
> > > > @@ -1469,9 +1472,11 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> > > >               .sk = sk,
> > > >               .uaddr = uaddr,
> > > >               .t_ctx = t_ctx,
> > > > +             .uaddrlen = uaddrlen,
> > > >       };
> > > >       struct sockaddr_storage unspec;
> > > >       struct cgroup *cgrp;
> > > > +     int ret;
> > > >
> > > >       /* Check socket family since not all sockets represent network
> > > >        * endpoint (e.g. AF_UNIX).
> > > > @@ -1482,11 +1487,16 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> > > >       if (!ctx.uaddr) {
> > > >               memset(&unspec, 0, sizeof(unspec));
> > > >               ctx.uaddr = (struct sockaddr *)&unspec;
> > > > +             ctx.uaddrlen = sizeof(unspec);
> > > >       }
> > > >
> > > >       cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
> > > > -     return bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
> > > > -                                  0, flags);
> > > > +     ret = bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
> > > > +                                 0, flags);
> > > > +     if (ret)
> > > > +             return ret;
> > > > +
> > > > +     return (int) ctx.uaddrlen;
> > >
> > > But that is big behavioral change..
> > > instead of 0 or 1 now it will be sizeof(unspec) or 1?
> > > That will surely break some of the __cgroup_bpf_run_filter_sock_addr callers.
> >
> > It will now always return the size of the addrlen as set by the bpf
> > program or the original addrlen if the bpf program did not change it.
> > I modified all the callers of __cgroup_bpf_run_filter_sock_addr() to
> > ignore the returned addrlen so as to not introduce any breakages. Only
> > when unix socket support is introduced in the following patch do we
> > actually start making use of the addrlen returned by
> > __cgroup_bpf_run_filter_sock_addr(). Alternatively, I can pass in an
> > optional pointer to __cgroup_bpf_run_filter_sock_addr() which is set
> > to the modified addrlen if it is provided and then only make use of
> > this in af_unix.c, but I figure since we're already returning an int,
> > we can use that to propagate the modified address length as well.
> >
> > bpf_prog_run_array_cg() will return either 0 or -EPERM so we'll either
> > return an error if an error occurs or the modified address length if
> > no error occurs.
> >
> > For the default size of the address length if none is provided, I used
> > sizeof(unspec) since that's the size of the memory we provide to the
> > BPF program, but I suppose that zero could also make sense here, to
> > indicate that we're providing an empty address. Let me know which one
> > is preferred.
>
> I still don't understand how it's possible to modify the callers to
> have correct behavior.
>
> - return bpf_prog_run_array_cg();
> + ret = bpf_prog_run_array_cg();
> +       if (ret)
> +               return ret;
> +
> +       return (int) ctx.uaddrlen;
>
> It used to return 0 or 1.
> Now 1 is indistinguishable between 1 from prog and 0 from prog, but uaddrlen == 1.
> I don't see how callers can deal with that.

I think I'm missing something crucial somewhere. If I understand
bpf_prog_run_array_cg(() correctly, when called with retval = 0, it
will return -EPERM on failure and 0 on success. If I understand that
correctly, with this change, we'll still return -EPERM on failure but
instead of returning 0 on success, we'll now return the address
length. Am I misunderstanding how bpf_prog_run_array_cg() behaves in
this case?


On Wed, 26 Apr 2023 at 02:06, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Apr 24, 2023 at 03:58:24PM +0200, Daan De Meyer wrote:
> > > On Fri, Apr 21, 2023 at 06:27:11PM +0200, Daan De Meyer wrote:
> > > >   *
> > > >   * This function will return %-EPERM if an attached program is found and
> > > > - * returned value != 1 during execution. In all other cases, 0 is returned.
> > > > + * returned value != 1 during execution. In all other cases, the new address
> > > > + * length of the sockaddr is returned.
> > > >   */
> > > >  int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> > > >                                     struct sockaddr *uaddr,
> > > > +                                   u32 uaddrlen,
> > > >                                     enum cgroup_bpf_attach_type atype,
> > > >                                     void *t_ctx,
> > > >                                     u32 *flags)
> > > > @@ -1469,9 +1472,11 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> > > >               .sk = sk,
> > > >               .uaddr = uaddr,
> > > >               .t_ctx = t_ctx,
> > > > +             .uaddrlen = uaddrlen,
> > > >       };
> > > >       struct sockaddr_storage unspec;
> > > >       struct cgroup *cgrp;
> > > > +     int ret;
> > > >
> > > >       /* Check socket family since not all sockets represent network
> > > >        * endpoint (e.g. AF_UNIX).
> > > > @@ -1482,11 +1487,16 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> > > >       if (!ctx.uaddr) {
> > > >               memset(&unspec, 0, sizeof(unspec));
> > > >               ctx.uaddr = (struct sockaddr *)&unspec;
> > > > +             ctx.uaddrlen = sizeof(unspec);
> > > >       }
> > > >
> > > >       cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
> > > > -     return bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
> > > > -                                  0, flags);
> > > > +     ret = bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
> > > > +                                 0, flags);
> > > > +     if (ret)
> > > > +             return ret;
> > > > +
> > > > +     return (int) ctx.uaddrlen;
> > >
> > > But that is big behavioral change..
> > > instead of 0 or 1 now it will be sizeof(unspec) or 1?
> > > That will surely break some of the __cgroup_bpf_run_filter_sock_addr callers.
> >
> > It will now always return the size of the addrlen as set by the bpf
> > program or the original addrlen if the bpf program did not change it.
> > I modified all the callers of __cgroup_bpf_run_filter_sock_addr() to
> > ignore the returned addrlen so as to not introduce any breakages. Only
> > when unix socket support is introduced in the following patch do we
> > actually start making use of the addrlen returned by
> > __cgroup_bpf_run_filter_sock_addr(). Alternatively, I can pass in an
> > optional pointer to __cgroup_bpf_run_filter_sock_addr() which is set
> > to the modified addrlen if it is provided and then only make use of
> > this in af_unix.c, but I figure since we're already returning an int,
> > we can use that to propagate the modified address length as well.
> >
> > bpf_prog_run_array_cg() will return either 0 or -EPERM so we'll either
> > return an error if an error occurs or the modified address length if
> > no error occurs.
> >
> > For the default size of the address length if none is provided, I used
> > sizeof(unspec) since that's the size of the memory we provide to the
> > BPF program, but I suppose that zero could also make sense here, to
> > indicate that we're providing an empty address. Let me know which one
> > is preferred.
>
> I still don't understand how it's possible to modify the callers to
> have correct behavior.
>
> - return bpf_prog_run_array_cg();
> + ret = bpf_prog_run_array_cg();
> +       if (ret)
> +               return ret;
> +
> +       return (int) ctx.uaddrlen;
>
> It used to return 0 or 1.
> Now 1 is indistinguishable between 1 from prog and 0 from prog, but uaddrlen == 1.
> I don't see how callers can deal with that.
Martin KaFai Lau April 26, 2023, 10:07 p.m. UTC | #5 
On 4/21/23 9:27 AM, Daan De Meyer wrote:
> As prep for adding unix socket support to the cgroup sockaddr hooks,
> let's expose the sockaddr addrlen in bpf_sock_addr_kern. While not
> important for AF_INET or AF_INET6, the sockaddr length is important
> when working with AF_UNIX sockaddrs as the size of the sockaddr cannot
> be determined just from the address family or the sockaddr's contents.
> 
> __cgroup_bpf_run_filter_sock_addr() is modified to return the addr_len
> in preparation for adding unix socket support for which we'll need to
> return the modified address length.
> 
> Signed-off-by: Daan De Meyer <daan.j.demeyer@gmail.com>
> ---
>   include/linux/bpf-cgroup.h | 73 +++++++++++++++++++-------------------
>   include/linux/filter.h     |  1 +
>   kernel/bpf/cgroup.c        | 16 +++++++--
>   net/ipv4/af_inet.c         |  8 ++---
>   net/ipv4/ping.c            |  8 ++++-
>   net/ipv4/tcp_ipv4.c        |  8 ++++-
>   net/ipv4/udp.c             | 17 ++++++---
>   net/ipv6/af_inet6.c        |  8 ++---
>   net/ipv6/ping.c            |  8 ++++-
>   net/ipv6/tcp_ipv6.c        |  8 ++++-
>   net/ipv6/udp.c             | 14 ++++++--
>   11 files changed, 111 insertions(+), 58 deletions(-)
> 
> diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> index 57e9e109257e..f3f5adf3881f 100644
> --- a/include/linux/bpf-cgroup.h
> +++ b/include/linux/bpf-cgroup.h
> @@ -120,6 +120,7 @@ int __cgroup_bpf_run_filter_sk(struct sock *sk,
>   
>   int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
>   				      struct sockaddr *uaddr,
> +				      u32 uaddrlen,

If the bpf_sock_addr_set() kfunc can only change the sin[6]_addr and unix_path 
(the comment in patch 5), the "u32 uaddrlen" can be changed to "u32 *uaddrlen" 
here. The new unix_path length can be passed back to af_unix.c in "*uaddrlen". 
The inet[6] code path can just pass NULL and most of the code churn in this 
patch will no longer be needed?
diff mbox series

Patch

diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 57e9e109257e..f3f5adf3881f 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -120,6 +120,7 @@  int __cgroup_bpf_run_filter_sk(struct sock *sk,
 
 int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
 				      struct sockaddr *uaddr,
+				      u32 uaddrlen,
 				      enum cgroup_bpf_attach_type atype,
 				      void *t_ctx,
 				      u32 *flags);
@@ -230,22 +231,22 @@  static inline bool cgroup_bpf_sock_enabled(struct sock *sk,
 #define BPF_CGROUP_RUN_PROG_INET6_POST_BIND(sk)				       \
 	BPF_CGROUP_RUN_SK_PROG(sk, CGROUP_INET6_POST_BIND)
 
-#define BPF_CGROUP_RUN_SA_PROG(sk, uaddr, atype)				       \
+#define BPF_CGROUP_RUN_SA_PROG(sk, uaddr, uaddrlen, atype)		       \
 ({									       \
 	int __ret = 0;							       \
 	if (cgroup_bpf_enabled(atype))					       \
-		__ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, atype,     \
-							  NULL, NULL);	       \
+		__ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, uaddrlen, \
+							  atype, NULL, NULL);  \
 	__ret;								       \
 })
 
-#define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, atype, t_ctx)		       \
+#define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, atype, t_ctx)	       \
 ({									       \
 	int __ret = 0;							       \
 	if (cgroup_bpf_enabled(atype))	{				       \
 		lock_sock(sk);						       \
-		__ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, atype,     \
-							  t_ctx, NULL);	       \
+		__ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, uaddrlen, \
+							  atype, t_ctx, NULL); \
 		release_sock(sk);					       \
 	}								       \
 	__ret;								       \
@@ -256,14 +257,14 @@  static inline bool cgroup_bpf_sock_enabled(struct sock *sk,
  * (at bit position 0) is to indicate CAP_NET_BIND_SERVICE capability check
  * should be bypassed (BPF_RET_BIND_NO_CAP_NET_BIND_SERVICE).
  */
-#define BPF_CGROUP_RUN_PROG_INET_BIND_LOCK(sk, uaddr, atype, bind_flags)	       \
+#define BPF_CGROUP_RUN_PROG_INET_BIND_LOCK(sk, uaddr, uaddrlen, atype, bind_flags) \
 ({									       \
 	u32 __flags = 0;						       \
 	int __ret = 0;							       \
 	if (cgroup_bpf_enabled(atype))	{				       \
 		lock_sock(sk);						       \
-		__ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, atype,     \
-							  NULL, &__flags);     \
+		__ret = __cgroup_bpf_run_filter_sock_addr(sk, uaddr, uaddrlen, \
+							  atype, NULL, &__flags); \
 		release_sock(sk);					       \
 		if (__flags & BPF_RET_BIND_NO_CAP_NET_BIND_SERVICE)	       \
 			*bind_flags |= BIND_NO_CAP_NET_BIND_SERVICE;	       \
@@ -276,29 +277,29 @@  static inline bool cgroup_bpf_sock_enabled(struct sock *sk,
 	  cgroup_bpf_enabled(CGROUP_INET6_CONNECT)) &&		       \
 	 (sk)->sk_prot->pre_connect)
 
-#define BPF_CGROUP_RUN_PROG_INET4_CONNECT(sk, uaddr)			       \
-	BPF_CGROUP_RUN_SA_PROG(sk, uaddr, CGROUP_INET4_CONNECT)
+#define BPF_CGROUP_RUN_PROG_INET4_CONNECT(sk, uaddr, uaddrlen)			\
+	BPF_CGROUP_RUN_SA_PROG(sk, uaddr, uaddrlen, CGROUP_INET4_CONNECT)
 
-#define BPF_CGROUP_RUN_PROG_INET6_CONNECT(sk, uaddr)			       \
-	BPF_CGROUP_RUN_SA_PROG(sk, uaddr, CGROUP_INET6_CONNECT)
+#define BPF_CGROUP_RUN_PROG_INET6_CONNECT(sk, uaddr, uaddrlen)			\
+	BPF_CGROUP_RUN_SA_PROG(sk, uaddr, uaddrlen, CGROUP_INET6_CONNECT)
 
-#define BPF_CGROUP_RUN_PROG_INET4_CONNECT_LOCK(sk, uaddr)		       \
-	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, CGROUP_INET4_CONNECT, NULL)
+#define BPF_CGROUP_RUN_PROG_INET4_CONNECT_LOCK(sk, uaddr, uaddrlen)		\
+	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, CGROUP_INET4_CONNECT, NULL)
 
-#define BPF_CGROUP_RUN_PROG_INET6_CONNECT_LOCK(sk, uaddr)		       \
-	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, CGROUP_INET6_CONNECT, NULL)
+#define BPF_CGROUP_RUN_PROG_INET6_CONNECT_LOCK(sk, uaddr, uaddrlen)		\
+	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, CGROUP_INET6_CONNECT, NULL)
 
-#define BPF_CGROUP_RUN_PROG_UDP4_SENDMSG_LOCK(sk, uaddr, t_ctx)		       \
-	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, CGROUP_UDP4_SENDMSG, t_ctx)
+#define BPF_CGROUP_RUN_PROG_UDP4_SENDMSG_LOCK(sk, uaddr, uaddrlen, t_ctx)	\
+	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, CGROUP_UDP4_SENDMSG, t_ctx)
 
-#define BPF_CGROUP_RUN_PROG_UDP6_SENDMSG_LOCK(sk, uaddr, t_ctx)		       \
-	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, CGROUP_UDP6_SENDMSG, t_ctx)
+#define BPF_CGROUP_RUN_PROG_UDP6_SENDMSG_LOCK(sk, uaddr, uaddrlen, t_ctx)	\
+	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, CGROUP_UDP6_SENDMSG, t_ctx)
 
-#define BPF_CGROUP_RUN_PROG_UDP4_RECVMSG_LOCK(sk, uaddr)			\
-	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, CGROUP_UDP4_RECVMSG, NULL)
+#define BPF_CGROUP_RUN_PROG_UDP4_RECVMSG_LOCK(sk, uaddr, uaddrlen)		\
+	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, CGROUP_UDP4_RECVMSG, NULL)
 
-#define BPF_CGROUP_RUN_PROG_UDP6_RECVMSG_LOCK(sk, uaddr)			\
-	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, CGROUP_UDP6_RECVMSG, NULL)
+#define BPF_CGROUP_RUN_PROG_UDP6_RECVMSG_LOCK(sk, uaddr, uaddrlen)		\
+	BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, CGROUP_UDP6_RECVMSG, NULL)
 
 /* The SOCK_OPS"_SK" macro should be used when sock_ops->sk is not a
  * fullsock and its parent fullsock cannot be traced by
@@ -477,24 +478,24 @@  static inline int bpf_percpu_cgroup_storage_update(struct bpf_map *map,
 }
 
 #define cgroup_bpf_enabled(atype) (0)
-#define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, atype, t_ctx) ({ 0; })
-#define BPF_CGROUP_RUN_SA_PROG(sk, uaddr, atype) ({ 0; })
+#define BPF_CGROUP_RUN_SA_PROG_LOCK(sk, uaddr, uaddrlen, atype, t_ctx) ({ 0; })
+#define BPF_CGROUP_RUN_SA_PROG(sk, uaddr, uaddrlen, atype) ({ 0; })
 #define BPF_CGROUP_PRE_CONNECT_ENABLED(sk) (0)
 #define BPF_CGROUP_RUN_PROG_INET_INGRESS(sk,skb) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_INET_EGRESS(sk,skb) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_INET_SOCK(sk) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_INET_SOCK_RELEASE(sk) ({ 0; })
-#define BPF_CGROUP_RUN_PROG_INET_BIND_LOCK(sk, uaddr, atype, flags) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_INET_BIND_LOCK(sk, uaddr, uaddrlen, atype, flags) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_INET4_POST_BIND(sk) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_INET6_POST_BIND(sk) ({ 0; })
-#define BPF_CGROUP_RUN_PROG_INET4_CONNECT(sk, uaddr) ({ 0; })
-#define BPF_CGROUP_RUN_PROG_INET4_CONNECT_LOCK(sk, uaddr) ({ 0; })
-#define BPF_CGROUP_RUN_PROG_INET6_CONNECT(sk, uaddr) ({ 0; })
-#define BPF_CGROUP_RUN_PROG_INET6_CONNECT_LOCK(sk, uaddr) ({ 0; })
-#define BPF_CGROUP_RUN_PROG_UDP4_SENDMSG_LOCK(sk, uaddr, t_ctx) ({ 0; })
-#define BPF_CGROUP_RUN_PROG_UDP6_SENDMSG_LOCK(sk, uaddr, t_ctx) ({ 0; })
-#define BPF_CGROUP_RUN_PROG_UDP4_RECVMSG_LOCK(sk, uaddr) ({ 0; })
-#define BPF_CGROUP_RUN_PROG_UDP6_RECVMSG_LOCK(sk, uaddr) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_INET4_CONNECT(sk, uaddr, uaddrlen) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_INET4_CONNECT_LOCK(sk, uaddr, uaddrlen) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_INET6_CONNECT(sk, uaddr, uaddrlen) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_INET6_CONNECT_LOCK(sk, uaddr, uaddrlen) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_UDP4_SENDMSG_LOCK(sk, uaddr, uaddrlen, t_ctx) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_UDP6_SENDMSG_LOCK(sk, uaddr, uaddrlen, t_ctx) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_UDP4_RECVMSG_LOCK(sk, uaddr, uaddrlen) ({ 0; })
+#define BPF_CGROUP_RUN_PROG_UDP6_RECVMSG_LOCK(sk, uaddr, uaddrlen) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_SOCK_OPS(sock_ops) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(atype, major, minor, access) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_SYSCTL(head,table,write,buf,count,pos) ({ 0; })
diff --git a/include/linux/filter.h b/include/linux/filter.h
index bbce89937fde..89e891003c03 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1294,6 +1294,7 @@  struct bpf_sock_addr_kern {
 	 */
 	u64 tmp_reg;
 	void *t_ctx;	/* Attach type specific context. */
+	u32 uaddrlen;
 };
 
 struct bpf_sock_ops_kern {
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 53edb8ad2471..c2191f0e138e 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1449,6 +1449,7 @@  EXPORT_SYMBOL(__cgroup_bpf_run_filter_sk);
  *                                       provided by user sockaddr
  * @sk: sock struct that will use sockaddr
  * @uaddr: sockaddr struct provided by user
+ * @uaddrlen: Size of the sockaddr struct provided by user
  * @type: The type of program to be executed
  * @t_ctx: Pointer to attach type specific context
  * @flags: Pointer to u32 which contains higher bits of BPF program
@@ -1457,10 +1458,12 @@  EXPORT_SYMBOL(__cgroup_bpf_run_filter_sk);
  * socket is expected to be of type INET or INET6.
  *
  * This function will return %-EPERM if an attached program is found and
- * returned value != 1 during execution. In all other cases, 0 is returned.
+ * returned value != 1 during execution. In all other cases, the new address
+ * length of the sockaddr is returned.
  */
 int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
 				      struct sockaddr *uaddr,
+				      u32 uaddrlen,
 				      enum cgroup_bpf_attach_type atype,
 				      void *t_ctx,
 				      u32 *flags)
@@ -1469,9 +1472,11 @@  int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
 		.sk = sk,
 		.uaddr = uaddr,
 		.t_ctx = t_ctx,
+		.uaddrlen = uaddrlen,
 	};
 	struct sockaddr_storage unspec;
 	struct cgroup *cgrp;
+	int ret;
 
 	/* Check socket family since not all sockets represent network
 	 * endpoint (e.g. AF_UNIX).
@@ -1482,11 +1487,16 @@  int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
 	if (!ctx.uaddr) {
 		memset(&unspec, 0, sizeof(unspec));
 		ctx.uaddr = (struct sockaddr *)&unspec;
+		ctx.uaddrlen = sizeof(unspec);
 	}
 
 	cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
-	return bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
-				     0, flags);
+	ret = bpf_prog_run_array_cg(&cgrp->bpf, atype, &ctx, bpf_prog_run,
+				    0, flags);
+	if (ret)
+		return ret;
+
+	return (int) ctx.uaddrlen;
 }
 EXPORT_SYMBOL(__cgroup_bpf_run_filter_sock_addr);
 
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 940062e08f57..6b3e5d77d354 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -446,9 +446,9 @@  int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 	/* BPF prog is run before any checks are done so that if the prog
 	 * changes context in a wrong way it will be caught.
 	 */
-	err = BPF_CGROUP_RUN_PROG_INET_BIND_LOCK(sk, uaddr,
+	err = BPF_CGROUP_RUN_PROG_INET_BIND_LOCK(sk, uaddr, addr_len,
 						 CGROUP_INET4_BIND, &flags);
-	if (err)
+	if (err < 0)
 		return err;
 
 	return __inet_bind(sk, uaddr, addr_len, flags);
@@ -785,7 +785,7 @@  int inet_getname(struct socket *sock, struct sockaddr *uaddr,
 		}
 		sin->sin_port = inet->inet_dport;
 		sin->sin_addr.s_addr = inet->inet_daddr;
-		BPF_CGROUP_RUN_SA_PROG(sk, (struct sockaddr *)sin,
+		BPF_CGROUP_RUN_SA_PROG(sk, (struct sockaddr *)sin, sizeof(*sin),
 				       CGROUP_INET4_GETPEERNAME);
 	} else {
 		__be32 addr = inet->inet_rcv_saddr;
@@ -793,7 +793,7 @@  int inet_getname(struct socket *sock, struct sockaddr *uaddr,
 			addr = inet->inet_saddr;
 		sin->sin_port = inet->inet_sport;
 		sin->sin_addr.s_addr = addr;
-		BPF_CGROUP_RUN_SA_PROG(sk, (struct sockaddr *)sin,
+		BPF_CGROUP_RUN_SA_PROG(sk, (struct sockaddr *)sin, sizeof(*sin),
 				       CGROUP_INET4_GETSOCKNAME);
 	}
 	release_sock(sk);
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 5178a3f3cb53..9192cf5cd3ef 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -304,6 +304,8 @@  EXPORT_SYMBOL_GPL(ping_close);
 static int ping_pre_connect(struct sock *sk, struct sockaddr *uaddr,
 			    int addr_len)
 {
+	int err;
+
 	/* This check is replicated from __ip4_datagram_connect() and
 	 * intended to prevent BPF program called below from accessing bytes
 	 * that are out of the bound specified by user in addr_len.
@@ -311,7 +313,11 @@  static int ping_pre_connect(struct sock *sk, struct sockaddr *uaddr,
 	if (addr_len < sizeof(struct sockaddr_in))
 		return -EINVAL;
 
-	return BPF_CGROUP_RUN_PROG_INET4_CONNECT_LOCK(sk, uaddr);
+	err = BPF_CGROUP_RUN_PROG_INET4_CONNECT_LOCK(sk, uaddr, addr_len);
+	if (err < 0)
+		return err;
+
+	return 0;
 }
 
 /* Checks the bind address and possibly modifies sk->sk_bound_dev_if. */
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 39bda2b1066e..50fed4e1820d 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -184,6 +184,8 @@  EXPORT_SYMBOL_GPL(tcp_twsk_unique);
 static int tcp_v4_pre_connect(struct sock *sk, struct sockaddr *uaddr,
 			      int addr_len)
 {
+	int err;
+
 	/* This check is replicated from tcp_v4_connect() and intended to
 	 * prevent BPF program called below from accessing bytes that are out
 	 * of the bound specified by user in addr_len.
@@ -193,7 +195,11 @@  static int tcp_v4_pre_connect(struct sock *sk, struct sockaddr *uaddr,
 
 	sock_owned_by_me(sk);
 
-	return BPF_CGROUP_RUN_PROG_INET4_CONNECT(sk, uaddr);
+	err = BPF_CGROUP_RUN_PROG_INET4_CONNECT(sk, uaddr, addr_len);
+	if (err < 0)
+		return err;
+
+	return 0;
 }
 
 /* This will initiate an outgoing connection. */
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index aa32afd871ee..8f4b64252b51 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1157,8 +1157,10 @@  int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 
 	if (cgroup_bpf_enabled(CGROUP_UDP4_SENDMSG) && !connected) {
 		err = BPF_CGROUP_RUN_PROG_UDP4_SENDMSG_LOCK(sk,
-					    (struct sockaddr *)usin, &ipc.addr);
-		if (err)
+					    (struct sockaddr *)usin,
+					    msg->msg_namelen,
+					    &ipc.addr);
+		if (err < 0)
 			goto out_free;
 		if (usin) {
 			if (usin->sin_port == 0) {
@@ -1927,7 +1929,8 @@  int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int flags,
 		*addr_len = sizeof(*sin);
 
 		BPF_CGROUP_RUN_PROG_UDP4_RECVMSG_LOCK(sk,
-						      (struct sockaddr *)sin);
+						      (struct sockaddr *)sin,
+						      *addr_len);
 	}
 
 	if (udp_sk(sk)->gro_enabled)
@@ -1959,6 +1962,8 @@  int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int flags,
 
 int udp_pre_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 {
+	int err;
+
 	/* This check is replicated from __ip4_datagram_connect() and
 	 * intended to prevent BPF program called below from accessing bytes
 	 * that are out of the bound specified by user in addr_len.
@@ -1966,7 +1971,11 @@  int udp_pre_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 	if (addr_len < sizeof(struct sockaddr_in))
 		return -EINVAL;
 
-	return BPF_CGROUP_RUN_PROG_INET4_CONNECT_LOCK(sk, uaddr);
+	err = BPF_CGROUP_RUN_PROG_INET4_CONNECT_LOCK(sk, uaddr, addr_len);
+	if (err < 0)
+		return err;
+
+	return 0;
 }
 EXPORT_SYMBOL(udp_pre_connect);
 
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index e1b679a590c9..acba6a47ae44 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -455,9 +455,9 @@  int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 	/* BPF prog is run before any checks are done so that if the prog
 	 * changes context in a wrong way it will be caught.
 	 */
-	err = BPF_CGROUP_RUN_PROG_INET_BIND_LOCK(sk, uaddr,
+	err = BPF_CGROUP_RUN_PROG_INET_BIND_LOCK(sk, uaddr, addr_len,
 						 CGROUP_INET6_BIND, &flags);
-	if (err)
+	if (err < 0)
 		return err;
 
 	return __inet6_bind(sk, uaddr, addr_len, flags);
@@ -534,7 +534,7 @@  int inet6_getname(struct socket *sock, struct sockaddr *uaddr,
 		sin->sin6_addr = sk->sk_v6_daddr;
 		if (np->sndflow)
 			sin->sin6_flowinfo = np->flow_label;
-		BPF_CGROUP_RUN_SA_PROG(sk, (struct sockaddr *)sin,
+		BPF_CGROUP_RUN_SA_PROG(sk, (struct sockaddr *)sin, sizeof(*sin),
 				       CGROUP_INET6_GETPEERNAME);
 	} else {
 		if (ipv6_addr_any(&sk->sk_v6_rcv_saddr))
@@ -542,7 +542,7 @@  int inet6_getname(struct socket *sock, struct sockaddr *uaddr,
 		else
 			sin->sin6_addr = sk->sk_v6_rcv_saddr;
 		sin->sin6_port = inet->inet_sport;
-		BPF_CGROUP_RUN_SA_PROG(sk, (struct sockaddr *)sin,
+		BPF_CGROUP_RUN_SA_PROG(sk, (struct sockaddr *)sin, sizeof(*sin),
 				       CGROUP_INET6_GETSOCKNAME);
 	}
 	sin->sin6_scope_id = ipv6_iface_scope_id(&sin->sin6_addr,
diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c
index c4835dbdfcff..d3630cad2c21 100644
--- a/net/ipv6/ping.c
+++ b/net/ipv6/ping.c
@@ -48,6 +48,8 @@  static int dummy_ipv6_chk_addr(struct net *net, const struct in6_addr *addr,
 static int ping_v6_pre_connect(struct sock *sk, struct sockaddr *uaddr,
 			       int addr_len)
 {
+	int err;
+
 	/* This check is replicated from __ip6_datagram_connect() and
 	 * intended to prevent BPF program called below from accessing
 	 * bytes that are out of the bound specified by user in addr_len.
@@ -56,7 +58,11 @@  static int ping_v6_pre_connect(struct sock *sk, struct sockaddr *uaddr,
 	if (addr_len < SIN6_LEN_RFC2133)
 		return -EINVAL;
 
-	return BPF_CGROUP_RUN_PROG_INET6_CONNECT_LOCK(sk, uaddr);
+	err = BPF_CGROUP_RUN_PROG_INET6_CONNECT_LOCK(sk, uaddr, addr_len);
+	if (err < 0)
+		return err;
+
+	return 0;
 }
 
 static int ping_v6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 244cf86c4cbb..a59db6b3e41f 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -130,6 +130,8 @@  static u32 tcp_v6_init_ts_off(const struct net *net, const struct sk_buff *skb)
 static int tcp_v6_pre_connect(struct sock *sk, struct sockaddr *uaddr,
 			      int addr_len)
 {
+	int err;
+
 	/* This check is replicated from tcp_v6_connect() and intended to
 	 * prevent BPF program called below from accessing bytes that are out
 	 * of the bound specified by user in addr_len.
@@ -139,7 +141,11 @@  static int tcp_v6_pre_connect(struct sock *sk, struct sockaddr *uaddr,
 
 	sock_owned_by_me(sk);
 
-	return BPF_CGROUP_RUN_PROG_INET6_CONNECT(sk, uaddr);
+	err = BPF_CGROUP_RUN_PROG_INET6_CONNECT(sk, uaddr, addr_len);
+	if (err < 0)
+		return err;
+
+	return 0;
 }
 
 static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index e5a337e6b970..bc6c9a5586bf 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -429,7 +429,8 @@  int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 		*addr_len = sizeof(*sin6);
 
 		BPF_CGROUP_RUN_PROG_UDP6_RECVMSG_LOCK(sk,
-						      (struct sockaddr *)sin6);
+						      (struct sockaddr *)sin6,
+						      *addr_len);
 	}
 
 	if (udp_sk(sk)->gro_enabled)
@@ -1154,6 +1155,8 @@  static void udp_v6_flush_pending_frames(struct sock *sk)
 static int udpv6_pre_connect(struct sock *sk, struct sockaddr *uaddr,
 			     int addr_len)
 {
+	int err;
+
 	if (addr_len < offsetofend(struct sockaddr, sa_family))
 		return -EINVAL;
 	/* The following checks are replicated from __ip6_datagram_connect()
@@ -1169,7 +1172,11 @@  static int udpv6_pre_connect(struct sock *sk, struct sockaddr *uaddr,
 	if (addr_len < SIN6_LEN_RFC2133)
 		return -EINVAL;
 
-	return BPF_CGROUP_RUN_PROG_INET6_CONNECT_LOCK(sk, uaddr);
+	err = BPF_CGROUP_RUN_PROG_INET6_CONNECT_LOCK(sk, uaddr, addr_len);
+	if (err < 0)
+		return err;
+
+	return 0;
 }
 
 /**
@@ -1522,8 +1529,9 @@  int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	if (cgroup_bpf_enabled(CGROUP_UDP6_SENDMSG) && !connected) {
 		err = BPF_CGROUP_RUN_PROG_UDP6_SENDMSG_LOCK(sk,
 					   (struct sockaddr *)sin6,
+					   msg->msg_namelen,
 					   &fl6->saddr);
-		if (err)
+		if (err < 0)
 			goto out_no_dst;
 		if (sin6) {
 			if (ipv6_addr_v4mapped(&sin6->sin6_addr)) {