svn commit: r271635 - in head: lib/libc/sys sys/vm

[Andriy Gapon]

On 15/09/2014 20:20, John Baldwin wrote:
> Author: jhb
> Date: Mon Sep 15 17:20:13 2014
> New Revision: 271635
> URL: http://svnweb.freebsd.org/changeset/base/271635
> 
> Log:
>   Add stricter checking of some mmap() arguments:
>   - Fail with EINVAL if an invalid protection mask is passed to mmap().
>   - Fail with EINVAL if an unknown flag is passed to mmap().
>   - Fail with EINVAL if both MAP_PRIVATE and MAP_SHARED are passed to mmap().
>   - Require one of either MAP_PRIVATE or MAP_SHARED for non-anonymous
>     mappings.

This broke Java, at least java/openjdk7, for me:

 25323 java     CALL
mmap(0,0x3000000,0x3<PROT_READ|PROT_WRITE>,0x1042<MAP_PRIVATE|MAP_NORESERVE|MAP_ANON>,0xffffffff,0)
 25323 java     RET   mmap -1 errno 22 Invalid argument
 25323 java     CALL  write(0x1,0x7fffffbfd450,0x2b)
 25323 java     GIO   fd 1 wrote 43 bytes
       "Error occurred during initialization of VM
       "
 25323 java     RET   write 43/0x2b
 25323 java     CALL  write(0x1,0x80209a1e2,0x2d)
 25323 java     GIO   fd 1 wrote 45 bytes
       "Could not reserve enough space for code cache"

It seems that MAP_NORESERVE presence could be detected in sys/mman.h and then it
is used for some reason.
I guess that the port can be easily fixed, but this commit breaks compatibility
with older binaries.  Perhaps MAP_NORESERVE should be removed as well given that
we do not actually implement it.

>   Reviewed by:	alc, kib
>   MFC after:	2 weeks
>   Differential Revision:	https://reviews.freebsd.org/D698
> 
> Modified:
>   head/lib/libc/sys/mmap.2
>   head/sys/vm/vm_mmap.c
> 
> Modified: head/lib/libc/sys/mmap.2
> ==============================================================================
> --- head/lib/libc/sys/mmap.2	Mon Sep 15 17:14:09 2014	(r271634)
> +++ head/lib/libc/sys/mmap.2	Mon Sep 15 17:20:13 2014	(r271635)
> @@ -28,7 +28,7 @@
>  .\"	@(#)mmap.2	8.4 (Berkeley) 5/11/95
>  .\" $FreeBSD$
>  .\"
> -.Dd June 19, 2014
> +.Dd September 15, 2014
>  .Dt MMAP 2
>  .Os
>  .Sh NAME
> @@ -372,6 +372,29 @@ The
>  argument
>  is not a valid open file descriptor.
>  .It Bq Er EINVAL
> +An invalid value was passed in the
> +.Fa prot
> +argument.
> +.It Bq Er EINVAL
> +An undefined option was set in the
> +.Fa flags
> +argument.
> +.It Bq Er EINVAL
> +Both
> +.Dv MAP_PRIVATE
> +and
> +.Dv MAP_SHARED
> +were specified.
> +.It Bq Er EINVAL
> +None of
> +.Dv MAP_ANON ,
> +.Dv MAP_PRIVATE ,
> +.Dv MAP_SHARED ,
> +or
> +.Dv MAP_STACK
> +was specified.
> +At least one of these flags must be included.
> +.It Bq Er EINVAL
>  .Dv MAP_FIXED
>  was specified and the
>  .Fa addr
> 
> Modified: head/sys/vm/vm_mmap.c
> ==============================================================================
> --- head/sys/vm/vm_mmap.c	Mon Sep 15 17:14:09 2014	(r271634)
> +++ head/sys/vm/vm_mmap.c	Mon Sep 15 17:20:13 2014	(r271635)
> @@ -203,17 +203,17 @@ sys_mmap(td, uap)
>  	struct vnode *vp;
>  	vm_offset_t addr;
>  	vm_size_t size, pageoff;
> -	vm_prot_t cap_maxprot, prot, maxprot;
> +	vm_prot_t cap_maxprot, maxprot;
>  	void *handle;
>  	objtype_t handle_type;
> -	int align, error, flags;
> +	int align, error, flags, prot;
>  	off_t pos;
>  	struct vmspace *vms = td->td_proc->p_vmspace;
>  	cap_rights_t rights;
>  
>  	addr = (vm_offset_t) uap->addr;
>  	size = uap->len;
> -	prot = uap->prot & VM_PROT_ALL;
> +	prot = uap->prot;
>  	flags = uap->flags;
>  	pos = uap->pos;
>  
> @@ -244,8 +244,23 @@ sys_mmap(td, uap)
>  		flags |= MAP_ANON;
>  		pos = 0;
>  	}
> +	/* XXX: MAP_RENAME, MAP_NORESERVE */
> +	if ((flags & ~(MAP_SHARED | MAP_PRIVATE | MAP_FIXED | MAP_HASSEMAPHORE |
> +	    MAP_STACK | MAP_NOSYNC | MAP_ANON | MAP_EXCL | MAP_NOCORE |
> +	    MAP_PREFAULT_READ |
> +#ifdef MAP_32BIT
> +	    MAP_32BIT |
> +#endif
> +	    MAP_ALIGNMENT_MASK)) != 0)
> +		return (EINVAL);
>  	if ((flags & (MAP_EXCL | MAP_FIXED)) == MAP_EXCL)
>  		return (EINVAL);
> +	if ((flags & (MAP_ANON | MAP_SHARED | MAP_PRIVATE)) == 0 ||
> +	    (flags & (MAP_SHARED | MAP_PRIVATE)) == (MAP_SHARED | MAP_PRIVATE))
> +		return (EINVAL);
> +	if (prot != PROT_NONE &&
> +	    (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC)) != 0)
> +		return (EINVAL);
>  
>  	/*
>  	 * Align the file position to a page boundary,
> @@ -415,6 +430,8 @@ sys_mmap(td, uap)
>  map:
>  	td->td_fpop = fp;
>  	maxprot &= cap_maxprot;
> +
> +	/* This relies on VM_PROT_* matching PROT_*. */
>  	error = vm_mmap(&vms->vm_map, &addr, size, prot, maxprot,
>  	    flags, handle_type, handle, pos);
>  	td->td_fpop = NULL;
> 


-- 
Andriy Gapon