From owner-freebsd-bugs@FreeBSD.ORG Tue Apr 13 18:40:20 2004 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7F22216A4CE for ; Tue, 13 Apr 2004 18:40:20 -0700 (PDT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6110843D4C for ; Tue, 13 Apr 2004 18:40:20 -0700 (PDT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) i3E1eKbv099416 for ; Tue, 13 Apr 2004 18:40:20 -0700 (PDT) (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.10/8.12.10/Submit) id i3E1eKh6099415; Tue, 13 Apr 2004 18:40:20 -0700 (PDT) (envelope-from gnats) Date: Tue, 13 Apr 2004 18:40:20 -0700 (PDT) Message-Id: <200404140140.i3E1eKh6099415@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: "Mark W. Krentel" Subject: Re: kern/64573: mmap with PROT_NONE, but still could be read X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: "Mark W. Krentel" List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Apr 2004 01:40:20 -0000 The following reply was made to PR kern/64573; it has been noted by GNATS. From: "Mark W. Krentel" To: bug-followup@freebsd.org Cc: alc@freebsd.org, hddai@163.net Subject: Re: kern/64573: mmap with PROT_NONE, but still could be read Date: Tue, 13 Apr 2004 21:35:38 -0400 First, there is a much simpler program to reproduce the bug. This is from 5.2-current as of April 6, 2004. /* * Test mmap() and PROT_NONE. The first printf() should fail with seg * fault or bus error because the segment has protection PROT_NONE, * but actually, it runs ok. */ #include #include #include #include #include #include int main() { int fd; char *p; fd = open("myfile", O_RDWR); if (fd < 0) err(1, "open failed"); p = mmap(NULL, 4096, PROT_NONE, MAP_SHARED, fd, 0); if (p == MAP_FAILED) err(1, "mmap failed"); printf("Reading p[4] = %d\n", p[4]); printf("Success\n"); return 0; } % yes abcdefg | dd of=myfile bs=1024 count=4 4+0 records in 4+0 records out 4096 bytes transferred in 0.000223 secs (18354561 bytes/sec) % hd myfile 00000000 61 62 63 64 65 66 67 0a 61 62 63 64 65 66 67 0a |abcdefg.abcdefg.| * 00001000 % ./a.out Reading p[4] = 101 Success % The problem is that mmap() calls vm_mmap() and the non-MAP_STACK case calls vm_map_find() which calls vm_map_insert() which calls vm_map_pmap_enter() which calls pmap_enter_quick() and pmap_enter_quick() assumes that the segment always has read access. See: pmap_enter_quick() in sys/i386/i386/pmap.c. A similar bug happens with madvise(..., MADV_WILLNEED). Change the above program to call open() followed by mmap(..., PROT_READ, ...) followed by mprotect(..., PROT_NONE). At this point, reading from the segment produces a bus error. But now add madvise(..., MADV_WILLNEED) and you can read the segment. Again, the problem is that madvise() calls vm_map_madvise() and the case for MADV_WILLNEED calls vm_map_pmap_enter() as before. I could only produce the bug with mmap() on a file and with read access. Write access and mmap() with MAP_ANON and MAP_STACK behaved as expected. The patch below adds a parameter "prot" to vm_map_pmap_enter() so that vm_map_pmap_enter() will avoid calling pmap_enter_quick() when the segment doesn't have read access. It makes a bit more sense for vm_map_pmap_enter() to make this decision rather than the calling program, and prot may be useful for other things. With much guidence from: alc --Mark ---------------- --- vm_map.h.orig Tue Apr 6 16:21:07 2004 +++ vm_map.h Tue Apr 13 17:16:14 2004 @@ -337,7 +337,7 @@ vm_pindex_t *, vm_prot_t *, boolean_t *); void vm_map_lookup_done (vm_map_t, vm_map_entry_t); boolean_t vm_map_lookup_entry (vm_map_t, vm_offset_t, vm_map_entry_t *); -void vm_map_pmap_enter(vm_map_t map, vm_offset_t addr, +void vm_map_pmap_enter(vm_map_t map, vm_offset_t addr, vm_prot_t prot, vm_object_t object, vm_pindex_t pindex, vm_size_t size, int flags); int vm_map_protect (vm_map_t, vm_offset_t, vm_offset_t, vm_prot_t, boolean_t); int vm_map_remove (vm_map_t, vm_offset_t, vm_offset_t); --- vm_map.c.orig Tue Apr 6 16:21:07 2004 +++ vm_map.c Tue Apr 13 17:53:27 2004 @@ -881,7 +881,7 @@ #endif if (cow & (MAP_PREFAULT|MAP_PREFAULT_PARTIAL)) { - vm_map_pmap_enter(map, start, + vm_map_pmap_enter(map, start, prot, object, OFF_TO_IDX(offset), end - start, cow & MAP_PREFAULT_PARTIAL); } @@ -1252,14 +1252,14 @@ * immediately after an mmap(2). */ void -vm_map_pmap_enter(vm_map_t map, vm_offset_t addr, +vm_map_pmap_enter(vm_map_t map, vm_offset_t addr, vm_prot_t prot, vm_object_t object, vm_pindex_t pindex, vm_size_t size, int flags) { vm_offset_t tmpidx; int psize; vm_page_t p, mpte; - if (object == NULL) + if ((object == NULL) || ((prot & VM_PROT_READ) == 0)) return; mtx_lock(&Giant); VM_OBJECT_LOCK(object); @@ -1551,6 +1551,7 @@ if (behav == MADV_WILLNEED) { vm_map_pmap_enter(map, useStart, + current->protection, current->object.vm_object, pindex, (count << PAGE_SHIFT),