Bug 71258 - [vm] [patch] anonymous mmappings not always page aligned
Summary: [vm] [patch] anonymous mmappings not always page aligned
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 4.10-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: John Baldwin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-09-01 18:50 UTC by Martin Kammerhofer
Modified: 2010-05-30 20:30 UTC (History)
0 users

See Also:


Attachments
vmmmap.c.patch.txt (528 bytes, text/plain)
2009-11-08 02:23 UTC, Alexander Best
no flags Details
mmap.2.patch.txt (729 bytes, text/plain)
2009-11-08 02:50 UTC, Alexander Best
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Kammerhofer 2004-09-01 18:50:26 UTC
Quote from the mmap(2) manpage:

     MAP_ANON          Map anonymous memory not associated with any specific
		       file.  The file descriptor used for creating MAP_ANON
		       must be -1.  The offset argument is ignored.
		                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The actual implementation _does_ use the offset argument. The offset
modulo the hardware page size is used for size calculation and added
to the return value.

(This should be no problem with POSIX conforming applications because
POSIX _mandates_ EINVAL for nonaligned offsets.)

Fix: 

--=_3r61v0pks0u8----XuOZ7nQp1eoNA4OvFDYacqocJY98tCIcV0jvNAx4qyy7A18f
Content-Type: text/plain; name="file.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="file.diff"

--- vm_mmap.c.orig	Thu Aug  5 09:04:33 2004
+++ vm_mmap.c	Tue Aug 31 12:47:11 2004
@@ -227,25 +227,28 @@
 
 	if (flags & MAP_STACK) {
 		if ((uap->fd != -1) ||
 		    ((prot & (PROT_READ | PROT_WRITE)) != (PROT_READ | PROT_WRITE)))
 			return (EINVAL);
 		flags |= MAP_ANON;
-		pos = 0;
 	}
 
-	/*
-	 * Align the file position to a page boundary,
-	 * and save its page offset component.
-	 */
-	pageoff = (pos & PAGE_MASK);
-	pos -= pageoff;
+	if (flags & MAP_ANON) {
+		pageoff = pos = 0;
+	} else {
+		/*
+		 * Align the file position to a page boundary,
+		 * and save its page offset component.
+		 */
+		pageoff = (pos & PAGE_MASK);
+		pos -= pageoff;
 
-	/* Adjust size for rounding (on both ends). */
-	size += pageoff;			/* low end... */
-	size = (vm_size_t) round_page(size);	/* hi end */
+		/* Adjust size for rounding (on both ends). */
+		size += pageoff;			/* low end... */
+		size = (vm_size_t) round_page(size);	/* hi end */
+	}
 
 	/*
 	 * Check for illegal addresses.  Watch out for address wrap... Note
 	 * that VM_*_ADDRESS are not constants due to casts (argh).
 	 */
 	if (flags & MAP_FIXED) {
@@ -284,13 +287,12 @@
 	if (flags & MAP_ANON) {
 		/*
 		 * Mapping blank space is trivial.
 		 */
 		handle = NULL;
 		maxprot = VM_PROT_ALL;
-		pos = 0;
 	} else {
 		/*
 		 * Mapping file, get fp for validation. Obtain vnode and make
 		 * sure it is of appropriate type.
 		 * don't let the descriptor disappear on us if we block
 		 */
How-To-Repeat: #include <sys/types.h>
#include <sys/mman.h>
#include <stdio.h>

main() {
    printf("%p\n", mmap(0, 0x1000, PROT_NONE, MAP_ANON, -1, 0x12345678));
}
Comment 1 K. Macy freebsd_committer 2007-11-16 09:53:24 UTC
State Changed
From-To: open->feedback


Is this still an issue on RELENG_7?
Comment 2 K. Macy freebsd_committer 2007-11-16 16:59:21 UTC
State Changed
From-To: feedback->open


Please comment on whether or not it is worth updating the docs 
or this is just blatant pilot error. 


Comment 3 K. Macy freebsd_committer 2007-11-16 16:59:21 UTC
Responsible Changed
From-To: freebsd-bugs->alc


Please comment on whether or not it is worth updating the docs  
or this is just blatant pilot error.
Comment 4 Martin Kammerhofer 2007-11-19 10:47:22 UTC
Changing the mmap(2) wording like
s/The offset argument is ignored/The offset argument must be zero/
should suffice.

HTH Martin
Comment 5 Alexander Best 2009-11-08 02:23:32 UTC
the problem described in this pr has been discussed with John Baldwin and Alan
Cox in this thread

http://lists.freebsd.org/pipermail/freebsd-hackers/2009-October/029773.html

attached is a new patch to deal with the problem. this changes the semantics
of mmap like so:

1) if MAP_ANON is defined and offset !=0 ====>  return EINVAL
2) if MAP_STACK is defined and offset !=0 ====> offset = 0

please set this pr into analysed state.

