| Summary: | mmap with PROT_NONE, but still could be read | ||
|---|---|---|---|
| Product: | Base System | Reporter: | gordon <hddai> |
| Component: | kern | Assignee: | Alan Cox <alc> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | ||
| Priority: | Normal | ||
| Version: | Unspecified | ||
| Hardware: | Any | ||
| OS: | Any | ||
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 <err.h>
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <sys/types.h>
#include <sys/mman.h>
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),
Mark's patch with an additional comment and one minor change has been commited to current: Revision Changes Path 1.333 +5 -4 src/sys/vm/vm_map.c 1.109 +1 -1 src/sys/vm/vm_map.h I've asked for a volunteer to port and commit this change to RELENG_4. State Changed From-To: open->patched Patch applied to -CURRENT. Responsible Changed From-To: freebsd-bugs->alc Patch applied to -CURRENT. State Changed From-To: patched->closed Merged to relevant branches, closing the ticket (note no merging will occur for 4.x it is dead). |
I have ported a test program for LSB test suites to test the compatibility of mmap system call. As to Open Group Technical Standard System Interfaces and Headers, Issue 5, when mmap with PROT_NONE, the mapped pages should not be allowed to access. But as the program shows, the mapped page could still be read. Can anyone tell me how to solve this problem? #include <stdio.h> #include <unistd.h> #include <setjmp.h> #include <signal.h> #include <fcntl.h> #include <errno.h> #include <sys/mman.h> #include <sys/types.h> #include <sys/stat.h> static int vsrt_mmap_setup(void); static void vsrt_mmapsig_fn(int); static void vsrt_invalid_sig_fn(int); static int vsrt_signal(int , void (*handler)(int)); static int vsrt_open_file(char* s, int flag, mode_t mode, int npages); typedef struct { char name[10]; int signal; int flags; } vsrt_siglst; static vsrt_siglst vsrt_signals[] = { { "SIGFPE", SIGFPE, 0 }, { "SIGILL", SIGILL, 0 }, }; #define VSRT_NUM_SIGNALS (sizeof(vsrt_signals) / sizeof(vsrt_siglst)) #define VSRT_PROT_ALL (mode_t)(S_IRWXU|S_IRWXG|S_IRWXO) static char* test_file = "./dhd"; static int vsrt_got_sigsegv = 0; static int vsrt_got_sigbus = 0; static int vsrt_got_sig = 0; static int vsrt_setjmp_called = 0; static unsigned long vsrt_pgsz, vsrt_ipgsz; static sigjmp_buf vsrt_env; static struct sigaction Sigaction; main() { int fd, err; pid_t pid; int *addr; volatile int i; if (vsrt_mmap_setup() == -1){ printf("Setup error!\n"); return; } fd = vsrt_open_file(test_file, O_RDWR, VSRT_PROT_ALL, 2); if (fd == -1){ printf("Error in open file\n"); return; } if ((addr = (int *)mmap(0, vsrt_pgsz, PROT_NONE, MAP_SHARED, fd, (off_t)vsrt_pgsz)) == (int *) (-1)) { err = errno; printf("mmap failed, errno = %d \n", err); (void)close(fd); (void)unlink(test_file); return; } if ((pid = fork()) == 0){ vsrt_setjmp_called = 1; if(sigsetjmp(vsrt_env,1) ==0) i = *addr; if (!(vsrt_got_sigsegv || vsrt_got_sigbus)) { printf("Mapped page could be accessed\n"); exit(1); } else printf("Mapped page could not be accessed\n"); exit (0); } else if (pid == -1){ printf("fork failed\n"); exit(0); } else wait((int *)0); close(fd); munmap((void*) addr, (size_t)vsrt_pgsz); unlink(test_file); } int vsrt_mmap_setup(void) { int i; if ((vsrt_pgsz = sysconf(_SC_PAGESIZE)) == -1) { printf("Error to get pagesize\n"); return -1; } vsrt_ipgsz = vsrt_pgsz/sizeof(int); vsrt_got_sigsegv = vsrt_got_sigbus = vsrt_got_sig = 0; for (i = 0; i < VSRT_NUM_SIGNALS; i++) { if (vsrt_signal(vsrt_signals[i].signal, &vsrt_invalid_sig_fn) == -1) return -1; } if (vsrt_signal(SIGSEGV, vsrt_mmapsig_fn) == -1) return -1; if (vsrt_signal(SIGBUS, vsrt_mmapsig_fn) == -1) return -1; return 0; } int vsrt_signal(int sig, void (*handler)(int)) { int rval; rval = sigemptyset(&(Sigaction.sa_mask)); if (rval == -1) { printf("Error to sigemptyset\n"); return -1; } Sigaction.sa_handler = handler; Sigaction.sa_flags = 0; rval = sigaction(sig,&Sigaction,(struct sigaction *)NULL); if (rval == -1) { printf("Error in sigaction\n"); return -1; } return 0; } static void vsrt_invalid_sig_fn(int s) { int i; for (i = 0; i < VSRT_NUM_SIGNALS; i++) { if (s == vsrt_signals[i].signal) { printf("Invalid signal: %d(%s) received\n", s, vsrt_signals[i].name); return; } } } static void vsrt_mmapsig_fn(int s) { if (s == SIGSEGV) vsrt_got_sigsegv++; else if (s == SIGBUS) vsrt_got_sigbus++; vsrt_got_sig++; if (vsrt_setjmp_called) { vsrt_setjmp_called = 0; siglongjmp(vsrt_env, 1); } } int vsrt_open_file(char* s, int flag, mode_t mode, int npages) { int fd, *buf, i, j; unlink(s); if ((fd = open(s, (O_RDWR|O_CREAT|O_TRUNC), (mode_t)(S_IRWXU|S_IRWXG|S_IRWXO))) == -1) { printf("open() failed, errno = %d \n", errno); return -1; } if ((buf = (int *)malloc(sizeof(int) * vsrt_ipgsz * npages)) == NULL) { printf("malloc() failed, errno = %d \n", errno); return -1; } for (j = 0; j < vsrt_ipgsz*npages; j += sizeof(buf)/sizeof(buf[0])) { for (i = 0; i < sizeof(buf)/sizeof(buf[0]); i++) buf[i] = i + j; if (write(fd, (void*)buf, sizeof(buf)) != sizeof(buf)) { printf("write() failed, errno = %d \n", errno ); return -1; } } (void)close(fd); if (chmod(s, mode) == -1) { printf("chmod() failed, errno = %d \n", errno); return -1; } if ((fd = open(s, flag)) == -1) { printf("open() failed, errno = %d \n", errno); return -1; } return (fd); } How-To-Repeat: The ported program is attached in the Full Description part.