| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c73a6a41-9e05-c473-0e46-56d0fcfb9ac8@linux.dev>
Date: Tue, 5 Sep 2023 14:21:00 -0700
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Daan De Meyer <daan.j.demeyer@...il.com>
Cc: kernel-team@...a.com, netdev@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next v3 2/9] bpf: Propagate modified uaddrlen from
cgroup sockaddr programs
On 8/31/23 8:34 AM, Daan De Meyer wrote:
> As prep for adding unix socket support to the cgroup sockaddr hooks,
> let's propagate the sockaddr length back to the caller after running
> a bpf cgroup sockaddr hook program. 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 take the uaddrlen as
> an input/output argument. After running the program, the modified sockaddr
> length is stored in the uaddrlen pointer. If no uaddrlen pointer is
> provided, we determine the uaddrlen based on the socket family. For the
> existing AF_INET and AF_INET6 use cases, we don't pass in the address
> length explicitly and just determine it based on the passed in socket
> family.
The description on the inet address length needs an update, at least the
AF_INET6 one.
> diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> index 8506690dbb9c..31561e789715 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,
> + int *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 761af6b3cf2b..77db4263d68d 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -1285,6 +1285,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 5b2741aa0d9b..534b6c7f5659 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: Pointer to the 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
> @@ -1461,6 +1462,7 @@ EXPORT_SYMBOL(__cgroup_bpf_run_filter_sk);
> */
> int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> struct sockaddr *uaddr,
> + int *uaddrlen,
> enum cgroup_bpf_attach_type atype,
> void *t_ctx,
> u32 *flags)
> @@ -1472,6 +1474,7 @@ int __cgroup_bpf_run_filter_sock_addr(struct sock *sk,
> };
> 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 +1485,22 @@ 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 = 0;
> + } else if (uaddrlen)
> + ctx.uaddrlen = *uaddrlen;
> + else if (sk->sk_family == AF_INET)
> + ctx.uaddrlen = sizeof(struct sockaddr_in);
> + else if (sk->sk_family == AF_INET6)
> + ctx.uaddrlen = sizeof(struct sockaddr_in6);
I was thinking to pass addrlen whenever possible for AF_INET and AF_INET6.
If I read correctly, all AF_INET6 cases have addrlen available in this patch.
May be just pass addrlen for AF_INET also, then the new sk->sk_family test here
can be avoided?
Powered by blists - more mailing lists