Re: svn commit: r361568 - head/sys/powerpc/aim
On Wed, May 27, 2020 at 09:41:01PM -0500, Justin Hibbits wrote:
> On Thu, 28 May 2020 00:49:03 + (UTC)
> Brandon Bergren 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.cWed 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.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r361568 - head/sys/powerpc/aim
Justin Hibbits chmeeedalf at gmail.com wrote on
Thu May 28 02:41:06 UTC 2020 :
> On Thu, 28 May 2020 00:49:03 + (UTC)
> Brandon Bergren 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.cWed 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?
Using:
#define VM_MAXUSER_ADDRESS320xf000
as an example for 32-bit AIM powerpc.
Let (uintptr_t)uaddr==0xe000u
Let ulen==0x3000u
Then (uintptr_t)uaddr+ulen == 0x1000u
(wrapped/truncated: "Detect unsigned integer wrapping")
So (right hand sides forced unsigned
by left hand sides being so):
(uintptr_t)uaddr= VM_MAXUSER_ADDRESS
||
(uintptr_t)uaddr >= (uintptr_t)VM_MAXUSER_ADDRESS - ulen
(I've left equality handling as it was, despite, for
example, 0xe000u with length 0x2000u having a last
address of 0xefffu and 0xefffu < 0xf000u .
There may be reasons to disallow that for all I know.)
===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r361568 - head/sys/powerpc/aim
On Thu, 28 May 2020 00:49:03 + (UTC)
Brandon Bergren 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?
- Justin
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r361568 - head/sys/powerpc/aim
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.cWed May 27 23:20:35 2020
(r361567)
+++ head/sys/powerpc/aim/mmu_radix.cThu 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;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
