Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 4 Apr 2004 15:12:51 -0500
From:      Alan Cox <alc@cs.rice.edu>
To:        Alfred Perlstein <alfred@freebsd.org>
Cc:        current@freebsd.org
Subject:   Re: mmap breakage?
Message-ID:  <20040404201251.GP15786@cs.rice.edu>
In-Reply-To: <20040402195957.GN26131@elvis.mu.org>
References:  <20040402191254.GK26131@elvis.mu.org> <35917.1080933274@critter.freebsd.dk> <20040402195957.GN26131@elvis.mu.org>

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

--2/5bycvrmDh4d1IB
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Apr 02, 2004 at 11:59:57AM -0800, Alfred Perlstein wrote:
> * Poul-Henning Kamp <phk@phk.freebsd.dk> [040402 11:14] wrote:
> > In message <20040402191254.GK26131@elvis.mu.org>, Alfred Perlstein writes:
> > >* Poul-Henning Kamp <phk@phk.freebsd.dk> [040402 10:40] wrote:
> > >> In message <20040402160959.GJ26131@elvis.mu.org>, Alfred Perlstein writes:
> > >> >I noticed that mplayer seems to want to open /dev/zero with RDONLY
> > >> >but then tries to mmap it PROT_READ+PROT_WRITE and gets rejected.
> > >> 
> > >> Sounds like missing MAP_PRIVATE ?
> > >
> > >Nope, private is set.  It's just that the file is opened RDONLY,
> > >but PROT has PROT_WRITE in it.
> > 
> > That should be OK with MAP_PRIVATE as far as I interpret POLA.
> 
> Ok, but it's not. :)
 
Alfred,

Please try the attached patch.  This problems appears to be a
consequence of vm/vm_mmap.c revision 1.180, where as part of a much
needed code reorganization for locking the check for the /dev/zero
special case got placed after the permissions check for the general
case.  This patch simply delays the permissions check for devices
until you have the necessary lock to also check for /dev/zero.

Since kan@ authored the reorganization, I'm cc:'ing him on this
message for purposes of obtaining a review.  (At least one comment
should be updated to reflect my code changes.)

Regards,
Alan



--2/5bycvrmDh4d1IB
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="mmap_anon.patch"

Index: vm/vm_mmap.c
===================================================================
RCS file: /home/ncvs/src/sys/vm/vm_mmap.c,v
retrieving revision 1.186
diff -u -r1.186 vm_mmap.c
--- vm/vm_mmap.c	18 Mar 2004 20:58:51 -0000	1.186
+++ vm/vm_mmap.c	4 Apr 2004 10:55:23 -0000
@@ -341,14 +341,14 @@
 		 * permission although we opened it without asking
 		 * for it, bail out.
 		 */
-		if ((flags & MAP_SHARED) != 0 || vp->v_type == VCHR) {
+		if ((flags & MAP_SHARED) != 0) {
 			if ((fp->f_flag & FWRITE) != 0) {
 				maxprot |= VM_PROT_WRITE;
 			} else if ((prot & PROT_WRITE) != 0) {
 				error = EACCES;
 				goto done;
 			}
-		} else {
+		} else if (vp->v_type != VCHR || (fp->f_flag & FWRITE) != 0) {
 			maxprot |= VM_PROT_WRITE;
 		}
 		handle = (void *)vp;
@@ -1113,6 +1113,11 @@
 		/*
 		 * cdevs does not provide private mappings of any kind.
 		 */
+		if ((*maxprotp & VM_PROT_WRITE) == 0 &&
+		    (prot & PROT_WRITE) != 0) {
+			error = EACCES;
+			goto done;
+		}
 		/*
 		 * However, for XIG X server to continue to work,
 		 * we should allow the superuser to do it anyway.

--2/5bycvrmDh4d1IB--



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