[LTP] [PATCH 0/3] .skip_in_32bit + rewrite fork14 to new API

[LTP] [PATCH 0/3] .skip_in_32bit + rewrite fork14 to new API

[Petr Vorel]

Hi,

added .skip_in_32bit + resubmit Andrea's fork14 rewrite.

Andrea Cervesato (1):
  Refactor fork14 using new LTP API

Petr Vorel (2):
  tst_kernel.h: Convert docs to sphinx
  lib: Add .skip_in_32bit

 doc/developers/api_c_tests.rst          |   2 +
 include/tst_kernel.h                    |  56 +++++--
 include/tst_test.h                      |   6 +-
 lib/tst_kernel.c                        |  11 ++
 lib/tst_test.c                          |   5 +-
 testcases/kernel/syscalls/fork/fork14.c | 209 +++++++++++-------------
 6 files changed, 157 insertions(+), 132 deletions(-)

-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/3] tst_kernel.h: Convert docs to sphinx

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/developers/api_c_tests.rst |  2 ++
 include/tst_kernel.h           | 43 +++++++++++++++++++++-------------
 2 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/doc/developers/api_c_tests.rst b/doc/developers/api_c_tests.rst
index ec53ab33c..2a9f3e7b9 100644
--- a/doc/developers/api_c_tests.rst
+++ b/doc/developers/api_c_tests.rst
@@ -1,4 +1,5 @@
 .. SPDX-License-Identifier: GPL-2.0-or-later
+.. Copyright (c) Linux Test Project, 2024
 
 .. Include headers in this file with:
 .. .. kernel-doc:: ../../include/tst_test.h
@@ -11,6 +12,7 @@ Core LTP API
 
 .. kernel-doc:: ../../include/tst_res_flags.h
 .. kernel-doc:: ../../include/tst_test.h
+.. kernel-doc:: ../../include/tst_kernel.h
 
 Option parsing
 --------------
diff --git a/include/tst_kernel.h b/include/tst_kernel.h
index 89de79928..e0ce7ce46 100644
--- a/include/tst_kernel.h
+++ b/include/tst_kernel.h
@@ -5,33 +5,44 @@
 #ifndef TST_KERNEL_H__
 #define TST_KERNEL_H__
 
-/*
- * Returns 32 if we are running on 32bit kernel and 64 if on 64bit kernel.
+/**
+ * tst_kernel_bits() - Detect if running on 32bit or 64bit kernel.
+ *
+ * Return: 32 if the test process is running on 32bit kernel and 64 if on 64bit
+ * kernel.
  */
 int tst_kernel_bits(void);
 
-/*
- * Returns non-zero if the test process is running in compat mode.
+/**
+ * tst_is_compat_mode() - Detect if running in compat mode.
+ *
+ * Detect if the test is 32bit binary executed on a 64bit kernel,
+ * i.e. we are testing the kernel compat layer.
+ *
+ * Return: non-zero if the test process is running in compat mode.
  */
 int tst_is_compat_mode(void);
 
-/*
- * Checks if the kernel module is built-in.
+/**
+ * tst_check_builtin_driver() - Check if the kernel module is built-in.
  *
- * @param driver The name of the driver.
- * @return Returns 0 if builtin driver
- * -1 when driver is missing or config file not available.
- * On Android *always* 0 (always expect the driver is available).
+ * @driver: the name of the driver.
+ *
+ * Return: 0 if builtin driver or -1 when driver is missing or config file not
+ * available. On Android *always* 0 (always expect the driver is available).
  */
 int tst_check_builtin_driver(const char *driver);
 
