]> www.infradead.org Git - nvme.git/commitdiff
bpf, net: Fix a potential race in do_sock_getsockopt()
authorTze-nan Wu <Tze-nan.Wu@mediatek.com>
Fri, 30 Aug 2024 08:25:17 +0000 (16:25 +0800)
committerJakub Kicinski <kuba@kernel.org>
Tue, 3 Sep 2024 19:06:24 +0000 (12:06 -0700)
There's a potential race when `cgroup_bpf_enabled(CGROUP_GETSOCKOPT)` is
false during the execution of `BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN`, but
becomes true when `BPF_CGROUP_RUN_PROG_GETSOCKOPT` is called.
This inconsistency can lead to `BPF_CGROUP_RUN_PROG_GETSOCKOPT` receiving
an "-EFAULT" from `__cgroup_bpf_run_filter_getsockopt(max_optlen=0)`.
Scenario shown as below:

           `process A`                      `process B`
           -----------                      ------------
  BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN
                                            enable CGROUP_GETSOCKOPT
  BPF_CGROUP_RUN_PROG_GETSOCKOPT (-EFAULT)

To resolve this, remove the `BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN` macro and
directly uses `copy_from_sockptr` to ensure that `max_optlen` is always
set before `BPF_CGROUP_RUN_PROG_GETSOCKOPT` is invoked.

Fixes: 0d01da6afc54 ("bpf: implement getsockopt and setsockopt hooks")
Co-developed-by: Yanghui Li <yanghui.li@mediatek.com>
Signed-off-by: Yanghui Li <yanghui.li@mediatek.com>
Co-developed-by: Cheng-Jui Wang <cheng-jui.wang@mediatek.com>
Signed-off-by: Cheng-Jui Wang <cheng-jui.wang@mediatek.com>
Signed-off-by: Tze-nan Wu <Tze-nan.Wu@mediatek.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20240830082518.23243-1-Tze-nan.Wu@mediatek.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
include/linux/bpf-cgroup.h
net/socket.c

index fb3c3e7181e6d82edfc96b976503f7853089a26a..ce91d9b2acb9f8991150ceead4475b130bead438 100644 (file)
@@ -390,14 +390,6 @@ static inline bool cgroup_bpf_sock_enabled(struct sock *sk,
        __ret;                                                                 \
 })
 
-#define BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN(optlen)                              \
-({                                                                            \
-       int __ret = 0;                                                         \
-       if (cgroup_bpf_enabled(CGROUP_GETSOCKOPT))                             \
-               copy_from_sockptr(&__ret, optlen, sizeof(int));                \
-       __ret;                                                                 \
-})
-
 #define BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock, level, optname, optval, optlen,   \
                                       max_optlen, retval)                     \
 ({                                                                            \
@@ -518,7 +510,6 @@ static inline int bpf_percpu_cgroup_storage_update(struct bpf_map *map,
 #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; })
-#define BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN(optlen) ({ 0; })
 #define BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock, level, optname, optval, \
                                       optlen, max_optlen, retval) ({ retval; })
 #define BPF_CGROUP_RUN_PROG_GETSOCKOPT_KERN(sock, level, optname, optval, \
index fcbdd5bc47ac2ff98bb243a2c9ec4f91fd64e778..0a2bd22ec105c84de18154500e5beede6a178594 100644 (file)
@@ -2362,7 +2362,7 @@ INDIRECT_CALLABLE_DECLARE(bool tcp_bpf_bypass_getsockopt(int level,
 int do_sock_getsockopt(struct socket *sock, bool compat, int level,
                       int optname, sockptr_t optval, sockptr_t optlen)
 {
-       int max_optlen __maybe_unused;
+       int max_optlen __maybe_unused = 0;
        const struct proto_ops *ops;
        int err;
 
@@ -2371,7 +2371,7 @@ int do_sock_getsockopt(struct socket *sock, bool compat, int level,
                return err;
 
        if (!compat)
-               max_optlen = BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN(optlen);
+               copy_from_sockptr(&max_optlen, optlen, sizeof(int));
 
        ops = READ_ONCE(sock->ops);
        if (level == SOL_SOCKET) {