thanks.
alex
Comment 6 Alexander Best 2009-11-08 02:50:44 UTC
this patch changes mmap(2) to reflect the changes made by the previous patch.

alex
Comment 7 Martin Kammerhofer 2009-11-08 12:53:42 UTC
Mmap(2) behavior with Alexander's patch is the standards conforming  
way of doing it.  My original patch made mmap(2) behave like the  
manpage said.

Wether mmap(2) ignores non-zero MAP_ANON offsets (my patch; matches  
current manpage) or returns EINVAL (Alexander's patch; POSIX specified  
behavior) is not important to me.  Standards conformance is good, but  
doing it the POSIX way incurs a slight possibility of breaking  
existing software.
Any way, the manpage should describe mmap(2) behavior accurately!
Comment 8 Alexander Best 2009-11-09 19:29:27 UTC
jhb@ mentioned in this post

http://lists.freebsd.org/pipermail/freebsd-hackers/2009-November/029982.html

that of the following changes:

1) if MAP_ANON is defined and offset !=0 ====> return EINVAL
2) if MAP_STACK is defined and offset !=0 ====> offset = 0

2) isn't true, because that's the way mmap has always behaved. this leaves 1)
being the only patch-change in the mmap semantics.

alex
Comment 9 Mark Linimon freebsd_committer freebsd_triage 2010-02-26 22:26:54 UTC
State Changed
From-To: open->analyzed

A patch has been circulated and an approach agreed on.
Comment 10 dfilter service freebsd_committer 2010-03-23 21:08:17 UTC
Author: jhb
Date: Tue Mar 23 21:08:07 2010
New Revision: 205536
URL: http://svn.freebsd.org/changeset/base/205536

Log:
  Reject attempts to create a MAP_ANON mapping with a non-zero offset.
  
  PR:		kern/71258
  Submitted by:	Alexander Best
  MFC after:	2 weeks

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	Tue Mar 23 20:12:53 2010	(r205535)
+++ head/lib/libc/sys/mmap.2	Tue Mar 23 21:08:07 2010	(r205536)
@@ -105,7 +105,7 @@ The file descriptor used for creating
 must be \-1.
 The
 .Fa offset
-argument is ignored.
+argument must be 0.
 .\".It Dv MAP_FILE
 .\"Mapped from a regular file or character-special device memory.
 .It Dv MAP_ANONYMOUS
@@ -316,6 +316,11 @@ was equal to zero.
 was specified and the
 .Fa fd
 argument was not -1.
+.It Bq Er EINVAL
+.Dv MAP_ANON
+was specified and the
+.Fa offset
+argument was not 0.
 .It Bq Er ENODEV
 .Dv MAP_ANON
 has not been specified and

Modified: head/sys/vm/vm_mmap.c
==============================================================================
--- head/sys/vm/vm_mmap.c	Tue Mar 23 20:12:53 2010	(r205535)
+++ head/sys/vm/vm_mmap.c	Tue Mar 23 21:08:07 2010	(r205536)
@@ -233,7 +233,7 @@ mmap(td, uap)
 	/* Make sure mapping fits into numeric range, etc. */
 	if ((uap->len == 0 && !SV_CURPROC_FLAG(SV_AOUT) &&
 	     curproc->p_osrel >= 800104) ||
-	    ((flags & MAP_ANON) && uap->fd != -1))
+	    ((flags & MAP_ANON) && (uap->fd != -1 || pos != 0)))
 		return (EINVAL);
 
 	if (flags & MAP_STACK) {
@@ -300,7 +300,6 @@ mmap(td, uap)
 		handle = NULL;
 		handle_type = OBJT_DEFAULT;
 		maxprot = VM_PROT_ALL;
-		pos = 0;
 	} else {
 		/*
 		 * Mapping file, get fp for validation and
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 11 John Baldwin freebsd_committer freebsd_triage 2010-03-23 21:20:21 UTC
State Changed
From-To: analyzed->patched

A solution was applied to HEAD. 


Comment 12 John Baldwin freebsd_committer freebsd_triage 2010-03-23 21:20:21 UTC
Responsible Changed
From-To: alc->jhb

A solution was applied to HEAD.
Comment 13 John Baldwin freebsd_committer freebsd_triage 2010-04-14 16:23:30 UTC
State Changed
From-To: patched->closed

Fix merged to 7.
Comment 14 dfilter service freebsd_committer 2010-05-30 20:22:45 UTC
nox         2010-05-30 19:22:32 UTC

  FreeBSD ports repository

  Modified files:
    emulators/qemu       Makefile 
    emulators/qemu/files patch-osdep.c 
  Log:
  - Avoid using mmap MAP_ANON with a non-zero offset.  [1]
  - Bump PORTREVISION.
  
  PR:             kern/71258 [1]
  
  Revision  Changes    Path
  1.112     +1 -1      ports/emulators/qemu/Makefile
  1.5       +8 -1      ports/emulators/qemu/files/patch-osdep.c
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"