All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v3] syscalls/fork: add new case fork14
@ 2014-02-24 16:23 Madper Xie
  2014-03-19 18:01 ` chrubis
       [not found] ` <533143E9.5050104@cn.fujitsu.com>
  0 siblings, 2 replies; 4+ messages in thread
From: Madper Xie @ 2014-02-24 16:23 UTC (permalink / raw)
  To: ltp-list


This testcase is a reproducer for https://lkml.org/lkml/2012/4/24/328.[1]
Modified from Siddhesh Poyarekar's testcase posted on above link.
Since vma length in dup_mmap is calculated and stored in a unsigned
int, which is insufficient and hence overflows for very large maps
(beyond 16TB). Once overflow occurred, the fork after mmaped memory >
16TB will succeed incorrectly.

    This case will run following loop:
             + mmap one (more) GB memory
             + fork and check return value.
    When mmaped more than 16 * 1024 GB, it will check if fork still fail.
    Expected result: Fork failed even if mmaped memory > 16 * 1024 GB

The issue has been fixed by:
  commit 7edc8b0ac16cbaed7cb4ea4c6b95ce98d2997e84
  Author: Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com>
    mm/fork: fix overflow in vma length when copying mmap on clone

Signed-off-by: Madper Xie <cxie@redhat.com>
---
 runtest/syscalls                        |   1 +
 testcases/kernel/syscalls/fork/fork14.c | 116 ++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fork/fork14.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 540c130..d895ecc 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -281,6 +281,7 @@ fork09 fork09
 fork10 fork10
 fork11 fork11
 fork13 fork13 -i 1000000
+fork14 fork14
 
 fpathconf01 fpathconf01
 
diff --git a/testcases/kernel/syscalls/fork/fork14.c b/testcases/kernel/syscalls/fork/fork14.c
new file mode 100644
index 0000000..93a2a3a
--- /dev/null
+++ b/testcases/kernel/syscalls/fork/fork14.c
@@ -0,0 +1,116 @@
+/*********************************************************************
+ * Copyright (C) 2014  Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ * Further, this software is distributed without any warranty that it
+ * is free of the rightful claim of any third person regarding
+ * infringement or the like.  Any license provided herein, whether
+ * implied or otherwise, applies only to this software file.  Patent
+ * licenses, if any, provided herein do not apply to combinations of
+ * this program with other software, or any other product whatsoever.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ *
+ * This test is a reporducer for this patch:
+ *              https://lkml.org/lkml/2012/4/24/328
+ * Since vma length in dup_mmap is calculated and stored in a unsigned
+ * int, it will overflow when length of mmaped memory > 16 TB. When
+ * overflow occur, fork will  incorrectly succeed. The patch above
+ * fixed it.
+ ********************************************************************/
+
+#include <sys/mman.h>
+#include <sys/wait.h>
+#include <stdio.h>
+#include <unistd.h>
+#include "test.h"
+#include "usctest.h"
+
+char *TCID = "fork14";
+int TST_TOTAL = 1;
+
+#define GB		(1024 * 1024 * 1024L)
+
+/* set mmap threshold to 16TB */
+#define LARGE		(16 * 1024)
+#define EXTENT		(16 * 1024 + 10)
+
+static void setup(void);
+static void cleanup(void);
+static int  fork_test(void);
+
+int main(int ac, char **av)
+{
+	int lc, ret;
+	char *msg;
+
+	msg = parse_opts(ac, av, NULL, NULL);
+	if (msg != NULL)
+		tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
+/*
+ * Tested on ppc64/x86_64/i386/s390x. And only 64bit has this issue.
+ * Since a 32bit program can't mmap so many memory.
+ */
+#if __WORDSIZE == 32
+	tst_brkm(TCONF, NULL, "This test is only for 64bit.");
+#endif
+	setup();
+	for (lc = 0; TEST_LOOPING(lc); lc++) {
+		tst_count = 0;
+
+		ret = fork_test();
+		if (ret == 0)
+			tst_resm(TPASS, "fork failed as expected.");
+	}
+	cleanup();
+	tst_exit();
+}
+
+static void setup(void)
+{
+	tst_sig(FORK, DEF_HANDLER, cleanup);
+	TEST_PAUSE;
+}
+
+static void cleanup(void)
+{
+	TEST_CLEANUP;
+}
+
+static int fork_test(void)
+{
+	int i, ret = 0;
+	void *addr;
+
+	for (i = 0; i < EXTENT; i++) {
+		addr = mmap(NULL, 1 * GB, PROT_READ | PROT_WRITE,
+			MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
+		switch (fork()) {
+		case -1:
+			break;
+		case 0:
+			exit(0);
+		default:
+			if (waitpid(-1, NULL, 0) == -1)
+				tst_brkm(TBROK|TERRNO,
+					cleanup, "waitpid");
+
+			if (i >= LARGE) {
+				tst_brkm(TFAIL, NULL,
+					"Fork succeeds incorrectly");
+				ret++;
+			}
+		}
+	}
+	return ret;
+}
-- 
1.9.0


------------------------------------------------------------------------------
Flow-based real-time traffic analytics software. Cisco certified tool.
Monitor traffic, SLAs, QoS, Medianet, WAAS etc. with NetFlow Analyzer
Customize your own dashboards, set traffic alerts and generate reports.
Network behavioral analysis & security monitoring. All-in-one tool.
http://pubads.g.doubleclick.net/gampad/clk?id=126839071&iu=/4140/ostg.clktrk
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH v3] syscalls/fork: add new case fork14
  2014-02-24 16:23 [LTP] [PATCH v3] syscalls/fork: add new case fork14 Madper Xie
