Date: Sun, 9 Mar 2008 15:12:07 -0700 From: "Kip Macy" <kip.macy@gmail.com> To: "Jonathan Chen" <jon@freebsd.org> Cc: hackers@freebsd.org Subject: Re: mlock & COW Message-ID: <b1fa29170803091512v6e0b9ab6v24f3ce3659a714f8@mail.gmail.com> In-Reply-To: <20080309212441.GA56523@porthos.spock.org> References: <20080309212441.GA56523@porthos.spock.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Mar 9, 2008 at 2:24 PM, Jonathan Chen <jon@freebsd.org> wrote:
> I've been battling with a bug related to mlock and COW pages. Since I'm
> basically clueless when it comes to the VM subsystem, I was hoping someone
> with more clue can look at my fix and let me know if I'm doing the right
> thing here.
>
> The problem: User programs will crash (SEGV/BUS) if COW pages become
> writable after the pages are mlocked. This happens if shared libraries is
> loaded in a program after a call to mlockall(MCL_FUTURE), and can be seen
> with amd and ntpd when nss_ldap is used in the system. Included at the end
> of this message is a sample program that demonstrates the problem.
I think the problem is mlock wires the pages that are backing those
mappings. So you're writing to a page that you don't have write access
to. You're best off ensuring that you have a private copy of all those
pages before mlocking().
>
> The "solution": Forcibly wire the page via vm_fault_wire() when the page
> protection bits are changed. This seems to make the problem disappear, but
> I'm not sure if causing a fault on vm_map_protect() is a good thing to do.
Uhm no. Don't do that.
> Am I correct in thinking vm_fault_wire() will cause the COW page to be
> copied immediately? I think this is the right thing to do, given the page
> is supposed to be wired, but I'm not sure if somewhere in the vm fault
> routines might be a better place to put the fix. I'm also not sure what
> the last argument to vm_fault_wire() (fictitious) means, I just copied the
> argument from another invocation. My patch (against 7-STABLE) is included
> below.
I'm not certain what the correct semantics are. You're patch *may* be OK.
-Kip
>
> I'll commit this if someone blesses the fix as the right thing.
>
>
> Index: sys/vm/vm_map.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/vm/vm_map.c,v
> retrieving revision 1.388.2.3
> diff -u -p -r1.388.2.3 vm_map.c
> --- sys/vm/vm_map.c 18 Jan 2008 10:02:53 -0000 1.388.2.3
> +++ sys/vm/vm_map.c 9 Mar 2008 20:55:50 -0000
> @@ -1621,6 +1621,15 @@ vm_map_protect(vm_map_t map, vm_offset_t
> current->end,
> current->protection & MASK(current));
> #undef MASK
> + if ((entry->eflags &
> + (MAP_ENTRY_USER_WIRED|MAP_ENTRY_COW)) ==
> + (MAP_ENTRY_USER_WIRED|MAP_ENTRY_COW)) {
> + vm_fault_wire(map, current->start,
> + current->end, TRUE,
> + entry->object.vm_object != NULL &&
> + entry->object.vm_object->type ==
> + OBJT_DEVICE);
> + }
> }
> vm_map_simplify_entry(map, current);
> current = current->next;
>
>
>
>
> bug demonstration code
> ===================================================================
>
> #include <unistd.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <strings.h>
> #include <fcntl.h>
> #include <sys/mman.h>
>
> char *b;
>
> int main() {
> int fd = open("/usr/lib/libc.a", O_RDONLY);
> if (fd < 0) {
> perror("open");
> exit(1);
> }
> b = mmap(0, 1024, PROT_READ|PROT_EXEC, MAP_PRIVATE, fd, 0);
> if (b == MAP_FAILED) {
> perror("mmap");
> exit(0);
> }
> if (mlock(b, 1024) != 0) {
> perror("mlock");
> }
> if (mprotect(b, 1024, PROT_READ|PROT_WRITE|PROT_EXEC) != 0) {
> perror("mprotect");
> }
> printf("memset crash now\n");
> memset(b, 1, 1024);
> printf("still alive\n");
> }
>
> -Jon
> _______________________________________________
> freebsd-hackers@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?b1fa29170803091512v6e0b9ab6v24f3ce3659a714f8>