bpf: Change bpf_getsockopt(SOL_TCP) to reuse do_tcp_getsockopt()
authorMartin KaFai Lau <martin.lau@kernel.org>
Fri, 2 Sep 2022 00:29:18 +0000 (17:29 -0700)
committerAlexei Starovoitov <ast@kernel.org>
Sat, 3 Sep 2022 03:34:32 +0000 (20:34 -0700)
This patch changes bpf_getsockopt(SOL_TCP) to reuse
do_tcp_getsockopt().  It removes the duplicated code from
bpf_getsockopt(SOL_TCP).

Before this patch, there were some optnames available to
bpf_setsockopt(SOL_TCP) but missing in bpf_getsockopt(SOL_TCP).
For example, TCP_NODELAY, TCP_MAXSEG, TCP_KEEPIDLE, TCP_KEEPINTVL,
and a few more.  It surprises users from time to time.  This patch
automatically closes this gap without duplicating more code.

bpf_getsockopt(TCP_SAVED_SYN) does not free the saved_syn,
so it stays in sol_tcp_sockopt().

For string name value like TCP_CONGESTION, bpf expects it
is always null terminated, so sol_tcp_sockopt() decrements
optlen by one before calling do_tcp_getsockopt() and
the 'if (optlen < saved_optlen) memset(..,0,..);'
in __bpf_getsockopt() will always do a null termination.

Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Link: https://lore.kernel.org/r/20220902002918.2894511-1-kafai@fb.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
include/net/tcp.h
net/core/filter.c
net/ipv4/tcp.c

index c03a50c72f40e70fcafd45cd617f5892ef8a5272..735e957f7f4b0e92f6619eb70fcea12a1d97ad52 100644 (file)
@@ -402,6 +402,8 @@ void tcp_init_sock(struct sock *sk);
 void tcp_init_transfer(struct sock *sk, int bpf_op, struct sk_buff *skb);
 __poll_t tcp_poll(struct file *file, struct socket *sock,
                      struct poll_table_struct *wait);
+int do_tcp_getsockopt(struct sock *sk, int level,
+                     int optname, sockptr_t optval, sockptr_t optlen);
 int tcp_getsockopt(struct sock *sk, int level, int optname,
                   char __user *optval, int __user *optlen);
 bool tcp_bpf_bypass_getsockopt(int level, int optname);
index 9b26653a7e1f7e92207e55f7ad927227cc3afb88..beadd5b83e6cdb022f0bca2b0f12d71161adf75c 100644 (file)
@@ -5100,8 +5100,9 @@ static int bpf_sol_tcp_setsockopt(struct sock *sk, int optname,
        return 0;
 }
 
