[LTP] [PATCH v2 0/5] .needs_abi_bits + rewrite fork14 to new API

[LTP] [PATCH v2 0/5] .needs_abi_bits + rewrite fork14 to new API

[Petr Vorel]

Hi,

Changes v1->v2:
* Change from .skip_in_32bit to .needs_abi_bits
* update fork14 to use .needs_abi_bits
* convert sbrk03.c to use .needs_abi_bits and .supported_archs
  (NOTE: it'd be nice if .supported_archs also printed supported archs
  in TCONF, so far it only states that the current arch is not
  supported)
* Convert setsockopt0[38].c to use tst_is_compat_mode()

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

Petr Vorel (4):
  tst_kernel.h: Convert docs to sphinx
  lib: Add .needs_abi_bits
  sbrk03: Convert to detect support with flags
  setsockopt0[38]: Use tst_is_compat_mode()

 doc/developers/api_c_tests.rst                |   2 +
 include/tst_kernel.h                          |  55 +++--
 include/tst_test.h                            |   7 +-
 lib/tst_kernel.c                              |  10 +
 lib/tst_test.c                                |   5 +-
 testcases/kernel/syscalls/fork/fork14.c       | 209 ++++++++----------
 testcases/kernel/syscalls/sbrk/sbrk03.c       |   9 +-
 .../kernel/syscalls/setsockopt/setsockopt03.c |   5 +-
 .../kernel/syscalls/setsockopt/setsockopt08.c |   6 +-
 9 files changed, 165 insertions(+), 143 deletions(-)

-- 
2.43.0


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

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

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
The same as in v1

 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 v2 2/5] lib: Add .needs_abi_bits

[Petr Vorel]

This allows to force kernel ABI.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v1->v2:
* Change from .skip_in_32bit to .needs_abi_bits

 include/tst_kernel.h | 12 ++++++++++++
 include/tst_test.h   |  7 ++++++-
 lib/tst_kernel.c     | 10 ++++++++++
 lib/tst_test.c       |  5 ++++-
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/include/tst_kernel.h b/include/tst_kernel.h
index e0ce7ce46..5f49952b7 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,15 @@ int tst_kernel_bits(void);
  */
 int tst_is_compat_mode(void);
 
+/**
+ * tst_abi_bits() - Detect if compiled for required kernel ABI.
+ *
+ * @abi: kernel ABI bits (32 or 64).
+ *
+ * Return: true if compiled for required ABI or false otherwise.
+ */
+bool tst_abi_bits(int abi);
+
 /**
  * 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..8dc20d110 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,10 @@ 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.
  *
+ * @needs_abi_bits: Skip the test if runs on a different kernel ABI than
+ *                  requested (on 32bit instead of 64bit or vice versa).
+ *                  Possible values: 32, 64.
+ *
  * @needs_hugetlbfs: If set hugetlbfs is mounted at tst_test.mntpoint.
  *
  * @skip_filesystems: A NULL terminated array of unsupported file systems. The
@@ -494,6 +498,7 @@ struct tst_ulimit_val {
 	unsigned int skip_in_lockdown:1;
 	unsigned int skip_in_secureboot:1;
 	unsigned int skip_in_compat:1;
+	int needs_abi_bits;
 
 	unsigned int needs_hugetlbfs:1;
 
diff --git a/lib/tst_kernel.c b/lib/tst_kernel.c
index 7fd1af871..8dabfeba2 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,14 @@ int tst_is_compat_mode(void)
 	return TST_ABI != tst_kernel_bits();
 }
 
+bool tst_abi_bits(int abi)
+{
+	if (abi != 32 && abi != 64)
+		tst_brkm(TBROK | TERRNO, NULL, "abi parameter can be only 32 or 64");
+
+	return abi == TST_ABI;
+}
+
 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..190e8da2a 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->needs_abi_bits && !tst_abi_bits(tst_test->needs_abi_bits))
+		tst_brk(TCONF, "%dbit ABI is not supported", tst_test->needs_abi_bits);
+
 	if (tst_test->needs_cmds) {
 		const char *cmd;
 		int i;
-- 
2.43.0


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

[LTP] [PATCH v2 3/5] 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 = 1 with .needs_abi_bits = 64 ]
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v1->v2:
* Change from .skip_in_32bit = 1 to .needs_abi_bits = 64

 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..d61864b52 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,
+	.needs_abi_bits = 64,
+	.tags = (const struct tst_tag[]) {
+		{"linux-git", "7edc8b0ac16c"},
+		{}
+	}
+};
-- 
2.43.0


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

[LTP] [PATCH v2 4/5] sbrk03: Convert to detect support with flags

[Petr Vorel]

.needs_abi_bits and .supported_archs are doc friendly.
Also, it does tst_brk() (in case of more run with -i).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v2

 testcases/kernel/syscalls/sbrk/sbrk03.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/sbrk/sbrk03.c b/testcases/kernel/syscalls/sbrk/sbrk03.c
index 80d2020ff..875815629 100644
--- a/testcases/kernel/syscalls/sbrk/sbrk03.c
+++ b/testcases/kernel/syscalls/sbrk/sbrk03.c
@@ -39,7 +39,6 @@
 
 static void sbrk_test(void)
 {
-#if defined(__s390__) && defined(TST_ABI32)
 	void *ret1, *ret2;
 
 	/* set bkr to 0x10000000 */
