ACK: [SRU Groovy, Focal/linux-oem-5.6 1/1] net, sctp, filter: remap copy_from_user failure error

[Tim Gardner]

On 2/19/21 11:29 AM, Thadeu Lima de Souza Cascardo wrote:
> From: Daniel Borkmann <daniel at iogearbox.net>
> 
> [ 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: 0d01da6afc54 ("bpf: implement getsockopt and setsockopt hooks")
> Reported-by: Ryota Shiga (Flatt Security)
> Signed-off-by: Daniel Borkmann <daniel at iogearbox.net>
> Cc: Stanislav Fomichev <sdf at google.com>
> Cc: Eric Dumazet <edumazet at google.com>
> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner at gmail.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> (cherry picked from commit 55bac51762c39ef033b488dd09b60d48908d317f linux-5.4.y)
> CVE-2021-20239
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
> ---
>   net/core/filter.c | 2 +-
>   net/sctp/socket.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 3e4de9e461bd..853c0d5b9fb9 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -1464,7 +1464,7 @@ struct bpf_prog *__get_filter(struct sock_fprog *fprog, struct sock *sk)
>   
>   	if (copy_from_user(prog->insns, fprog->filter, fsize)) {
>   		__bpf_prog_free(prog);
> -		return ERR_PTR(-EFAULT);
> +		return ERR_PTR(-EINVAL);
>   	}
>   
>   	prog->len = fprog->len;
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 102aee4f7dfd..bde36e0a7308 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1325,7 +1325,7 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
>   
>   	kaddrs = memdup_user(addrs, addrs_size);
>   	if (IS_ERR(kaddrs))
> -		return PTR_ERR(kaddrs);
> +		return PTR_ERR(kaddrs) == -EFAULT ? -EINVAL : PTR_ERR(kaddrs);
>   
>   	/* Allow security module to validate connectx addresses. */
>   	err = security_sctp_bind_connect(sk, SCTP_SOCKOPT_CONNECTX,
> 

Acked-by: Tim Gardner <tim.gardner at canonical.com>

-----------
Tim Gardner
Canonical, Inc