-static int sol_tcp_setsockopt(struct sock *sk, int optname,
-                             char *optval, int optlen)
+static int sol_tcp_sockopt(struct sock *sk, int optname,
+                          char *optval, int *optlen,
+                          bool getopt)
 {
        if (sk->sk_prot->setsockopt != tcp_setsockopt)
                return -EINVAL;
@@ -5118,17 +5119,51 @@ static int sol_tcp_setsockopt(struct sock *sk, int optname,
        case TCP_USER_TIMEOUT:
        case TCP_NOTSENT_LOWAT:
        case TCP_SAVE_SYN:
-               if (optlen != sizeof(int))
+               if (*optlen != sizeof(int))
                        return -EINVAL;
                break;
        case TCP_CONGESTION:
+               if (*optlen < 2)
+                       return -EINVAL;
+               break;
+       case TCP_SAVED_SYN:
+               if (*optlen < 1)
+                       return -EINVAL;
                break;
        default:
-               return bpf_sol_tcp_setsockopt(sk, optname, optval, optlen);
+               if (getopt)
+                       return -EINVAL;
+               return bpf_sol_tcp_setsockopt(sk, optname, optval, *optlen);
+       }
+
+       if (getopt) {
+               if (optname == TCP_SAVED_SYN) {
+                       struct tcp_sock *tp = tcp_sk(sk);
+
+                       if (!tp->saved_syn ||
+                           *optlen > tcp_saved_syn_len(tp->saved_syn))
+                               return -EINVAL;
+                       memcpy(optval, tp->saved_syn->data, *optlen);
+                       /* It cannot free tp->saved_syn here because it
+                        * does not know if the user space still needs it.
+                        */
+                       return 0;
+               }
+
+               if (optname == TCP_CONGESTION) {
+                       if (!inet_csk(sk)->icsk_ca_ops)
+                               return -EINVAL;
+                       /* BPF expects NULL-terminated tcp-cc string */
+                       optval[--(*optlen)] = '\0';
+               }
+
+               return do_tcp_getsockopt(sk, SOL_TCP, optname,
+                                        KERNEL_SOCKPTR(optval),
+                                        KERNEL_SOCKPTR(optlen));
        }
 
        return do_tcp_setsockopt(sk, SOL_TCP, optname,
-                                KERNEL_SOCKPTR(optval), optlen);
+                                KERNEL_SOCKPTR(optval), *optlen);
 }
 
 static int sol_ip_setsockopt(struct sock *sk, int optname,
@@ -5183,7 +5218,7 @@ static int __bpf_setsockopt(struct sock *sk, int level, int optname,
        else if (IS_ENABLED(CONFIG_IPV6) && level == SOL_IPV6)
                return sol_ipv6_setsockopt(sk, optname, optval, optlen);
        else if (IS_ENABLED(CONFIG_INET) && level == SOL_TCP)
-               return sol_tcp_setsockopt(sk, optname, optval, optlen);
+               return sol_tcp_sockopt(sk, optname, optval, &optlen, false);
 
        return -EINVAL;
 }
@@ -5206,31 +5241,8 @@ static int __bpf_getsockopt(struct sock *sk, int level, int optname,
 
        if (level == SOL_SOCKET) {
                err = sol_socket_sockopt(sk, optname, optval, &optlen, true);
-       } else if (IS_ENABLED(CONFIG_INET) &&
-                  level == SOL_TCP && sk->sk_prot->getsockopt == tcp_getsockopt) {
-               struct inet_connection_sock *icsk;
-               struct tcp_sock *tp;
-
-               switch (optname) {
-               case TCP_CONGESTION:
-                       icsk = inet_csk(sk);
-
-                       if (!icsk->icsk_ca_ops || optlen <= 1)
-                               goto err_clear;
-                       strncpy(optval, icsk->icsk_ca_ops->name, optlen);
-                       optval[optlen - 1] = 0;
-                       break;
-               case TCP_SAVED_SYN:
-                       tp = tcp_sk(sk);
-
-                       if (optlen <= 0 || !tp->saved_syn ||
-                           optlen > tcp_saved_syn_len(tp->saved_syn))
-                               goto err_clear;
-                       memcpy(optval, tp->saved_syn->data, optlen);
-                       break;
-               default:
-                       goto err_clear;
-               }
+       } else if (IS_ENABLED(CONFIG_INET) && level == SOL_TCP) {
+               err = sol_tcp_sockopt(sk, optname, optval, &optlen, true);
        } else if (IS_ENABLED(CONFIG_INET) && level == SOL_IP) {
                struct inet_sock *inet = inet_sk(sk);
 
index 45c737ee95a16eafbb61ac13119ac3dd6f5b144e..a822cc627e2ad33b4bfc76ac2330d81c15eeec10 100644 (file)
@@ -4043,8 +4043,8 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk,
        return stats;
 }
 
-static int do_tcp_getsockopt(struct sock *sk, int level,
-                            int optname, sockptr_t optval, sockptr_t optlen)
+int do_tcp_getsockopt(struct sock *sk, int level,
+                     int optname, sockptr_t optval, sockptr_t optlen)
 {
        struct inet_connection_sock *icsk = inet_csk(sk);
        struct tcp_sock *tp = tcp_sk(sk);