| Message ID | 20220612213227.3881769-3-willy@infradead.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fixes for usercopy | expand |
On Sun, Jun 12, 2022 at 10:32:26PM +0100, Matthew Wilcox (Oracle) wrote: > Get rid of a lot of annoying casts by setting 'addr' once at the top > of the function. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > --- > mm/usercopy.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/mm/usercopy.c b/mm/usercopy.c > index fdd1bed3b90a..31deee7dd2f5 100644 > --- a/mm/usercopy.c > +++ b/mm/usercopy.c > @@ -161,19 +161,20 @@ static inline void check_bogus_address(const unsigned long ptr, unsigned long n, > static inline void check_heap_object(const void *ptr, unsigned long n, > bool to_user) > { > + uintptr_t addr = (uintptr_t)ptr; > struct folio *folio; > > if (is_kmap_addr(ptr)) { > - unsigned long page_end = (unsigned long)ptr | (PAGE_SIZE - 1); > + unsigned long page_end = addr | (PAGE_SIZE - 1); > > - if ((unsigned long)ptr + n - 1 > page_end) > + if (addr + n - 1 > page_end) > usercopy_abort("kmap", NULL, to_user, > offset_in_page(ptr), n); > return; > } > > if (is_vmalloc_addr(ptr)) { > - struct vmap_area *area = find_vmap_area((unsigned long)ptr); > + struct vmap_area *area = find_vmap_area(addr); > unsigned long offset; > > if (!area) { > @@ -183,8 +184,8 @@ static inline void check_heap_object(const void *ptr, unsigned long n, > > /* XXX: We should also abort for free vmap_areas */ > > - offset = (unsigned long)ptr - area->va_start; > - if ((unsigned long)ptr + n > area->va_end) > + offset = addr - area->va_start; > + if (addr + n > area->va_end) > usercopy_abort("vmalloc", NULL, to_user, offset, n); > return; > } > -- > 2.35.1 > Looks good to me: Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com> -- Uladzislau Rezki
On Mon, Jun 13, 2022 at 11:51:24AM +0200, Uladzislau Rezki wrote: > On Sun, Jun 12, 2022 at 10:32:26PM +0100, Matthew Wilcox (Oracle) wrote: > > Get rid of a lot of annoying casts by setting 'addr' once at the top > > of the function. > > > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > --- > > mm/usercopy.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/mm/usercopy.c b/mm/usercopy.c > > index fdd1bed3b90a..31deee7dd2f5 100644 > > --- a/mm/usercopy.c > > +++ b/mm/usercopy.c > > @@ -161,19 +161,20 @@ static inline void check_bogus_address(const unsigned long ptr, unsigned long n, > > static inline void check_heap_object(const void *ptr, unsigned long n, > > bool to_user) > > { > > + uintptr_t addr = (uintptr_t)ptr; > > struct folio *folio; > > > > if (is_kmap_addr(ptr)) { > > - unsigned long page_end = (unsigned long)ptr | (PAGE_SIZE - 1); > > + unsigned long page_end = addr | (PAGE_SIZE - 1); > > > > - if ((unsigned long)ptr + n - 1 > page_end) > > + if (addr + n - 1 > page_end) > > usercopy_abort("kmap", NULL, to_user, > > offset_in_page(ptr), n); > > return; > > } > > > > if (is_vmalloc_addr(ptr)) { > > - struct vmap_area *area = find_vmap_area((unsigned long)ptr); > > + struct vmap_area *area = find_vmap_area(addr); > > unsigned long offset; > > > > if (!area) { > > @@ -183,8 +184,8 @@ static inline void check_heap_object(const void *ptr, unsigned long n, > > > > /* XXX: We should also abort for free vmap_areas */ > > > > - offset = (unsigned long)ptr - area->va_start; > > - if ((unsigned long)ptr + n > area->va_end) > > + offset = addr - area->va_start; > > + if (addr + n > area->va_end) > > usercopy_abort("vmalloc", NULL, to_user, offset, n); > > return; > > } > > -- > > 2.35.1 > > > Looks good to me: Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com> For the future, please put your tags ("Reviewed-by") on a separate line or the workflow tools (b4, patchwork, etc) won't see it. :)
> > For the future, please put your tags ("Reviewed-by") on a separate line > or the workflow tools (b4, patchwork, etc) won't see it. :) > OK. Thanks :)
diff --git a/mm/usercopy.c b/mm/usercopy.c index fdd1bed3b90a..31deee7dd2f5 100644 --- a/mm/usercopy.c +++ b/mm/usercopy.c @@ -161,19 +161,20 @@ static inline void check_bogus_address(const unsigned long ptr, unsigned long n, static inline void check_heap_object(const void *ptr, unsigned long n, bool to_user) { + uintptr_t addr = (uintptr_t)ptr; struct folio *folio; if (is_kmap_addr(ptr)) { - unsigned long page_end = (unsigned long)ptr | (PAGE_SIZE - 1); + unsigned long page_end = addr | (PAGE_SIZE - 1); - if ((unsigned long)ptr + n - 1 > page_end) + if (addr + n - 1 > page_end) usercopy_abort("kmap", NULL, to_user, offset_in_page(ptr), n); return; } if (is_vmalloc_addr(ptr)) { - struct vmap_area *area = find_vmap_area((unsigned long)ptr); + struct vmap_area *area = find_vmap_area(addr); unsigned long offset; if (!area) { @@ -183,8 +184,8 @@ static inline void check_heap_object(const void *ptr, unsigned long n, /* XXX: We should also abort for free vmap_areas */ - offset = (unsigned long)ptr - area->va_start; - if ((unsigned long)ptr + n > area->va_end) + offset = addr - area->va_start; + if (addr + n > area->va_end) usercopy_abort("vmalloc", NULL, to_user, offset, n); return; }
Get rid of a lot of annoying casts by setting 'addr' once at the top of the function. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- mm/usercopy.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)