On Sun, Jun 12, 2022 at 11:27 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Sun, Jun 12, 2022 at 03:03:20PM +0200, Uladzislau Rezki wrote:
> > > @@ -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.
>
> I don't think offset is necessarily zero. 'ptr' is a pointer somewhere
> in the object, not necessarily the start of the object.
>
> > >
> > > + 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>
>
> Hmm ... we should probably be more careful about wrapping.
>
> if (n > area->va_end - addr)
> usercopy_abort("vmalloc", NULL, to_user, offset, n);
>
> ... and that goes for the whole function actually. I'll split that into
> a separate change.
Please let me know if there is something we want to test -- I can
reproduce the problem reliably:
------------[ cut here ]------------
kernel BUG at mm/usercopy.c:101!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
CPU: 4 PID: 3259 Comm: iptables Not tainted 5.19.0-rc1-lockdep+ #1
pc : usercopy_abort+0x9c/0xa0
lr : usercopy_abort+0x9c/0xa0
sp : ffffffc010bd78d0
x29: ffffffc010bd78e0 x28: 42ffff80ac08d8ec x27: 42ffff80ac08d8ec
x26: 42ffff80ac08d8c0 x25: 000000000000000a x24: ffffffdf4c7e5120
x23: 000000000bec44c2 x22: efffffc000000000 x21: ffffffdf2896b0c0
x20: 0000000000000001 x19: 000000000000000b x18: 0000000000000000
x17: 2820636f6c6c616d x16: 0000000000000042 x15: 6574636574656420
x14: 74706d6574746120 x13: 0000000000000018 x12: 000000000000000d
x11: ff80007fffffffff x10: 0000000000000001 x9 : db174b7f89103400
x8 : db174b7f89103400 x7 : 0000000000000000 x6 : 79706f6372657375
x5 : ffffffdf4d9c617e x4 : 0000000000000000 x3 : ffffffdf4b7d017c
x2 : ffffff80eb188b18 x1 : 42ffff80ac08d8c8 x0 : 0000000000000066
Call trace:
usercopy_abort+0x9c/0xa0
__check_object_size+0x38c/0x400
xt_obj_to_user+0xe4/0x200
xt_compat_target_to_user+0xd8/0x18c
compat_copy_entries_to_user+0x278/0x424
do_ipt_get_ctl+0x7bc/0xb2c
nf_getsockopt+0x7c/0xb4
ip_getsockopt+0xee8/0xfa4
raw_getsockopt+0xf4/0x23c
sock_common_getsockopt+0x48/0x54
__sys_getsockopt+0x11c/0x2f8
__arm64_sys_getsockopt+0x60/0x70
el0_svc_common+0xfc/0x1cc
do_el0_svc_compat+0x38/0x5c
el0_svc_compat+0x68/0xf4
el0t_32_sync_handler+0xc0/0xf0
el0t_32_sync+0x190/0x194
Code: aa0903e4 a9017bfd 910043fd 9438be18 (d4210000)
---[ end trace 0000000000000000 ]---
