mmap breakage?

[Alan Cox]

On Fri, Apr 02, 2004 at 11:59:57AM -0800, Alfred Perlstein wrote:
> * Poul-Henning Kamp <phk at phk.freebsd.dk> [040402 11:14] wrote:
> > In message <20040402191254.GK26131 at elvis.mu.org>, Alfred Perlstein writes:
> > >* Poul-Henning Kamp <phk at phk.freebsd.dk> [040402 10:40] wrote:
> > >> In message <20040402160959.GJ26131 at 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


-------------- next part --------------
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.