-/*
- * Checks support for the kernel module (both built-in and loadable).
+/**
+ * tst_check_driver() - Check support for the kernel module.
+ *
+ * Check support for the kernel module (both built-in and loadable).
+ *
+ * @driver: the name of the driver.
  *
- * @param driver The name of the driver.
- * @return Returns 0 if the kernel has the driver,
- * -1 when driver is missing or config file not available.
- * On Android *always* 0 (always expect the driver is available).
+ * Return: 0 if the kernel has the driver, -1 when driver is missing or config
+ * file not available. On Android *always* 0 (always expect the driver is
+ * available).
  */
 int tst_check_driver(const char *driver);
 
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/3] lib: Add .skip_in_32bit

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 include/tst_kernel.h | 13 +++++++++++++
 include/tst_test.h   |  6 +++++-
 lib/tst_kernel.c     | 11 +++++++++++
 lib/tst_test.c       |  5 ++++-
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/include/tst_kernel.h b/include/tst_kernel.h
index e0ce7ce46..e3fadbeb0 100644
--- a/include/tst_kernel.h
+++ b/include/tst_kernel.h
@@ -1,10 +1,13 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later
  * Copyright (c) 2017 Cyril Hrubis <chrubis@suse.cz>
+ * Copyright (c) Linux Test Project, 2018-2024
  */
 
 #ifndef TST_KERNEL_H__
 #define TST_KERNEL_H__
 
+#include <stdbool.h>
+
 /**
  * tst_kernel_bits() - Detect if running on 32bit or 64bit kernel.
  *
@@ -23,6 +26,16 @@ int tst_kernel_bits(void);
  */
 int tst_is_compat_mode(void);
 
+/**
+ * tst_is_32bit() - Detect if compiled for 32bit target.
+ *
+ * Detect if compiled for 32bit target (regardless if the test process is
+ * running on 32bit or 64bit kernel).
+ *
+ * Return: true if compiled for 32bit target or false otherwise.
+ */
+bool tst_is_32bit(void);
+
 /**
  * tst_check_builtin_driver() - Check if the kernel module is built-in.
  *
diff --git a/include/tst_test.h b/include/tst_test.h
index 69587917f..018249503 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 /*
  * Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>
- * Copyright (c) Linux Test Project, 2016-2019
+ * Copyright (c) Linux Test Project, 2016-2024
  */
 
 #ifndef TST_TEST_H__
