lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 13 Jun 2022 09:20:36 -0700 From: Kees Cook <keescook@...omium.org> To: Uladzislau Rezki <urezki@...il.com> Cc: "Matthew Wilcox (Oracle)" <willy@...radead.org>, linux-mm@...ck.org, Zorro Lang <zlang@...hat.com>, linux-xfs@...r.kernel.org, linux-hardening@...r.kernel.org Subject: Re: [PATCH 2/3] usercopy: Cast pointer to an integer once 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@...radead.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@...il.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. :) -- Kees Cook
Powered by blists - more mailing lists