NetBSD Problem Report #48910

From www@NetBSD.org  Sun Jun 15 11:49:59 2014
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(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 1E59CA6549
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 15 Jun 2014 11:49:59 +0000 (UTC)
Message-Id: <20140615114957.BCB76A6550@mollari.NetBSD.org>
Date: Sun, 15 Jun 2014 11:49:57 +0000 (UTC)
From: yaneurabeya@gmail.com
Reply-To: yaneurabeya@gmail.com
To: gnats-bugs@NetBSD.org
Subject: msync(2) should return ENOMEM on unmapped pages per POSIX; test criteria in t_msync incorrect
X-Send-Pr-Version: www-1.0

>Number:         48910
>Category:       kern
>Synopsis:       msync(2) should return ENOMEM on unmapped pages per POSIX; test criteria in t_msync incorrect
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jun 15 11:50:00 +0000 2014
>Last-Modified:  Wed Jun 18 03:10:00 +0000 2014
>Originator:     Garrett Cooper
>Release:        n/a
>Organization:
EMC / Isilon Storage Division
>Environment:
n/a
>Description:
The msync_err testcase checks for EFAULT when testing msync(2) with an previously mapped address, but that requirement isn't listed in the msync(2) manpage, nor is it listed as the correct error to throw per POSIX:

165 ATF_TC_BODY(msync_err, tc)
166 {
...
176         errno = 0;
177 
178         /*
179          * Map a page and then unmap to get an unmapped address.
180          */
181         map = mmap(NULL, page, PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE,
182             -1, 0);
183         ATF_REQUIRE(map != MAP_FAILED);
184 
185         (void)munmap(map, page);
186 
187         ATF_REQUIRE(msync(map, page, MS_SYNC) != 0);
188         ATF_REQUIRE(errno == EFAULT);
189 }

The errno that should be checked is ENOMEM (from http://pubs.opengroup.org/onlinepubs/000095399/functions/msync.html ):

[ENOMEM]
The addresses in the range starting at addr and continuing for len bytes are outside the range allowed for the address space of a process or specify one or more pages that are not mapped.
>How-To-Repeat:
- Change the errno in t_msync from EFAULT to ENOMEM.
- Recompile the testcase.
- Re-run the testcase.
>Fix:

>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/48910: msync(2) should return ENOMEM on unmapped pages per POSIX; test criteria in t_msync incorrect
Date: Sun, 15 Jun 2014 16:18:41 +0200

 Besides the obvious test changes, this would need a kernel change - untested,
 but this should do it.

 We could go over the other callers of uvm_map_clean() instead and change
 the return value in there as well - what do others think?

 Martin

 Index: uvm_mmap.c
 ===================================================================
 RCS file: /cvsroot/src/sys/uvm/uvm_mmap.c,v
 retrieving revision 1.148
 diff -u -r1.148 uvm_mmap.c
 --- uvm_mmap.c	25 Jan 2014 17:30:45 -0000	1.148
 +++ uvm_mmap.c	15 Jun 2014 14:16:43 -0000
 @@ -658,7 +658,7 @@
  		uvmflags |= PGO_SYNCIO;

  	error = uvm_map_clean(map, addr, addr+size, uvmflags);
 -	return error;
 +	return error == EFAULT ? ENOMEM : error;
  }

  /*

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/48910: msync(2) should return ENOMEM on unmapped pages per
 POSIX; test criteria in t_msync incorrect
Date: Tue, 17 Jun 2014 05:07:12 +0000

 On Sun, Jun 15, 2014 at 02:20:00PM +0000, Martin Husemann wrote:
  >  We could go over the other callers of uvm_map_clean() instead and change
  >  the return value in there as well - what do others think?

 I think repurposing ENOMEM for this is a POSIX_MISTAKE; not only does
 it have a well-established differeng meaning, but I don't see any
 reason trying to sync nonexistent memory shouldn't produce EFAULT or
 even SIGSEGV.

 ...so while I suppose we should be compliant, I don't think it's a
 good idea to go around changing it elsewhere.

 -- 
 David A. Holland
 dholland@netbsd.org

From: Garrett Cooper <yaneurabeya@gmail.com>
To: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>
Cc: kern-bug-people@netbsd.org, 
	"gnats-admin@netbsd.org" <gnats-admin@netbsd.org>, "netbsd-bugs@netbsd.org" <netbsd-bugs@netbsd.org>
Subject: Re: kern/48910: msync(2) should return ENOMEM on unmapped pages per
 POSIX; test criteria in t_msync incorrect
Date: Tue, 17 Jun 2014 14:33:31 -0700

 On Mon, Jun 16, 2014 at 10:10 PM, David Holland
 <dholland-bugs@netbsd.org> wrote:
 > The following reply was made to PR kern/48910; it has been noted by GNATS.
 >
 > From: David Holland <dholland-bugs@netbsd.org>
 > To: gnats-bugs@NetBSD.org
 > Cc:
 > Subject: Re: kern/48910: msync(2) should return ENOMEM on unmapped pages per
 >  POSIX; test criteria in t_msync incorrect
 > Date: Tue, 17 Jun 2014 05:07:12 +0000
 >
 >  On Sun, Jun 15, 2014 at 02:20:00PM +0000, Martin Husemann wrote:
 >   >  We could go over the other callers of uvm_map_clean() instead and change
 >   >  the return value in there as well - what do others think?
 >
 >  I think repurposing ENOMEM for this is a POSIX_MISTAKE; not only does
 >  it have a well-established differeng meaning, but I don't see any
 >  reason trying to sync nonexistent memory shouldn't produce EFAULT or
 >  even SIGSEGV.
 >
 >  ...so while I suppose we should be compliant, I don't think it's a
 >  good idea to go around changing it elsewhere.

 The manpages for some of the calls (e.g. *madvise(2), etc) don't state
 what the behavior should be though for cases where POSIX uses ENOMEM
 to denote exceptional "out of bounds" address conditions. David
 brought up a good point: should it be EFAULT or ENOMEM, and if EFAULT,
 shouldn't this variance be documented in the manpage as not strictly
 adhering to the POSIX standard?

 I did some tracking based on what Martin provided and I believe
 (please correct me if I'm wrong :/!!)...

 - mlock follows POSIX by returning ENOMEM.
 - mprotect might not be POSIX compliant with respect to the memory
 boundary requirement, like msync (
 http://pubs.opengroup.org/onlinepubs/009696799/functions/mprotect.html
 ):

 [ENOMEM]Addresses in the range [addr,addr+len) are invalid for the
 address space of a process, or specify one or more pages which are not
 mapped.

 Looking around that file and then studying the NetBSD/POSIX manpages a
 bit, mmap seems suspicious because it relies on the range_check static
 function for checking the range of the pages and range_check returns
 EFBIG for mmap which isn't documented in mmap(2) for NetBSD (and it's
 not in POSIX either). I'm not 100% clued into NetBSD kernel internals
 (only FreeBSD) so I need some help here -- does EFBIG get punted back
 to userland as EFAULT?

 Thank you in advance for the help :)!
 -Garrett

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/48910: msync(2) should return ENOMEM on unmapped pages per
 POSIX; test criteria in t_msync incorrect
Date: Wed, 18 Jun 2014 03:05:01 +0000

 On Tue, Jun 17, 2014 at 09:35:00PM +0000, Garrett Cooper wrote:
  >  Looking around that file and then studying the NetBSD/POSIX manpages a
  >  bit, mmap seems suspicious because it relies on the range_check static
  >  function for checking the range of the pages and range_check returns
  >  EFBIG for mmap which isn't documented in mmap(2) for NetBSD (and it's
  >  not in POSIX either). I'm not 100% clued into NetBSD kernel internals
  >  (only FreeBSD) so I need some help here -- does EFBIG get punted back
  >  to userland as EFAULT?

 It does not; that sounds broken.

 -- 
 David A. Holland
 dholland@netbsd.org

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-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.