@@ -328,6 +328,9 @@ struct tst_ulimit_val {
  * @skip_in_compat: Skip the test if we are executing 32bit binary on a 64bit
  *                  kernel, i.e. we are testing the kernel compat layer.
  *
+ * @skip_in_32bit: Skip the test if compiled for 32bit target (regardless if
+ *                 the test process is running on 32bit or 64bit kernel).
+ *
  * @needs_hugetlbfs: If set hugetlbfs is mounted at tst_test.mntpoint.
  *
  * @skip_filesystems: A NULL terminated array of unsupported file systems. The
@@ -494,6 +497,7 @@ struct tst_ulimit_val {
 	unsigned int skip_in_lockdown:1;
 	unsigned int skip_in_secureboot:1;
 	unsigned int skip_in_compat:1;
+	unsigned int skip_in_32bit:1;
 
 	unsigned int needs_hugetlbfs:1;
 
diff --git a/lib/tst_kernel.c b/lib/tst_kernel.c
index 7fd1af871..ada8d104b 100644
--- a/lib/tst_kernel.c
+++ b/lib/tst_kernel.c
@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2017 Cyril Hrubis <chrubis@suse.cz>
  * Copyright (c) 2020-2021 Petr Vorel <pvorel@suse.cz>
+ * Copyright (c) Linux Test Project, 2017-2024
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -18,6 +19,7 @@
 
 #include <sys/personality.h>
 #include <sys/utsname.h>
+#include <stdbool.h>
 #include <limits.h>
 
 #include "test.h"
@@ -96,6 +98,15 @@ int tst_is_compat_mode(void)
 	return TST_ABI != tst_kernel_bits();
 }
 
+bool tst_is_32bit(void)
+{
+#ifdef TST_ABI32
+	return true;
+#else
+	return false;
+#endif
+}
+
 static int tst_search_driver_(const char *driver, const char *file)
 {
 	struct stat st;
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 686ee428d..c35dc12b3 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2015-2016 Cyril Hrubis <chrubis@suse.cz>
- * Copyright (c) Linux Test Project, 2016-2021
+ * Copyright (c) Linux Test Project, 2016-2024
  */
 
 #include <limits.h>
@@ -1209,6 +1209,9 @@ static void do_setup(int argc, char *argv[])
 	if (tst_test->skip_in_compat && tst_is_compat_mode())
 		tst_brk(TCONF, "Not supported in 32-bit compat mode");
 
+	if (tst_test->skip_in_32bit && tst_is_32bit())
+		tst_brk(TCONF, "Not supported in 32-bit");
+
 	if (tst_test->needs_cmds) {
 		const char *cmd;
 		int i;
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v7 3/3] Refactor fork14 using new LTP API

[Petr Vorel]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Reviewed-by: Martin Doucha <mdoucha@suse.cz>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
[ pvorel: replace .skip_in_compat with .skip_in_32bit ]
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v6->v7:
* replace .skip_in_compat with .skip_in_32bit

 testcases/kernel/syscalls/fork/fork14.c | 209 +++++++++++-------------
 1 file changed, 95 insertions(+), 114 deletions(-)

diff --git a/testcases/kernel/syscalls/fork/fork14.c b/testcases/kernel/syscalls/fork/fork14.c
index 93af2ebac..bcfa8816d 100644
--- a/testcases/kernel/syscalls/fork/fork14.c
+++ b/testcases/kernel/syscalls/fork/fork14.c
@@ -1,143 +1,124 @@
-/*********************************************************************
+// SPDX-License-Identifier: GPL-2.0-only
+/*
  * Copyright (C) 2014  Red Hat, Inc.
+ * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
  *
- * 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 reproducer for kernel 3.5:
+ * 7edc8b0ac16c ("mm/fork: fix overflow in vma length when copying mmap on clone")
  *
- * 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
+ * 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.
- ********************************************************************/
+ * overflow occurs, fork will incorrectly succeed. The patch above fixed it.
+ */
 
-#include <sys/mman.h>
+#include "tst_test.h"
+#include <stdlib.h>
 #include <sys/wait.h>
-#include <stdio.h>
-#include <unistd.h>
-#include "test.h"
-#include "safe_macros.h"
-#include "lapi/abisize.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 char **pointer_vec;
+static char **memvec;
 
-static void setup(void);
-static void cleanup(void);
-static int  fork_test(void);
-
-int main(int ac, char **av)
+static void run(void)
 {
-	int lc, reproduced;
-
-	tst_parse_opts(ac, av, NULL, NULL);
-/*
- * Tested on ppc64/x86_64/i386/s390x. And only 64bit has this issue.
- * Since a 32bit program can't mmap so many memory.
- */
-#ifdef TST_ABI32
-	tst_brkm(TCONF, NULL, "This test is only for 64bit.");
-#endif
-	setup();
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-
-		reproduced = fork_test();
-		if (reproduced == 0)
-			tst_resm(TPASS, "fork failed as expected.");
-	}
-	cleanup();
-	tst_exit();
-}
+	int i, j, ret;
+	pid_t pid;
+	void *mem;
+	int prev_failed = 0;
+	int passed = 1;
+	int failures = 0;
 
-static void setup(void)
-{
-	tst_sig(FORK, DEF_HANDLER, cleanup);
-	TEST_PAUSE;
+	memset(memvec, 0, EXTENT);
 
-	pointer_vec = SAFE_MALLOC(cleanup, EXTENT * sizeof(char *));
-}
+	for (i = 0; i < EXTENT; i++) {
+		mem = mmap(NULL, 1 * TST_GB,
+			PROT_READ | PROT_WRITE,
+			MAP_PRIVATE | MAP_ANONYMOUS,
+			0, 0);
 
-static void cleanup(void)
-{
-	free(pointer_vec);
-}
+		if (mem == MAP_FAILED) {
+			failures++;
 
-static int fork_test(void)
-{
-	int i, j, prev_failed = 0, fails = 0, cnt = 0;
-	int reproduced = 0;
-	void *addr;
+			tst_res(TINFO, "mmap() failed");
 
-	for (i = 0; i < EXTENT; i++) {
-		addr = mmap(NULL, 1 * GB, PROT_READ | PROT_WRITE,
-			    MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
-		if (addr == MAP_FAILED) {
-			pointer_vec[i] = NULL;
-			fails++;
-			/*
-			 * EXTENT is "16*1024+10", if fails count exceeds 10,
-			 * we are almost impossible to get an vm_area_struct
-			 * sized 16TB
-			 */
-			if (fails == 11) {
-				tst_brkm(TCONF, cleanup, "mmap() fails too many"
-					 "times, so we are almost impossible to"
-					 " get an vm_area_struct sized 16TB.");
+			if (failures > 10) {
+				tst_brk(TCONF, "mmap() fails too many "
+					"times, so it's almost impossible to "
+					"get a vm_area_struct sized 16TB");
 			}
-		} else {
-			pointer_vec[i] = addr;
+
+			continue;
 		}
-		cnt++;
 
-		switch (tst_fork()) {
-		case -1:
+		memvec[i] = mem;
+
+		pid = fork();
+
+		if (pid == -1) {
+			/* keep track of the failed fork() and verify that next one
+			 * is failing as well.
+			 */
 			prev_failed = 1;
-		break;
-		case 0:
+			continue;
+		}
+
+		if (!pid)
 			exit(0);
-		default:
-			SAFE_WAITPID(cleanup, -1, NULL, 0);
 
-			if (prev_failed > 0 && i >= LARGE) {
-				tst_resm(TFAIL, "Fork succeeds incorrectly");
-				reproduced = 1;
-				goto clear_memory_map;
-			}
+		ret = waitpid(pid, NULL, 0);
+		if (ret == -1 && errno != ECHILD)
+			tst_brk(TBROK | TERRNO, "waitpid() error");
+
+		if (prev_failed && i >= LARGE) {
+			passed = 0;
+			break;
 		}
+
+		prev_failed = 0;
+
+		tst_res(TDEBUG, "fork() passed at %d attempt", i);
 	}
 
-clear_memory_map:
-	for (j = 0; j < cnt; j++) {
-		if (pointer_vec[j])
-			SAFE_MUNMAP(cleanup, pointer_vec[j], 1 * GB);
+	for (j = 0; j < i; j++) {
+		if (memvec[j])
+			SAFE_MUNMAP(memvec[j], 1 * TST_GB);
 	}
 
-	return reproduced;
+	if (passed)
+		tst_res(TPASS, "fork() failed as expected");
+	else
+		tst_res(TFAIL, "fork() succeeded incorrectly");
 }
+
+static void setup(void)
+{
+	memvec = SAFE_MALLOC(EXTENT * sizeof(char *));
+}
+
+static void cleanup(void)
+{
+	for (long i = 0; i < EXTENT; i++) {
+		if (memvec && memvec[i])
+			SAFE_MUNMAP(memvec[i], 1 * TST_GB);
+	}
+
+	if (memvec)
+		free(memvec);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.forks_child = 1,
+	.skip_in_32bit = 1,
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "7edc8b0ac16c"},
+		{}
+	}
+};
-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] lib: Add .skip_in_32bit

[Cyril Hrubis]

Hi!
> @@ -494,6 +497,7 @@ struct tst_ulimit_val {
>  	unsigned int skip_in_lockdown:1;
>  	unsigned int skip_in_secureboot:1;
>  	unsigned int skip_in_compat:1;
> +	unsigned int skip_in_32bit:1;

I was thinking about this and maybe it would make more sense to make
this more generic. If we ever need to add flags for a test to be skipped
on 64bit ABI we would have to add another flag. One example would be an
EOVERFLOW error from mmap() that can happen only on 32bit.

So maybe it would make sense to change this to be .needs_abi_bits and
the possible values would be 32 and 64.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] lib: Add .skip_in_32bit

[Petr Vorel]

> Hi!
> > @@ -494,6 +497,7 @@ struct tst_ulimit_val {
> >  	unsigned int skip_in_lockdown:1;
> >  	unsigned int skip_in_secureboot:1;
> >  	unsigned int skip_in_compat:1;
> > +	unsigned int skip_in_32bit:1;

> I was thinking about this and maybe it would make more sense to make
> this more generic. If we ever need to add flags for a test to be skipped
> on 64bit ABI we would have to add another flag. One example would be an
> EOVERFLOW error from mmap() that can happen only on 32bit.

> So maybe it would make sense to change this to be .needs_abi_bits and
> the possible values would be 32 and 64.

.needs_abi_bits sounds good, I'll send v2.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp