> On Sun, Jun 12, 2022 at 12:42:30PM +0800, Zorro Lang wrote:
> > Looks likt it's not a s390x specific bug, I just hit this issue once (not 100%
> > reproducible) on aarch64 with linux v5.19.0-rc1+ [1]. So back to cc linux-mm
> > to get more review.
> >
> > [1]
> > [ 980.200947] usercopy: Kernel memory exposure attempt detected from vmalloc 'no area' (offset 0, size 1)!
>
> if (is_vmalloc_addr(ptr)) {
> struct vm_struct *area = find_vm_area(ptr);
> if (!area) {
> usercopy_abort("vmalloc", "no area", to_user, 0, n);
>
> Oh. Looks like XFS uses vm_map_ram() and vm_map_ram() doesn't allocate
> a vm_struct.
>
> Ulad, how does this look to you?
>
It looks like a correct way to me :) XFS uses per-cpu-vm_map_ram()-vm_unmap_ram()
API which do not allocate "vm_struct" because it is not needed.
>
> diff --git a/mm/usercopy.c b/mm/usercopy.c
> index baeacc735b83..6bc2a1407c59 100644
> --- a/mm/usercopy.c
> +++ b/mm/usercopy.c
> @@ -173,7 +173,7 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
> }
>
> if (is_vmalloc_addr(ptr)) {
> - struct vm_struct *area = find_vm_area(ptr);
> + struct vmap_area *area = find_vmap_area((unsigned long)ptr);
> unsigned long offset;
>
> if (!area) {
> @@ -181,8 +181,9 @@ static inline void check_heap_object(const void *ptr, unsigned long n,
> return;
> }
>
> - offset = ptr - area->addr;
> - if (offset + n > get_vm_area_size(area))
> + /* XXX: We should also abort for free vmap_areas */
> + offset = (unsigned long)ptr - area->va_start;
>
I was a bit confused about "offset" and why it is needed here. It is always zero.
So we can get rid of it to make it less confused. From the other hand a zero offset
contributes to nothing.
>
> + if (offset + n >= area->va_end)
>
I think it is a bit wrong. As i see it, "n" is a size and what we would like to do
here is boundary check:
<snip>
if (n > va_size(area))
usercopy_abort("vmalloc", NULL, to_user, 0, n);
<snip>
--
Uladzislau Rezki
