Skip to content

Commit a25a66b

Browse files
borkmannsmb49
authored andcommitted
net, sctp, filter: remap copy_from_user failure error
BugLink: https://bugs.launchpad.net/bugs/1915186 [ no upstream commit ] Fix a potential kernel address leakage for the prerequisite where there is a BPF program attached to the cgroup/setsockopt hook. The latter can only be attached under root, however, if the attached program returns 1 to then run the related kernel handler, an unprivileged program could probe for kernel addresses that way. The reason this is possible is that we're under set_fs(KERNEL_DS) when running the kernel setsockopt handler. Aside from old cBPF there is also SCTP's struct sctp_getaddrs_old which contains pointers in the uapi struct that further need copy_from_user() inside the handler. In the normal case this would just return -EFAULT, but under a temporary KERNEL_DS setting the memory would be copied and we'd end up at a different error code, that is, -EINVAL, for both cases given subsequent validations fail, which then allows the app to distinguish and make use of this fact for probing the address space. In case of later kernel versions this issue won't work anymore thanks to Christoph Hellwig's work that got rid of the various temporary set_fs() address space overrides altogether. One potential option for 5.4 as the only affected stable kernel with the least complexity would be to remap those affected -EFAULT copy_from_user() error codes with -EINVAL such that they cannot be probed anymore. Risk of breakage should be rather low for this particular error case. Fixes: 0d01da6 ("bpf: implement getsockopt and setsockopt hooks") Reported-by: Ryota Shiga (Flatt Security) Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Cc: Stanislav Fomichev <sdf@google.com> Cc: Eric Dumazet <edumazet@google.com> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Kamal Mostafa <kamal@canonical.com> Signed-off-by: William Breathitt Gray <william.gray@canonical.com>
1 parent e778e16 commit a25a66b

File tree

2 files changed

+2
-2
lines changed

2 files changed

+2
-2
lines changed

net/core/filter.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1475,7 +1475,7 @@ struct bpf_prog *__get_filter(struct sock_fprog *fprog, struct sock *sk)
14751475

14761476
if (copy_from_user(prog->insns, fprog->filter, fsize)) {
14771477
__bpf_prog_free(prog);
1478-
return ERR_PTR(-EFAULT);
1478+
return ERR_PTR(-EINVAL);
14791479
}
14801480

14811481
prog->len = fprog->len;

net/sctp/socket.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1319,7 +1319,7 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
13191319

13201320
kaddrs = memdup_user(addrs, addrs_size);
13211321
if (IS_ERR(kaddrs))
1322-
return PTR_ERR(kaddrs);
1322+
return PTR_ERR(kaddrs) == -EFAULT ? -EINVAL : PTR_ERR(kaddrs);
13231323

13241324
/* Allow security module to validate connectx addresses. */
13251325
err = security_sctp_bind_connect(sk, SCTP_SOCKOPT_CONNECTX,

0 commit comments

Comments
 (0)