[SRU Groovy, Focal/linux-oem-5.10/Hirsute 2/2] bpf: Prevent writable memory-mapping of read-only ringbuf pages

[Tim Gardner]

The Fixed-by URL in https://ubuntu.com/security/cve-2021-3489 is a 
little weird, but if you tease out the SHA1 from the URL, the commit 
referenced is quite different then this patch. I also noticed that you 
got that patch merged upstream as a fix, yet we were also carrying a 
SAUCE patch that you're reverting here. I'm a little confused.

rtg

On 5/26/21 6:11 PM, Thadeu Lima de Souza Cascardo wrote:
> From: Andrii Nakryiko <andrii at kernel.org>
> 
> Only the very first page of BPF ringbuf that contains consumer position
> counter is supposed to be mapped as writeable by user-space. Producer
> position is read-only and can be modified only by the kernel code. BPF ringbuf
> data pages are read-only as well and are not meant to be modified by
> user-code to maintain integrity of per-record headers.
> 
> This patch allows to map only consumer position page as writeable and
> everything else is restricted to be read-only. remap_vmalloc_range()
> internally adds VM_DONTEXPAND, so all the established memory mappings can't be
> extended, which prevents any future violations through mremap()'ing.
> 
> Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier support for it")
> Reported-by: Ryota Shiga (Flatt Security)
> Reported-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
> Signed-off-by: Andrii Nakryiko <andrii at kernel.org>
> Signed-off-by: Daniel Borkmann <daniel at iogearbox.net>
> Acked-by: Alexei Starovoitov <ast at kernel.org>
> (cherry picked from commit 04ea3086c4d73da7009de1e84962a904139af219 bpf.git)
> CVE-2021-3489
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
> ---
>   kernel/bpf/ringbuf.c | 21 ++++++++-------------
>   1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
> index 1619afe00ded..e1da496135c6 100644
> --- a/kernel/bpf/ringbuf.c
> +++ b/kernel/bpf/ringbuf.c
> @@ -247,25 +247,20 @@ static int ringbuf_map_get_next_key(struct bpf_map *map, void *key,
>   	return -ENOTSUPP;
>   }
>   
> -static size_t bpf_ringbuf_mmap_page_cnt(const struct bpf_ringbuf *rb)
> -{
> -	size_t data_pages = (rb->mask + 1) >> PAGE_SHIFT;
> -
> -	/* consumer page + producer page + 2 x data pages */
> -	return RINGBUF_POS_PAGES + 2 * data_pages;
> -}
> -
>   static int ringbuf_map_mmap(struct bpf_map *map, struct vm_area_struct *vma)
>   {
>   	struct bpf_ringbuf_map *rb_map;
> -	size_t mmap_sz;
>   
>   	rb_map = container_of(map, struct bpf_ringbuf_map, map);
> -	mmap_sz = bpf_ringbuf_mmap_page_cnt(rb_map->rb) << PAGE_SHIFT;
> -
> -	if (vma->vm_pgoff * PAGE_SIZE + (vma->vm_end - vma->vm_start) > mmap_sz)
> -		return -EINVAL;
>   
> +	if (vma->vm_flags & VM_WRITE) {
> +		/* allow writable mapping for the consumer_pos only */
> +		if (vma->vm_pgoff != 0 || vma->vm_end - vma->vm_start != PAGE_SIZE)
> +			return -EPERM;
> +	} else {
> +		vma->vm_flags &= ~VM_MAYWRITE;
> +	}
> +	/* remap_vmalloc_range() checks size and offset constraints */
>   	return remap_vmalloc_range(vma, rb_map->rb,
>   				   vma->vm_pgoff + RINGBUF_PGOFF);
>   }
> 

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