https://bugzilla.kernel.org/show_bug.cgi?id=216073 --- Comment #17 from urezki@xxxxxxxxx --- > 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. > Right you are. Just checked the __find_vmap_area() it returns VA of the address it belongs to. Initially i was thinking that addr have to be exactly as va->start only, so i was wrong. > > > > > > + 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. > Based on that offset can be > 0, checking "offset + n" with va->va_end is OK. <snip> if (offset + n > area->va_end) usercopy_abort("vmalloc", NULL, to_user, offset, n); <snip> -- Uladzislau Rezki -- You may reply to this email to add a comment. You are receiving this mail because: You are watching someone on the CC list of the bug.
