Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 10 Jun 2019 16:54:09 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Doug Moore <dougm@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r348843 - head/sys/vm
Message-ID:  <20190610160930.S2504@besplex.bde.org>
In-Reply-To: <201906100307.x5A37BFt099669@repo.freebsd.org>
References:  <201906100307.x5A37BFt099669@repo.freebsd.org>

index | next in thread | previous in thread | raw e-mail

On Mon, 10 Jun 2019, Doug Moore wrote:

> Log:
>  There are times when a len==0 parameter to mmap is okay. But on a
>  32-bit machine, a len parameter just a few bytes short of 4G, rounded
>  up to a page boundary and hitting zero then, is not okay. Return
>  failure in that case.

Some overflows still occur.

The problem is not limited to 32-bit machines.  The first overflow is for
len parameter just a few bytes short of SIZE_MAX added to a page offset of
a few bytes.  This overflows to a small value.  Then rounding up to a page
boundary doesn't overflow, but gives 0 or PAGE_SIZE, so the new overflow
check doesn't work and overflow still occurs.

The second overflow is for a len parameter just a few bytes short of
SIZE_MAX with the first overflow not occurring (usually because the offset
is 0).  This is now detected.

>  Reported by: pho
>  Reviewed by: alc, kib (mentor)
>  Tested by: pho
>  Differential Revision: https://reviews.freebsd.org/D20580
>
> Modified:
>  head/sys/vm/vm_mmap.c
>
> Modified: head/sys/vm/vm_mmap.c
> ==============================================================================
> --- head/sys/vm/vm_mmap.c	Sun Jun  9 22:55:21 2019	(r348842)
> +++ head/sys/vm/vm_mmap.c	Mon Jun 10 03:07:10 2019	(r348843)
> @@ -257,7 +257,10 @@ kern_mmap(struct thread *td, uintptr_t addr0, size_t s
>
> 	/* Adjust size for rounding (on both ends). */
> 	size += pageoff;			/* low end... */

The first overflow occurs here.  Except in special cases, pageoff can be
anything between 0 and PAGE_SIZE - 1, and size can be anything between 0
and SIZE_MAX.

> -	size = (vm_size_t) round_page(size);	/* hi end */
> +	/* Check for rounding up to zero. */
> +	if (round_page(size) < size)
> +		return (EINVAL);
> +	size = round_page(size);		/* hi end */
>
> 	/* Ensure alignment is at least a page and fits in a pointer. */
> 	align = flags & MAP_ALIGNMENT_MASK;

This bug was implemented in r239247 and affects all versions of FreeBSD
newer than FreeBSD-7.  Before then, FreeBSD used the bogus 4.4BSD check
that (ssize_t)uap->len >= 0 (else return EINVAL).  This behaviour was
even documented.  POSIX doesn't allow this -- it requires ENOMEM for
invalid ranges, though it should require EOVERFLOW for ranges that are
so invalid that they overflow something.

Bruce


help

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