lib/55177: mremap(MAP_REMAPDUP) fails after fork()

[carenas%gmail.com@localhost]

>Number:         55177
>Category:       lib
>Synopsis:       mremap(MAP_REMAPDUP) fails after fork()
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Apr 15 06:00:00 +0000 2020
>Originator:     Carlo Arenas
>Release:        NetBSD 8.2
>Organization:
>Environment:
System: NetBSD localhost 8.2 NetBSD 8.2 (GENERIC) #0: Tue Mar 31 05:08:40 UTC 2020 mkrepro%mkrepro.NetBSD.org@localhost:/usr/src/sys/arch/i386/compile/GENERIC i386
>Description:
an application that has obtained a pair of maps to the same physical
page as described in mremap(2) won't be able to use the RW map to
update the code to execute in the memory pointed by the RX map after
a fork() on neither the parent or the child.

the problem seems to be related to the way the pages are inherited
between processes by the fork(), and the fact that at least the RW
page is marked as COW and therefore uses a different physical page
when written again that is no longer linked to the RX map.

had reproduced the problem with 9 and current in both i386 and amd64
>How-To-Repeat:
to reproduce, apply following patch and run tests

Index: arch/Makefile.exec_prot
===================================================================
RCS file: /cvsroot/src/tests/lib/libc/arch/Makefile.exec_prot,v
retrieving revision 1.1
diff -u -r1.1 Makefile.exec_prot
--- arch/Makefile.exec_prot     18 Jul 2011 23:16:08 -0000      1.1
+++ arch/Makefile.exec_prot     14 Apr 2020 12:37:16 -0000
@@ -5,4 +5,4 @@
 EXEC_PROT_ARCHDIR=     ${.PARSEDIR}/${MACHINE_CPU}
 .PATH: ${EXEC_PROT_ARCHDIR}

-SRCS_EXEC_PROT=                exec_prot_support.c return_one.S
+SRCS_EXEC_PROT=                exec_prot_support.c return_one.S return_two.S
Index: arch/i386/return_two.S
===================================================================
RCS file: arch/i386/return_two.S
diff -N arch/i386/return_two.S
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ arch/i386/return_two.S      14 Apr 2020 12:37:16 -0000
@@ -0,0 +1,10 @@
+/*     $NetBSD: return_two.S,v 1.1 2011/07/18 23:16:09 carenas Exp $ */
+
+#include <machine/asm.h>
+
+RCSID("$NetBSD: return_two.S,v 1.1 2011/07/18 23:16:09 carenas Exp $");
+
+_ENTRY(return_two)
+       movl    $0x2,%eax
+       ret
+LABEL(return_two_end)
Index: common/exec_prot.h
===================================================================
RCS file: /cvsroot/src/tests/lib/libc/common/exec_prot.h,v
retrieving revision 1.1
diff -u -r1.1 exec_prot.h
--- common/exec_prot.h  18 Jul 2011 23:16:11 -0000      1.1
+++ common/exec_prot.h  14 Apr 2020 12:37:16 -0000
@@ -38,10 +38,12 @@
  */

 /*
- * Trivial MD shellcode that justs returns 1.
+ * Trivial MD shellcode that justs returns 1 or 2.
  */
 int return_one(void);     /* begin marker -- shellcode entry */
 int return_one_end(void); /* end marker */
+int return_two(void);     /* begin marker -- shellcode entry */
+int return_two_end(void); /* end marker */

 /*
  * MD callback to verify whether host offers executable space protection.
Index: sys/t_mprotect.c
===================================================================
RCS file: /cvsroot/src/tests/lib/libc/sys/t_mprotect.c,v
retrieving revision 1.8
diff -u -r1.8 t_mprotect.c
--- sys/t_mprotect.c    16 Jul 2019 17:29:18 -0000      1.8
+++ sys/t_mprotect.c    14 Apr 2020 12:37:16 -0000
@@ -383,6 +383,58 @@
        ATF_REQUIRE(munmap(map2, page) == 0);
 }

+ATF_TC(mprotect_mremap_fork_exec);
+ATF_TC_HEAD(mprotect_mremap_fork_exec, tc)
+{
+       atf_tc_set_md_var(tc, "descr",
+           "Test mremap(2)+fork(2)+mprotect(2) executable space protections");
+}
+
+ATF_TC_BODY(mprotect_mremap_fork_exec, tc)
+{
+       void *map, *map2;
+       pid_t pid;
+
+       /*
+        * Map a page read/write/exec and duplicate it.
+        * Map the copy executable.
+        * Copy a function to the writeable mapping and execute it
+        * Fork a child and wait for it
+        * Copy a different function to the writeable mapping and execute it
+        * The original function shouldn't be called
+        */
+
+       map = mmap(NULL, page, PROT_READ|PROT_WRITE|PROT_MPROTECT(PROT_EXEC),
+           MAP_ANON, -1, 0);
+       ATF_REQUIRE(map != MAP_FAILED);
+       map2 = mremap(map, page, NULL, page, MAP_REMAPDUP);
+       ATF_REQUIRE(map2 != MAP_FAILED);
+       ATF_REQUIRE(mprotect(map2, page, PROT_EXEC|PROT_READ) == 0);
+
+       memcpy(map, (void *)return_one,
+           (uintptr_t)return_one_end - (uintptr_t)return_one);
+       __builtin___clear_cache(map, (void *)((uintptr_t)map + page));
+
+       ATF_REQUIRE(((int (*)(void))map2)() == 1);
+
+       pid = fork();
+       ATF_REQUIRE(pid >= 0);
+
+       if (pid == 0)
+               _exit(0);
+
+       (void)wait(NULL);
+
+       memcpy(map, (void *)return_two,
+           (uintptr_t)return_two_end - (uintptr_t)return_two);
+       __builtin___clear_cache(map, (void *)((uintptr_t)map + page));
+
+       ATF_REQUIRE(((int (*)(void))map2)() == 2);
+
+       ATF_REQUIRE(munmap(map, page) == 0);
+       ATF_REQUIRE(munmap(map2, page) == 0);
+}
+
 ATF_TP_ADD_TCS(tp)
 {
        page = sysconf(_SC_PAGESIZE);


>Fix:
a workaround to avoid the pages being protected is to use MAP_SHARED but that will of course just introduce race conditions between parent/child that will result in crashes