NetBSD Problem Report #55177

From www@netbsd.org  Wed Apr 15 05:58:19 2020
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id DE4EE1A9219
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 15 Apr 2020 05:58:19 +0000 (UTC)
Message-Id: <20200415055818.8C52F1A924B@mollari.NetBSD.org>
Date: Wed, 15 Apr 2020 05:58:18 +0000 (UTC)
From: carenas@gmail.com
Reply-To: carenas@gmail.com
To: gnats-bugs@NetBSD.org
Subject: mremap(MAP_REMAPDUP) fails after fork()
X-Send-Pr-Version: www-1.0

>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
>Last-Modified:  Sat Apr 18 17:45:02 +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:/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

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/55177 CVS commit: src/tests/lib/libc/sys
Date: Sat, 18 Apr 2020 13:44:53 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Sat Apr 18 17:44:53 UTC 2020

 Modified Files:
 	src/tests/lib/libc/sys: Makefile t_mprotect.c
 Added Files:
 	src/tests/lib/libc/sys: t_mprotect_helper.c

 Log Message:
 PR/55177: Carlo Arenas: mremap(MAP_REMAPDUP) fails after fork()


 To generate a diff of this commit:
 cvs rdiff -u -r1.61 -r1.62 src/tests/lib/libc/sys/Makefile
 cvs rdiff -u -r1.8 -r1.9 src/tests/lib/libc/sys/t_mprotect.c
 cvs rdiff -u -r0 -r1.1 src/tests/lib/libc/sys/t_mprotect_helper.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.