On 03/28/2018 05:41 AM, Alexei Starovoitov wrote:
[...]
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index e8c7fad8c329..2dec266507dc 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -450,6 +450,13 @@ int inet_bind(struct socket *sock, struct sockaddr
> *uaddr, int addr_len)
> if (addr_len < sizeof(struct sockaddr_in))
> goto out;
>
> + /* 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_INET4_BIND(sk, uaddr);
> + if (err)
> + goto out;
> +
Should the hook not come at the very beginning?
/* If the socket has its own bind function then use it. (RAW) */
if (sk->sk_prot->bind) {
err = sk->sk_prot->bind(sk, uaddr, addr_len);
goto out;
}
err = -EINVAL;
if (addr_len < sizeof(struct sockaddr_in))
goto out;
/* 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_INET4_BIND(sk, uaddr);
if (err)
goto out;
E.g. when you have v4/v6 ping or raw sockets used from language runtimes
or apps, then they provide their own bind handler here in kernel, thus any
bind rewrite won't be caught for them. Shouldn't this be covered as well
and the BPF_CGROUP_RUN_PROG_INET4_BIND() come first?
> if (addr->sin_family != AF_INET) {
> /* Compatibility games : accept AF_UNSPEC (mapped to AF_INET)
> * only if s_addr is INADDR_ANY.
> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index dbbe04018813..fa24e3f06ac6 100644
> --- a/net/ipv6/af_inet6.c
> +++ b/net/ipv6/af_inet6.c
> @@ -295,6 +295,13 @@ int inet6_bind(struct socket *sock, struct sockaddr
> *uaddr, int addr_len)
> if (addr_len < SIN6_LEN_RFC2133)
> return -EINVAL;
>
> + /* 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_INET6_BIND(sk, uaddr);
> + if (err)
> + return err;
> +
> if (addr->sin6_family != AF_INET6)
> return -EAFNOSUPPORT;
(Same here?)
