From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=3.0 tests=FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DC917C2BA19 for ; Thu, 16 Apr 2020 03:02:49 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 9C9272076A for ; Thu, 16 Apr 2020 03:02:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9C9272076A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=sina.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 4BA2B8E0074; Wed, 15 Apr 2020 23:02:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 46C258E0001; Wed, 15 Apr 2020 23:02:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 380718E0074; Wed, 15 Apr 2020 23:02:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0091.hostedemail.com [216.40.44.91]) by kanga.kvack.org (Postfix) with ESMTP id 1EEE68E0001 for ; Wed, 15 Apr 2020 23:02:49 -0400 (EDT) Received: from smtpin09.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id D0385612C for ; Thu, 16 Apr 2020 03:02:48 +0000 (UTC) X-FDA: 76712220816.09.glue63_18514a337b84a X-HE-Tag: glue63_18514a337b84a X-Filterd-Recvd-Size: 8058 Received: from r3-21.sinamail.sina.com.cn (r3-21.sinamail.sina.com.cn [202.108.3.21]) by imf42.hostedemail.com (Postfix) with SMTP for ; Thu, 16 Apr 2020 03:02:46 +0000 (UTC) Received: from unknown (HELO localhost.localdomain)([221.219.5.127]) by sina.com with ESMTP id 5E97CAD1000102AF; Thu, 16 Apr 2020 11:02:44 +0800 (CST) X-Sender: hdanton@sina.com X-Auth-ID: hdanton@sina.com X-SMAIL-MID: 15851628864 From: Hillf Danton To: Jann Horn Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Alexei Starovoitov , Daniel Borkmann , Martin KaFai Lau , Song Liu , Yonghong Song , Andrii Nakryiko , John Fastabend , KP Singh , bpf@vger.kernel.org Subject: Re: [PATCH] vmalloc: Fix remap_vmalloc_range() bounds checks Date: Thu, 16 Apr 2020 11:02:31 +0800 Message-Id: <20200416030232.15680-1-hdanton@sina.com> In-Reply-To: <20200415222312.236431-1-jannh@google.com> References: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, 16 Apr 2020 00:23:12 +0200 Jann Horn wrote: >=20 > remap_vmalloc_range() has had various issues with the bounds checks it > promises to perform ("This function checks that addr is a valid vmalloc= 'ed > area, and that it is big enough to cover the vma") over time, e.g.: >=20 > - not detecting pgoff< - not detecting (pgoff< - not checking whether addr and addr+(pgoff< vmalloc allocation > - comparing a potentially wildly out-of-bounds pointer with the end of= the > vmalloc region >=20 > In particular, since commit fc9702273e2e ("bpf: Add mmap() support for > BPF_MAP_TYPE_ARRAY"), unprivileged users can cause kernel null pointer > dereferences by calling mmap() on a BPF map with a size that is bigger = than > the distance from the start of the BPF map to the end of the address sp= ace. > This could theoretically be used as a kernel ASLR bypass, by using whet= her > mmap() with a given offset oopses or returns an error code to perform a > binary search over the possible address range. >=20 > To allow remap_vmalloc_range_partial() to verify that addr and > addr+(pgoff< to remap_vmalloc_range_partial() instead of adding it to the pointer in > remap_vmalloc_range(). >=20 > In remap_vmalloc_range_partial(), fix the check against get_vm_area_siz= e() > by using size comparisons instead of pointer comparisons, and add check= s > for pgoff. >=20 > Cc: stable@vger.kernel.org > Fixes: 833423143c3a ("[PATCH] mm: introduce remap_vmalloc_range()") > Signed-off-by: Jann Horn > --- > I'm just sending this on the public list, since the worst-case impact f= or > non-root users is leaking kernel pointers to userspace. In a context wh= ere > you can reach BPF (no sandboxing), I don't think that kernel ASLR is ve= ry > effective at the moment anyway. >=20 > fs/proc/vmcore.c | 5 +++-- > include/linux/vmalloc.h | 2 +- > mm/vmalloc.c | 16 +++++++++++++--- > samples/vfio-mdev/mdpy.c | 2 +- > 4 files changed, 18 insertions(+), 7 deletions(-) >=20 > diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c > index 7dc800cce3543..c663202da8de7 100644 > --- a/fs/proc/vmcore.c > +++ b/fs/proc/vmcore.c > @@ -266,7 +266,8 @@ static int vmcoredd_mmap_dumps(struct vm_area_struc= t *vma, unsigned long dst, > if (start < offset + dump->size) { > tsz =3D min(offset + (u64)dump->size - start, (u64)size); > buf =3D dump->buf + start - offset; > - if (remap_vmalloc_range_partial(vma, dst, buf, tsz)) { > + if (remap_vmalloc_range_partial(vma, dst, buf, 0, > + tsz)) { > ret =3D -EFAULT; > goto out_unlock; > } > @@ -624,7 +625,7 @@ static int mmap_vmcore(struct file *file, struct vm= _area_struct *vma) > tsz =3D min(elfcorebuf_sz + elfnotes_sz - (size_t)start, size); > kaddr =3D elfnotes_buf + start - elfcorebuf_sz - vmcoredd_orig_sz; > if (remap_vmalloc_range_partial(vma, vma->vm_start + len, > - kaddr, tsz)) > + kaddr, 0, tsz)) > goto fail; > =20 > size -=3D tsz; > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h > index 0507a162ccd0e..a95d3cc74d79b 100644 > --- a/include/linux/vmalloc.h > +++ b/include/linux/vmalloc.h > @@ -137,7 +137,7 @@ extern void vunmap(const void *addr); > =20 > extern int remap_vmalloc_range_partial(struct vm_area_struct *vma, > unsigned long uaddr, void *kaddr, > - unsigned long size); > + unsigned long pgoff, unsigned long size); > =20 > extern int remap_vmalloc_range(struct vm_area_struct *vma, void *addr, > unsigned long pgoff); > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 399f219544f74..9a8227afa0738 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -34,6 +34,7 @@ > #include > #include > #include > +#include > =20 > #include > #include > @@ -3054,6 +3055,7 @@ long vwrite(char *buf, char *addr, unsigned long = count) > * @vma: vma to cover > * @uaddr: target user address to start at > * @kaddr: virtual address of vmalloc kernel memory > + * @pgoff: offset from @kaddr to start at > * @size: size of map area > * > * Returns: 0 for success, -Exxx on failure > @@ -3066,9 +3068,15 @@ long vwrite(char *buf, char *addr, unsigned long= count) > * Similar to remap_pfn_range() (see mm/memory.c) > */ > int remap_vmalloc_range_partial(struct vm_area_struct *vma, unsigned l= ong uaddr, > - void *kaddr, unsigned long size) > + void *kaddr, unsigned long pgoff, > + unsigned long size) > { > struct vm_struct *area; > + unsigned long off; > + unsigned long end_index; > + > + if (check_shl_overflow(pgoff, PAGE_SHIFT, &off)) > + return -EINVAL; > =20 > size =3D PAGE_ALIGN(size); > =20 > @@ -3082,8 +3090,10 @@ int remap_vmalloc_range_partial(struct vm_area_s= truct *vma, unsigned long uaddr, > if (!(area->flags & (VM_USERMAP | VM_DMA_COHERENT))) > return -EINVAL; > =20 The current kaddr is checked valid by finding area with it despite there is room for adding change in checking its boundary in a valid area. > - if (kaddr + size > area->addr + get_vm_area_size(area)) > + if (check_add_overflow(size, off, &end_index) || > + end_index > get_vm_area_size(area)) > return -EINVAL; > + kaddr +=3D off; > =20 > do { > struct page *page =3D vmalloc_to_page(kaddr); > @@ -3122,7 +3132,7 @@ int remap_vmalloc_range(struct vm_area_struct *vm= a, void *addr, > unsigned long pgoff) > { > return remap_vmalloc_range_partial(vma, vma->vm_start, > - addr + (pgoff << PAGE_SHIFT), > + addr, pgoff, > vma->vm_end - vma->vm_start); > } > EXPORT_SYMBOL(remap_vmalloc_range); > diff --git a/samples/vfio-mdev/mdpy.c b/samples/vfio-mdev/mdpy.c > index cc86bf6566e42..9894693f3be17 100644 > --- a/samples/vfio-mdev/mdpy.c > +++ b/samples/vfio-mdev/mdpy.c > @@ -418,7 +418,7 @@ static int mdpy_mmap(struct mdev_device *mdev, stru= ct vm_area_struct *vma) > return -EINVAL; > =20 > return remap_vmalloc_range_partial(vma, vma->vm_start, > - mdev_state->memblk, > + mdev_state->memblk, 0, > vma->vm_end - vma->vm_start); > } > =20 >=20 > base-commit: 8632e9b5645bbc2331d21d892b0d6961c1a08429 > --=20 > 2.26.0.110.g2183baf09c-goog --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -3074,7 +3074,7 @@ int remap_vmalloc_range_partial(struct v if (!(area->flags & (VM_USERMAP | VM_DMA_COHERENT))) return -EINVAL; =20 - if (kaddr + size > area->addr + get_vm_area_size(area)) + if (size > area->addr + get_vm_area_size(area) - kaddr) return -EINVAL; =20 do {