@@ -59,13 +58,15 @@ static void sbrk_test(void)
 	}
 
 	tst_res(TPASS, "sbrk verify: %p", ret2);
-#else
-	tst_res(TCONF, "Only works in 32bit on s390 series system");
-#endif
 }
 
 static struct tst_test test = {
 	.test_all = sbrk_test,
+	.supported_archs = (const char *const []) {
+		"s390",
+		NULL
+	},
+	.needs_abi_bits = 32,
 	.tags = (const struct tst_tag[]) {
 		{"linux-git", "473a06572fcd"},
 		{}
-- 
2.43.0


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

[LTP] [PATCH v2 5/5] setsockopt0[38]: Use tst_is_compat_mode()

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v2

I wonder if both shouldn't be TCONF or TINFO.

Kind regards,
Petr

 testcases/kernel/syscalls/setsockopt/setsockopt03.c | 5 ++---
 testcases/kernel/syscalls/setsockopt/setsockopt08.c | 6 ++----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt03.c b/testcases/kernel/syscalls/setsockopt/setsockopt03.c
index 7a1458277..d910280c8 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt03.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt03.c
@@ -48,9 +48,8 @@ struct payload {
 
 static void setup(void)
 {
-	if (tst_kernel_bits() == 32 || sizeof(long) > 4)
-		tst_res(TCONF,
-			"The vulnerability was only present in 32-bit compat mode");
+	if (!tst_is_compat_mode())
+		tst_res(TCONF, "The vulnerability was only present in 32-bit compat mode");
 }
 
 static void run(void)
diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
index 7f8243de1..3b7bd8482 100644
--- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
+++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
@@ -95,10 +95,8 @@ static void *buffer;
 
 void setup(void)
 {
-	if (tst_kernel_bits() == 32 || sizeof(long) > 4) {
-		tst_res(TINFO,
-			"The vulnerability was only present in 32-bit compat mode");
-	}
+	if (!tst_is_compat_mode())
+		tst_res(TINFO, "The vulnerability was only present in 32-bit compat mode");
 
 	tst_setup_netns();
 }
-- 
2.43.0


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

Re: [LTP] [PATCH v2 3/5] Refactor fork14 using new LTP API

[Andrea Cervesato via ltp]

Hi!

Looks good to me, thanks

Acked-by: Andrea Cervesato <andrea.cervesato@suse.com>

On 5/21/24 12:53, Petr Vorel wrote:
> 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 = 1 with .needs_abi_bits = 64 ]
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Changes v1->v2:
> * Change from .skip_in_32bit = 1 to .needs_abi_bits = 64
>
>   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..d61864b52 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,
> +	.needs_abi_bits = 64,
> +	.tags = (const struct tst_tag[]) {
> +		{"linux-git", "7edc8b0ac16c"},
> +		{}
> +	}
> +};

Andrea


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

Re: [LTP] [PATCH v2 1/5] tst_kernel.h: Convert docs to sphinx

[Andrea Cervesato via ltp]

Hi!

LGTM

Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>

On 5/21/24 12:53, Petr Vorel wrote:
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> The same as in v1
>
>   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);
>   

Andrea


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

Re: [LTP] [PATCH v2 2/5] lib: Add .needs_abi_bits

[Cyril Hrubis]

Hi!
The tst_test API looks good to me now.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 5/5] setsockopt0[38]: Use tst_is_compat_mode()

[Cyril Hrubis]

Hi!
>  testcases/kernel/syscalls/setsockopt/setsockopt03.c | 5 ++---
>  testcases/kernel/syscalls/setsockopt/setsockopt08.c | 6 ++----
>  2 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt03.c b/testcases/kernel/syscalls/setsockopt/setsockopt03.c
> index 7a1458277..d910280c8 100644
> --- a/testcases/kernel/syscalls/setsockopt/setsockopt03.c
> +++ b/testcases/kernel/syscalls/setsockopt/setsockopt03.c
> @@ -48,9 +48,8 @@ struct payload {
>  
>  static void setup(void)
>  {
> -	if (tst_kernel_bits() == 32 || sizeof(long) > 4)
> -		tst_res(TCONF,
> -			"The vulnerability was only present in 32-bit compat mode");
> +	if (!tst_is_compat_mode())
> +		tst_res(TCONF, "The vulnerability was only present in 32-bit compat mode");
>  }
>  
>  static void run(void)
> diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> index 7f8243de1..3b7bd8482 100644
> --- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> +++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> @@ -95,10 +95,8 @@ static void *buffer;
>  
>  void setup(void)
>  {
> -	if (tst_kernel_bits() == 32 || sizeof(long) > 4) {
> -		tst_res(TINFO,
> -			"The vulnerability was only present in 32-bit compat mode");
> -	}
> +	if (!tst_is_compat_mode())
> +		tst_res(TINFO, "The vulnerability was only present in 32-bit compat mode");
>  
>  	tst_setup_netns();
>  }

I guess this is something that should be looked into after the release,
either we will need .needs_compat flag or relax the condtions...

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 4/5] sbrk03: Convert to detect support with flags

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 5/5] setsockopt0[38]: Use tst_is_compat_mode()

