Skip to content

Commit 33f339a

Browse files
Tze-nan Wukuba-moo
authored andcommitted
bpf, net: Fix a potential race in do_sock_getsockopt()
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: 0d01da6 ("bpf: implement getsockopt and setsockopt hooks") Co-developed-by: Yanghui Li <[email protected]> Signed-off-by: Yanghui Li <[email protected]> Co-developed-by: Cheng-Jui Wang <[email protected]> Signed-off-by: Cheng-Jui Wang <[email protected]> Signed-off-by: Tze-nan Wu <[email protected]> Acked-by: Stanislav Fomichev <[email protected]> Acked-by: Alexei Starovoitov <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 77461c1 commit 33f339a

File tree

2 files changed

+2
-11
lines changed

2 files changed

+2
-11
lines changed

include/linux/bpf-cgroup.h

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -390,14 +390,6 @@ static inline bool cgroup_bpf_sock_enabled(struct sock *sk,
390390
__ret; \
391391
})
392392

393-
#define BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN(optlen) \
394-
({ \
395-
int __ret = 0; \
396-
if (cgroup_bpf_enabled(CGROUP_GETSOCKOPT)) \
397-
copy_from_sockptr(&__ret, optlen, sizeof(int)); \
398-
__ret; \
399-
})
400-
401393
#define BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock, level, optname, optval, optlen, \
402394
max_optlen, retval) \
403395
({ \
@@ -518,7 +510,6 @@ static inline int bpf_percpu_cgroup_storage_update(struct bpf_map *map,
518510
#define BPF_CGROUP_RUN_PROG_SOCK_OPS(sock_ops) ({ 0; })
519511
#define BPF_CGROUP_RUN_PROG_DEVICE_CGROUP(atype, major, minor, access) ({ 0; })
520512
#define BPF_CGROUP_RUN_PROG_SYSCTL(head,table,write,buf,count,pos) ({ 0; })
521-
#define BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN(optlen) ({ 0; })
522513
#define BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock, level, optname, optval, \
523514
optlen, max_optlen, retval) ({ retval; })
524515
#define BPF_CGROUP_RUN_PROG_GETSOCKOPT_KERN(sock, level, optname, optval, \

net/socket.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2362,7 +2362,7 @@ INDIRECT_CALLABLE_DECLARE(bool tcp_bpf_bypass_getsockopt(int level,
23622362
int do_sock_getsockopt(struct socket *sock, bool compat, int level,
23632363
int optname, sockptr_t optval, sockptr_t optlen)
23642364
{
2365-
int max_optlen __maybe_unused;
2365+
int max_optlen __maybe_unused = 0;
23662366
const struct proto_ops *ops;
23672367
int err;
23682368

@@ -2371,7 +2371,7 @@ int do_sock_getsockopt(struct socket *sock, bool compat, int level,
23712371
return err;
23722372

23732373
if (!compat)
2374-
max_optlen = BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN(optlen);
2374+
copy_from_sockptr(&max_optlen, optlen, sizeof(int));
23752375

23762376
ops = READ_ONCE(sock->ops);
23772377
if (level == SOL_SOCKET) {

0 commit comments

Comments
 (0)