Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 28 May 2020 13:30:16 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Justin Hibbits <chmeeedalf@gmail.com>
Cc:        Brandon Bergren <bdragon@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r361568 - head/sys/powerpc/aim
Message-ID:  <20200528103016.GH48478@kib.kiev.ua>
In-Reply-To: <20200527214101.59293529@titan.knownspace>
References:  <202005280049.04S0n3jS096937@repo.freebsd.org> <20200527214101.59293529@titan.knownspace>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, May 27, 2020 at 09:41:01PM -0500, Justin Hibbits wrote:
> On Thu, 28 May 2020 00:49:03 +0000 (UTC)
> Brandon Bergren <bdragon@FreeBSD.org> wrote:
> 
> > Author: bdragon
> > Date: Thu May 28 00:49:02 2020
> > New Revision: 361568
> > URL: https://svnweb.freebsd.org/changeset/base/361568
> > 
> > Log:
> >   [PowerPC] Fix radix crash when passing -1 from userspace
> >   
> >   Found by running libc tests with radix enabled.
> >   
> >   Detect unsigned integer wrapping with a postcondition.
> >   
> >   Note: Radix MMU is not enabled by default yet.
> >   
> >   Sponsored by:	Tag1 Consulting, Inc.
> > 
> > Modified:
> >   head/sys/powerpc/aim/mmu_radix.c
> > 
> > Modified: head/sys/powerpc/aim/mmu_radix.c
> > ==============================================================================
> > --- head/sys/powerpc/aim/mmu_radix.c	Wed May 27 23:20:35
> > 2020	(r361567) +++ head/sys/powerpc/aim/mmu_radix.c	Thu
> > May 28 00:49:02 2020	(r361568) @@ -6000,7 +6000,8 @@
> > mmu_radix_kremove(vm_offset_t va) int mmu_radix_map_user_ptr(pmap_t
> > pm, volatile const void *uaddr, void **kaddr, size_t ulen, size_t
> > *klen) {
> > -	if ((uintptr_t)uaddr + ulen >= VM_MAXUSER_ADDRESS)
> > +	if ((uintptr_t)uaddr + ulen >= VM_MAXUSER_ADDRESS ||
> > +	    (uintptr_t)uaddr + ulen < (uintptr_t)uaddr)
> >  		return (EFAULT);
> >  
> >  	*kaddr = (void *)(uintptr_t)uaddr;
> 
> Wouldn't
> 
>     if ((uintptr_t)uaddr >= VM_MAXUSER_ADDRESS ||
>         (uintptr_t)uaddr + ulen >= VM_MAXUSER_ADDRESS)
> 
> be more appropriate?

The committed change is the canonical way to detect unsigned overflow,
so I think it is fine and does not depend on specific values of
VM_MAXUSER_ADDRESS.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20200528103016.GH48478>