[Petr Vorel]

Hi Cyril, all,

> I guess this is something that should be looked into after the release,
> either we will need .needs_compat flag or relax the condtions...

OK, the previous 4 got a review thus merged, this one is left out.

Thanks for your time!

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 5/5] setsockopt0[38]: Use tst_is_compat_mode()

[Petr Vorel]

Hi Cyril,

> Hi!
> >  testcases/kernel/syscalls/setsockopt/setsockopt03.c | 5 ++---
> >  testcases/kernel/syscalls/setsockopt/setsockopt08.c | 6 ++----
> >  2 files changed, 4 insertions(+), 7 deletions(-)

> > diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt03.c b/testcases/kernel/syscalls/setsockopt/setsockopt03.c
> > index 7a1458277..d910280c8 100644
> > --- a/testcases/kernel/syscalls/setsockopt/setsockopt03.c
> > +++ b/testcases/kernel/syscalls/setsockopt/setsockopt03.c
> > @@ -48,9 +48,8 @@ struct payload {

> >  static void setup(void)
> >  {
> > -	if (tst_kernel_bits() == 32 || sizeof(long) > 4)
> > -		tst_res(TCONF,
> > -			"The vulnerability was only present in 32-bit compat mode");
> > +	if (!tst_is_compat_mode())
> > +		tst_res(TCONF, "The vulnerability was only present in 32-bit compat mode");
> >  }

> >  static void run(void)
> > diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt08.c b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> > index 7f8243de1..3b7bd8482 100644
> > --- a/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> > +++ b/testcases/kernel/syscalls/setsockopt/setsockopt08.c
> > @@ -95,10 +95,8 @@ static void *buffer;

> >  void setup(void)
> >  {
> > -	if (tst_kernel_bits() == 32 || sizeof(long) > 4) {
> > -		tst_res(TINFO,
> > -			"The vulnerability was only present in 32-bit compat mode");
> > -	}
> > +	if (!tst_is_compat_mode())
> > +		tst_res(TINFO, "The vulnerability was only present in 32-bit compat mode");

> >  	tst_setup_netns();
> >  }

> I guess this is something that should be looked into after the release,
> either we will need .needs_compat flag or relax the condtions...

Looking into this old patch, do we relax the conditions or use .needs_abi_bits = 32?
setsockopt08.c prints only TINFO, thus !tst_is_compat_mode() would have to be
used for the check.

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 5/5] setsockopt0[38]: Use tst_is_compat_mode()

[Cyril Hrubis]

Hi!
> > I guess this is something that should be looked into after the release,
> > either we will need .needs_compat flag or relax the condtions...
> 
> Looking into this old patch, do we relax the conditions or use .needs_abi_bits = 32?
> setsockopt08.c prints only TINFO, thus !tst_is_compat_mode() would have to be
> used for the check.

I guess that doing:

	if (!tst_is_compat_mode())
		tst_res(TINFO, "The vunerability was only present in 32-bit compat mode");


Is a sensible approach. It does not hurt to run the test either way but
it makes it clear that the original CVE it's not going to be reproduced
without 32-bit compat mode.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 5/5] setsockopt0[38]: Use tst_is_compat_mode()

[Petr Vorel]

Hi Cyril,

> Hi!
> > > I guess this is something that should be looked into after the release,
> > > either we will need .needs_compat flag or relax the condtions...

> > Looking into this old patch, do we relax the conditions or use .needs_abi_bits = 32?
> > setsockopt08.c prints only TINFO, thus !tst_is_compat_mode() would have to be
> > used for the check.

> I guess that doing:

> 	if (!tst_is_compat_mode())
> 		tst_res(TINFO, "The vunerability was only present in 32-bit compat mode");


> Is a sensible approach. It does not hurt to run the test either way but
> it makes it clear that the original CVE it's not going to be reproduced
> without 32-bit compat mode.

+1. I modified that and dared to merge with your RBT (9dacb1ac27).
Thanks for your review!

Kind regards,
Petr


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