Date: Wed, 27 May 2020 22:49:29 -0700 From: Mark Millard <marklmi@yahoo.com> To: Justin Hibbits <chmeeedalf@gmail.com>, svn-src-head@freebsd.org Subject: Re: svn commit: r361568 - head/sys/powerpc/aim Message-ID: <3ACF2BCA-284B-4957-AF39-FF6576B2BE3E@yahoo.com> References: <3ACF2BCA-284B-4957-AF39-FF6576B2BE3E.ref@yahoo.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Justin Hibbits chmeeedalf at gmail.com wrote on Thu May 28 02:41:06 UTC 2020 : > On Thu, 28 May 2020 00:49:03 +0000 (UTC) > Brandon Bergren <bdragon at FreeBSD.org> wrote: >=20 > > Author: bdragon > > Date: Thu May 28 00:49:02 2020 > > New Revision: 361568 > > URL: https://svnweb.freebsd.org/changeset/base/361568 > >=20 > > Log: > > [PowerPC] Fix radix crash when passing -1 from userspace > > =20 > > Found by running libc tests with radix enabled. > > =20 > > Detect unsigned integer wrapping with a postcondition. > > =20 > > Note: Radix MMU is not enabled by default yet. > > =20 > > Sponsored by: Tag1 Consulting, Inc. > >=20 > > Modified: > > head/sys/powerpc/aim/mmu_radix.c > >=20 > > Modified: head/sys/powerpc/aim/mmu_radix.c > > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > > --- 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 >=3D VM_MAXUSER_ADDRESS) > > + if ((uintptr_t)uaddr + ulen >=3D VM_MAXUSER_ADDRESS || > > + (uintptr_t)uaddr + ulen < (uintptr_t)uaddr) > > return (EFAULT); > > =20 > > *kaddr =3D (void *)(uintptr_t)uaddr; >=20 > Wouldn't >=20 > if ((uintptr_t)uaddr >=3D VM_MAXUSER_ADDRESS || > (uintptr_t)uaddr + ulen >=3D VM_MAXUSER_ADDRESS) >=20 > be more appropriate? Using: #define VM_MAXUSER_ADDRESS32 0xfffff000 as an example for 32-bit AIM powerpc. Let (uintptr_t)uaddr=3D=3D0xffffe000u Let ulen=3D=3D 0x3000u Then (uintptr_t)uaddr+ulen =3D=3D 0x1000u (wrapped/truncated: "Detect unsigned integer wrapping") So (right hand sides forced unsigned by left hand sides being so): (uintptr_t)uaddr<VM_MAXUSER_ADDRESS32 && (uintptr_t)uaddr+ulen<VM_MAXUSER_ADDRESS32 (making your if skip the code it controls access to). Another way to write a test that avoids the wrap messing things up is: (unitptr_t)ulen >=3D VM_MAXUSER_ADDRESS || (uintptr_t)uaddr >=3D (uintptr_t)VM_MAXUSER_ADDRESS - ulen (I've left equality handling as it was, despite, for example, 0xffffe000u with length 0x2000u having a last address of 0xffffefffu and 0xffffefffu < 0xfffff000u . There may be reasons to disallow that for all I know.) =3D=3D=3D Mark Millard marklmi at yahoo.com ( dsl-only.net went away in early 2018-Mar)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3ACF2BCA-284B-4957-AF39-FF6576B2BE3E>