NetBSD Problem Report #50985

From martin@duskware.de  Sun Mar 20 12:45:01 2016
Return-Path: <martin@duskware.de>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 6D2717ABDF
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 20 Mar 2016 12:45:01 +0000 (UTC)
Date: Sun, 20 Mar 2016 13:43:50 CET
From: martin@NetBSD.org
Reply-To: martin@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: uvm_mmap.c::range_test should use runtime limits of the vmspace?
X-Send-Pr-Version: 3.95

>Number:         50985
>Category:       kern
>Synopsis:       uvm_mmap.c::range_test should use runtime limits of the vmspace?
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    martin
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Mar 20 12:50:00 +0000 2016
>Closed-Date:    Tue May 24 20:22:15 +0000 2016
>Last-Modified:  Tue May 24 20:25:01 +0000 2016
>Originator:     Martin Husemann
>Release:        NetBSD 7.99.26
>Organization:
The NetBSD Foundation, Inc.
>Environment:
System: NetBSD night-owl.duskware.de 7.99.26 NetBSD 7.99.26 (NIGHT-OWL) #393: Fri Mar 11 14:19:47 CET 2016 martin@night-owl.duskware.de:/usr/src/sys/arch/amd64/compile/NIGHT-OWL amd64
Architecture: x86_64
Machine: amd64
>Description:

When creating a vmspace, we take various things (like the binary we are going
to execute) into account. This may result in different min/max values for
the virtual address in the new vmspace.

However, in uvm_mmap.c::range_test() we do not check this (potential differing)
values, but compare against the compile time defaults VM_MIN_ADDRESS
and VM_MAXUSER_ADDRESS.

Differences might be seen on some emulations, or for example with old
sparc64 binaries compiled with a restricted compiler memory model.

>How-To-Repeat:
code inspection

>Fix:
Pass the vmspace to the function and use its vm_map headers .end/.start members
instead? Maybe create accessor macros symetric to vm_map_setmin/vm_map_setmax.

>Release-Note:

>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: Chuck Silvers <chs@NetBSD.org>
Subject: Re: kern/50985: uvm_mmap.c::range_test should use runtime limits of the vmspace?
Date: Mon, 21 Mar 2016 21:32:32 +0100

 An (untestested) simple/mechanical fix would be the change below.

 Chuck, care to review?

 Martin

 Index: uvm_mmap.c
 ===================================================================
 RCS file: /cvsroot/src/sys/uvm/uvm_mmap.c,v
 retrieving revision 1.154
 diff -u -p -r1.154 uvm_mmap.c
 --- uvm_mmap.c	26 Nov 2015 13:15:34 -0000	1.154
 +++ uvm_mmap.c	21 Mar 2016 20:27:12 -0000
 @@ -70,10 +70,10 @@ static int uvm_mmap(struct vm_map *, vad
  		    int, int, struct uvm_object *, voff_t, vsize_t);

  static int
 -range_test(vaddr_t addr, vsize_t size, bool ismmap)
 +range_test(struct vm_map *map, vaddr_t addr, vsize_t size, bool ismmap)
  {
 -	vaddr_t vm_min_address = VM_MIN_ADDRESS;
 -	vaddr_t vm_max_address = VM_MAXUSER_ADDRESS;
 +	vaddr_t vm_min_address = vm_map_min(map);
 +	vaddr_t vm_max_address = vm_map_max(map);
  	vaddr_t eaddr = addr + size;
  	int res = 0;

 @@ -356,7 +356,7 @@ sys_mmap(struct lwp *l, const struct sys
  		if (addr & PAGE_MASK)
  			return (EINVAL);

 -		error = range_test(addr, size, true);
 +		error = range_test(&p->p_vmspace->vm_map, addr, size, true);
  		if (error) {
  			return error;
  		}
 @@ -487,16 +487,16 @@ sys___msync13(struct lwp *l, const struc
  	size += pageoff;
  	size = (vsize_t)round_page(size);

 -	error = range_test(addr, size, false);
 -	if (error)
 -		return error;

  	/*
  	 * get map
  	 */
 -
  	map = &p->p_vmspace->vm_map;

 +	error = range_test(map, addr, size, false);
 +	if (error)
 +		return error;
 +
  	/*
  	 * XXXCDC: do we really need this semantic?
  	 *
 @@ -573,12 +573,12 @@ sys_munmap(struct lwp *l, const struct s
  	if (size == 0)
  		return (0);

 -	error = range_test(addr, size, false);
 +	map = &p->p_vmspace->vm_map;
 +
 +	error = range_test(map, addr, size, false);
  	if (error)
  		return error;

 -	map = &p->p_vmspace->vm_map;
 -
  	/*
  	 * interesting system call semantic: make sure entire range is
  	 * allocated before allowing an unmap.
 @@ -634,7 +634,7 @@ sys_mprotect(struct lwp *l, const struct
  	size += pageoff;
  	size = round_page(size);

 -	error = range_test(addr, size, false);
 +	error = range_test(&p->p_vmspace->vm_map, addr, size, false);
  	if (error)
  		return error;

 @@ -675,7 +675,7 @@ sys_minherit(struct lwp *l, const struct
  	size += pageoff;
  	size = (vsize_t)round_page(size);

 -	error = range_test(addr, size, false);
 +	error = range_test(&p->p_vmspace->vm_map, addr, size, false);
  	if (error)
  		return error;

 @@ -716,7 +716,7 @@ sys_madvise(struct lwp *l, const struct 
  	size += pageoff;
  	size = (vsize_t)round_page(size);

 -	error = range_test(addr, size, false);
 +	error = range_test(&p->p_vmspace->vm_map, addr, size, false);
  	if (error)
  		return error;

 @@ -816,7 +816,7 @@ sys_mlock(struct lwp *l, const struct sy
  	size += pageoff;
  	size = (vsize_t)round_page(size);

 -	error = range_test(addr, size, false);
 +	error = range_test(&p->p_vmspace->vm_map, addr, size, false);
  	if (error)
  		return error;

 @@ -867,7 +867,7 @@ sys_munlock(struct lwp *l, const struct 
  	size += pageoff;
  	size = (vsize_t)round_page(size);

 -	error = range_test(addr, size, false);
 +	error = range_test(&p->p_vmspace->vm_map, addr, size, false);
  	if (error)
  		return error;


From: matthew green <mrg@eterna.com.au>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org, martin@NetBSD.org
Subject: re: kern/50985: uvm_mmap.c::range_test should use runtime limits of the vmspace?
Date: Tue, 22 Mar 2016 07:48:56 +1100

 >  -	vaddr_t vm_min_address = VM_MIN_ADDRESS;
 >  -	vaddr_t vm_max_address = VM_MAXUSER_ADDRESS;
 >  +	vaddr_t vm_min_address = vm_map_min(map);
 >  +	vaddr_t vm_max_address = vm_map_max(map);

 the max address now is in the kernel space on some platforms, i think.


 .mrg.

From: Martin Husemann <martin@duskware.de>
To: matthew green <mrg@eterna.com.au>
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/50985: uvm_mmap.c::range_test should use runtime limits of the vmspace?
Date: Mon, 21 Mar 2016 22:13:34 +0100

 On Tue, Mar 22, 2016 at 07:48:56AM +1100, matthew green wrote:
 > >  -	vaddr_t vm_min_address = VM_MIN_ADDRESS;
 > >  -	vaddr_t vm_max_address = VM_MAXUSER_ADDRESS;
 > >  +	vaddr_t vm_min_address = vm_map_min(map);
 > >  +	vaddr_t vm_max_address = vm_map_max(map);
 > 
 > the max address now is in the kernel space on some platforms, i think.

 It comes from exec paccks ep_vm_maxaddr, which usually is VM_MAXUSER_ADDRESS,
 I think.

 We can min()/max() it with VM_MAXUSER_ADDRESS/VM_MIN_ADDRESS though, or
 KASSERT that the limit is within the static limit.

 Martin

From: Chuck Silvers <chuq@chuq.com>
To: Martin Husemann <martin@duskware.de>
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/50985: uvm_mmap.c::range_test should use runtime limits of
 the vmspace?
Date: Mon, 28 Mar 2016 08:51:35 -0700

 On Mon, Mar 21, 2016 at 09:32:32PM +0100, Martin Husemann wrote:
 > An (untestested) simple/mechanical fix would be the change below.
 > 
 > Chuck, care to review?

 looks fine to me.

 -Chuck

Responsible-Changed-From-To: kern-bug-people->martin
Responsible-Changed-By: martin@NetBSD.org
Responsible-Changed-When: Tue, 24 May 2016 20:22:15 +0000
Responsible-Changed-Why:
Fixed


State-Changed-From-To: open->closed
State-Changed-By: martin@NetBSD.org
State-Changed-When: Tue, 24 May 2016 20:22:15 +0000
State-Changed-Why:
Fixed


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/50985 CVS commit: src/sys/uvm
Date: Tue, 24 May 2016 20:20:57 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Tue May 24 20:20:57 UTC 2016

 Modified Files:
 	src/sys/uvm: uvm_mmap.c

 Log Message:
 PR kern/50985: use the runtime limits of the vmspace in range_test()
 instead of the compile time defaults for it.


 To generate a diff of this commit:
 cvs rdiff -u -r1.157 -r1.158 src/sys/uvm/uvm_mmap.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.