@ 2014-03-19 18:01 ` chrubis
  2014-03-19 18:03   ` chrubis
       [not found] ` <533143E9.5050104@cn.fujitsu.com>
  1 sibling, 1 reply; 4+ messages in thread
From: chrubis @ 2014-03-19 18:01 UTC (permalink / raw)
  To: Madper Xie; +Cc: ltp-list

Hi!
> This testcase is a reproducer for https://lkml.org/lkml/2012/4/24/328.[1]
> Modified from Siddhesh Poyarekar's testcase posted on above link.
> Since vma length in dup_mmap is calculated and stored in a unsigned
> int, which is insufficient and hence overflows for very large maps
> (beyond 16TB). Once overflow occurred, the fork after mmaped memory >
> 16TB will succeed incorrectly.
> 
>     This case will run following loop:
>              + mmap one (more) GB memory
>              + fork and check return value.
>     When mmaped more than 16 * 1024 GB, it will check if fork still fail.
>     Expected result: Fork failed even if mmaped memory > 16 * 1024 GB
> 
> The issue has been fixed by:
>   commit 7edc8b0ac16cbaed7cb4ea4c6b95ce98d2997e84
>   Author: Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com>
>     mm/fork: fix overflow in vma length when copying mmap on clone

I've added an .gitignore record for the fork13 binary, removed the set
but not used addr variable and pushed the test.

I've also checked that the test indeed fails on older kernels.

Thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH v3] syscalls/fork: add new case fork14
  2014-03-19 18:01 ` chrubis
@ 2014-03-19 18:03   ` chrubis
  0 siblings, 0 replies; 4+ messages in thread
From: chrubis @ 2014-03-19 18:03 UTC (permalink / raw)
  To: Madper Xie; +Cc: ltp-list

Hi!
> I've added an .gitignore record for the fork13 binary, removed the set
                                          ^
					  aw, should be fork14
				  (sometimes I type faster than think...)
-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [LTP] [PATCH v3] syscalls/fork: add new case fork14
       [not found] ` <533143E9.5050104@cn.fujitsu.com>
@ 2014-03-25 10:04   ` chrubis
  0 siblings, 0 replies; 4+ messages in thread
From: chrubis @ 2014-03-25 10:04 UTC (permalink / raw)
  To: Xiaoguang Wang; +Cc: ltp-list

Hi!
> > +static void cleanup(void)
> > +{
> > +	TEST_CLEANUP;
> > +}
> > +
> > +static int fork_test(void)
> > +{
> > +	int i, ret = 0;
> > +	void *addr;
> > +
> > +	for (i = 0; i < EXTENT; i++) {
> > +		addr = mmap(NULL, 1 * GB, PROT_READ | PROT_WRITE,
> > +			MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> 
> I have some question here.
> In you v2 patch, you added some code to determine the fail of mmap():
> 
> > addr = mmap(NULL, (size_t) 1 * GB, PROT_READ | PROT_WRITE,
> > +			MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> > +		if (addr == MAP_FAILED)
> > +			tst_brkm(TBROK|TERRNO, cleanup, "mmap");
> 
> But in v3 patch, you deleted it. I think this maybe not right in some conditions.
>
> If in this for loop, mmap() fails for 10000 times(Indeed, I don't know whether it would occur),
> then we won't get a memory map, whose vm_area_struct is 16TB size, so the overflow won't occur.
> 
> So I think TCONF should be return in case mmap() failes. At least, we can ensure we have
> a vm_area_struct sized 16TB.

The problem is that with memory overcommit turned off the mmap will
start to fail as soon as all machine memory is used up. While ignoring
the return value is not best solution it is the easiest one. Returning
TCONF may be a bit better but that would complicate the test, see below.

> According to the Siddhesh Poyarekar's mail, he attached a more clearer test program.
> #include <unistd.h>
> #include <sys/mman.h>
> #include <errno.h>
> 
> #define GIG 1024 * 1024 * 1024L
> #define EXTENT 16393
> 
> int main(void)
> {
>         int i, r;
>         void *m;
>         char buf[1024];
>         int prev_failed = 0;
> 
>         for (i = 0; i < EXTENT; i++) {
>                 printf("i %d\n", i);
>                 m = mmap(NULL, (size_t) 1 * 1024 * 1024 * 1024L,
>                          PROT_READ | PROT_WRITE, MAP_PRIVATE |
>                          MAP_ANONYMOUS, 0, 0);
> 
>                 if (m == (void *)-1) {
>                         printf("MMAP Failed: %d\n", errno);
>                         return 1;
>                 }
> 
>                 r = fork();
> 
>                 if (r == 0) {
>                         return 0;
>                 } else if (r < 0) {
>                         prev_failed = 1;
>                         /* Fork failed as expected */
>                 } else if (r > 0 && prev_failed) {
>                         printf("Unexpected success at %d\n", i);
>                         wait(NULL);
>                         return 1;
>                 }
>         }
> 
>         return 0;
> }
> 
> I think this test program will be more appropriate. If the test failed before, then
> prev_failed is 1, and  if we continue to mmap,the vm_area_struct's size will
> continue to extend. If overflow does not occur, that is the bug is not exist, then
> fork will continue to fail. But if in some point, fork() success, then we can determine the
> overflow occurs now.

This program has two problems.

1. It will fail when memory overcommit is turned off
2. And it will fail if it's ported to LTP and the test was was
   started with -i 10 (because the mappings are not freed mmap will
   start to fail sooner or later)

If you want to make the test better you have to clear the mappings on
each test iteration (save the addr and unmap it later) and you may
change the code to exit with TCONF when mmap() failed before EXTENT was
reached.

-- 
Cyril Hrubis
chrubis@suse.cz

------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/13534_NeoTech
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-03-25 10:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-24 16:23 [LTP] [PATCH v3] syscalls/fork: add new case fork14 Madper Xie
2014-03-19 18:01 ` chrubis
2014-03-19 18:03   ` chrubis
     [not found] ` <533143E9.5050104@cn.fujitsu.com>
2014-03-25 10:04   ` chrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.