[PATCH v3 0/2] selftests/fchmodat2: Error handling and general cleanups

[PATCH v3 0/2] selftests/fchmodat2: Error handling and general cleanups

[Mark Brown]

I looked at the fchmodat2() tests since I've been experiencing some
random intermittent segfaults with them in my test systems, while doing
so I noticed these two issues.  Unfortunately I didn't figure out the
original yet, unless I managed to fix it unwittingly.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
Changes in v3:
- Rebase onto v6.18-rc1.
- Link to v2: https://lore.kernel.org/r/20250812-selftests-fchmodat2-v2-0-f2d5380e94c3@kernel.org

Changes in v2:
- Rebase onto v6.17-rc1.
- Link to v1: https://lore.kernel.org/r/20250714-selftests-fchmodat2-v1-0-b74f3ee0d09c@kernel.org

---
Mark Brown (2):
      selftests/fchmodat2: Clean up temporary files and directories
      selftests/fchmodat2: Use ksft_finished()

 tools/testing/selftests/fchmodat2/fchmodat2_test.c | 166 ++++++++++++++-------
 1 file changed, 112 insertions(+), 54 deletions(-)
---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20250711-selftests-fchmodat2-c30374c376f8

Best regards,
--  
Mark Brown <broonie@kernel.org>

[PATCH v3 1/2] selftests/fchmodat2: Clean up temporary files and directories

[Mark Brown]

The fchmodat2() test program creates a temporary directory with a file and
a symlink for every test it runs but never cleans these up, resulting in
${TMPDIR} getting left with stale files after every run. Restructure the
program a bit to ensure that we clean these up, this is more invasive than
it might otherwise be due to the extensive use of ksft_exit_fail_msg() in
the program.

As a side effect this also ensures that we report a consistent test name
for the tests and always try both tests even if they are skipped.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/fchmodat2/fchmodat2_test.c | 161 ++++++++++++++-------
 1 file changed, 111 insertions(+), 50 deletions(-)

diff --git a/tools/testing/selftests/fchmodat2/fchmodat2_test.c b/tools/testing/selftests/fchmodat2/fchmodat2_test.c
index e0319417124d..e977d942c00b 100644
--- a/tools/testing/selftests/fchmodat2/fchmodat2_test.c
+++ b/tools/testing/selftests/fchmodat2/fchmodat2_test.c
@@ -9,6 +9,11 @@
 
 #include "../kselftest.h"
 
+struct testdir {
+	char *dirname;
+	int dfd;
+};
+
 int sys_fchmodat2(int dfd, const char *filename, mode_t mode, int flags)
 {
 	int ret = syscall(__NR_fchmodat2, dfd, filename, mode, flags);
@@ -16,9 +21,9 @@ int sys_fchmodat2(int dfd, const char *filename, mode_t mode, int flags)
 	return ret >= 0 ? ret : -errno;
 }
 
-int setup_testdir(void)
+static void setup_testdir(struct testdir *testdir)
 {
-	int dfd, ret;
+	int ret, dfd;
 	char dirname[] = "/tmp/ksft-fchmodat2.XXXXXX";
 
 	/* Make the top-level directory. */
@@ -26,21 +31,48 @@ int setup_testdir(void)
 		ksft_exit_fail_msg("%s: failed to create tmpdir\n", __func__);
 
 	dfd = open(dirname, O_PATH | O_DIRECTORY);
-	if (dfd < 0)
-		ksft_exit_fail_msg("%s: failed to open tmpdir\n", __func__);
+	if (dfd < 0) {
+		ksft_perror("failed to open tmpdir");
+		goto err;
+	}
 
 	ret = openat(dfd, "regfile", O_CREAT | O_WRONLY | O_TRUNC, 0644);
-	if (ret < 0)
-		ksft_exit_fail_msg("%s: failed to create file in tmpdir\n",
-				__func__);
+	if (ret < 0) {
+		ksft_perror("failed to create file in tmpdir");
+		goto err;
+	}
 	close(ret);
 
 	ret = symlinkat("regfile", dfd, "symlink");
-	if (ret < 0)
-		ksft_exit_fail_msg("%s: failed to create symlink in tmpdir\n",
-				__func__);
+	if (ret < 0) {
+		ksft_perror("symlinkat() failed");
+		goto err_regfile;
+	}
+
+	testdir->dirname = strdup(dirname);
+	if (!testdir->dirname) {
+		ksft_perror("Out of memory");
+		goto err_symlink;
+	}
+	testdir->dfd = dfd;
+
+	return;
+
+err_symlink:
+	unlinkat(testdir->dfd, "symlink", 0);
+err_regfile:
+	unlinkat(testdir->dfd, "regfile", 0);
+err:
+	unlink(dirname);
+	ksft_exit_fail();
+}
 
-	return dfd;
+static void cleanup_testdir(struct testdir *testdir)
+{
+	unlinkat(testdir->dfd, "regfile", 0);
+	unlinkat(testdir->dfd, "symlink", 0);
+	rmdir(testdir->dirname);
+	free(testdir->dirname);
 }
 
 int expect_mode(int dfd, const char *filename, mode_t expect_mode)
@@ -48,61 +80,80 @@ int expect_mode(int dfd, const char *filename, mode_t expect_mode)
 	struct stat st;
 	int ret = fstatat(dfd, filename, &st, AT_SYMLINK_NOFOLLOW);
 
-	if (ret)
-		ksft_exit_fail_msg("%s: %s: fstatat failed\n",
-				__func__, filename);
+	if (ret) {
+		ksft_perror("fstatat() failed\n");
+		return 0;
+	}
 
 	return (st.st_mode == expect_mode);
 }
 
 void test_regfile(void)
 {
-	int dfd, ret;
-
-	dfd = setup_testdir();
+	struct testdir testdir;
+	int ret;
 
-	ret = sys_fchmodat2(dfd, "regfile", 0640, 0);
+	setup_testdir(&testdir);
 
-	if (ret < 0)
-		ksft_exit_fail_msg("%s: fchmodat2(noflag) failed\n", __func__);
-
-	if (!expect_mode(dfd, "regfile", 0100640))
-		ksft_exit_fail_msg("%s: wrong file mode bits after fchmodat2\n",
-				__func__);
+	ret = sys_fchmodat2(testdir.dfd, "regfile", 0640, 0);
 
-	ret = sys_fchmodat2(dfd, "regfile", 0600, AT_SYMLINK_NOFOLLOW);
+	if (ret < 0) {
+		ksft_perror("fchmodat2(noflag) failed");
+		goto out;
+	}
 
-	if (ret < 0)
-		ksft_exit_fail_msg("%s: fchmodat2(AT_SYMLINK_NOFOLLOW) failed\n",
+	if (!expect_mode(testdir.dfd, "regfile", 0100640)) {
+		ksft_print_msg("%s: wrong file mode bits after fchmodat2\n",
 				__func__);
-
-	if (!expect_mode(dfd, "regfile", 0100600))
-		ksft_exit_fail_msg("%s: wrong file mode bits after fchmodat2 with nofollow\n",
-				__func__);
-
-	ksft_test_result_pass("fchmodat2(regfile)\n");
+		ret = 1;
+		goto out;
+	}
+
+	ret = sys_fchmodat2(testdir.dfd, "regfile", 0600, AT_SYMLINK_NOFOLLOW);
+
+	if (ret < 0) {
+		ksft_perror("fchmodat2(AT_SYMLINK_NOFOLLOW) failed");
+		goto out;
+	}
+
+	if (!expect_mode(testdir.dfd, "regfile", 0100600)) {
+		ksft_print_msg("%s: wrong file mode bits after fchmodat2 with nofollow\n",
+			       __func__);
+		ret = 1;
+	}
+
+out:
+	ksft_test_result(ret == 0, "fchmodat2(regfile)\n");
+	cleanup_testdir(&testdir);
 }
 
 void test_symlink(void)
 {
-	int dfd, ret;
+	struct testdir testdir;
+	int ret;
 
-	dfd = setup_testdir();
+	setup_testdir(&testdir);
 
-	ret = sys_fchmodat2(dfd, "symlink", 0640, 0);
+	ret = sys_fchmodat2(testdir.dfd, "symlink", 0640, 0);
 
-	if (ret < 0)
-		ksft_exit_fail_msg("%s: fchmodat2(noflag) failed\n", __func__);
+	if (ret < 0) {
+		ksft_perror("fchmodat2(noflag) failed");
+		goto err;
+	}
 
-	if (!expect_mode(dfd, "regfile", 0100640))
-		ksft_exit_fail_msg("%s: wrong file mode bits after fchmodat2\n",
-				__func__);
+	if (!expect_mode(testdir.dfd, "regfile", 0100640)) {
+		ksft_print_msg("%s: wrong file mode bits after fchmodat2\n",
+			       __func__);
+		goto err;
+	}
 
-	if (!expect_mode(dfd, "symlink", 0120777))
-		ksft_exit_fail_msg("%s: wrong symlink mode bits after fchmodat2\n",
-				__func__);
+	if (!expect_mode(testdir.dfd, "symlink", 0120777)) {
+		ksft_print_msg("%s: wrong symlink mode bits after fchmodat2\n",
+			       __func__);
+		goto err;
+	}
 
-	ret = sys_fchmodat2(dfd, "symlink", 0600, AT_SYMLINK_NOFOLLOW);
+	ret = sys_fchmodat2(testdir.dfd, "symlink", 0600, AT_SYMLINK_NOFOLLOW);
 
 	/*
 	 * On certain filesystems (xfs or btrfs), chmod operation fails. So we
@@ -111,18 +162,28 @@ void test_symlink(void)
 	 *
 	 * https://sourceware.org/legacy-ml/libc-alpha/2020-02/msg00467.html
 	 */
-	if (ret == 0 && !expect_mode(dfd, "symlink", 0120600))
-		ksft_exit_fail_msg("%s: wrong symlink mode bits after fchmodat2 with nofollow\n",
+	if (ret == 0 && !expect_mode(testdir.dfd, "symlink", 0120600)) {
+		ksft_print_msg("%s: wrong symlink mode bits after fchmodat2 with nofollow\n",
 				__func__);
+		ret = 1;
+		goto err;
+	}
 
-	if (!expect_mode(dfd, "regfile", 0100640))
-		ksft_exit_fail_msg("%s: wrong file mode bits after fchmodat2 with nofollow\n",
-				__func__);
+	if (!expect_mode(testdir.dfd, "regfile", 0100640)) {
+		ksft_print_msg("%s: wrong file mode bits after fchmodat2 with nofollow\n",
+			       __func__);
+	}
 
 	if (ret != 0)
 		ksft_test_result_skip("fchmodat2(symlink)\n");
 	else
 		ksft_test_result_pass("fchmodat2(symlink)\n");
+	cleanup_testdir(&testdir);
+	return;
+
+err:
+	ksft_test_result_fail("fchmodat2(symlink)\n");
+	cleanup_testdir(&testdir);
 }
 
 #define NUM_TESTS 2

-- 
2.47.2

[PATCH v3 2/2] selftests/fchmodat2: Use ksft_finished()

[Mark Brown]

The fchmodat2 test program open codes a version of ksft_finished(), use the
standard version.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/fchmodat2/fchmodat2_test.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tools/testing/selftests/fchmodat2/fchmodat2_test.c b/tools/testing/selftests/fchmodat2/fchmodat2_test.c
index e977d942c00b..d3682ccd7cb8 100644
--- a/tools/testing/selftests/fchmodat2/fchmodat2_test.c
+++ b/tools/testing/selftests/fchmodat2/fchmodat2_test.c
@@ -196,8 +196,5 @@ int main(int argc, char **argv)
 	test_regfile();
 	test_symlink();
 
-	if (ksft_get_fail_cnt() + ksft_get_error_cnt() > 0)
-		ksft_exit_fail();
-	else
-		ksft_exit_pass();
+	ksft_finished();
 }

-- 
2.47.2

Re: [PATCH v3 0/2] selftests/fchmodat2: Error handling and general cleanups

[Alexey Gladkov]

On Wed, Oct 15, 2025 at 01:36:24PM +0100, Mark Brown wrote:
> I looked at the fchmodat2() tests since I've been experiencing some
> random intermittent segfaults with them in my test systems, while doing
> so I noticed these two issues.  Unfortunately I didn't figure out the
> original yet, unless I managed to fix it unwittingly.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> Changes in v3:
> - Rebase onto v6.18-rc1.
> - Link to v2: https://lore.kernel.org/r/20250812-selftests-fchmodat2-v2-0-f2d5380e94c3@kernel.org
> 
> Changes in v2:
> - Rebase onto v6.17-rc1.
> - Link to v1: https://lore.kernel.org/r/20250714-selftests-fchmodat2-v1-0-b74f3ee0d09c@kernel.org
> 
> ---
> Mark Brown (2):
>       selftests/fchmodat2: Clean up temporary files and directories
>       selftests/fchmodat2: Use ksft_finished()
> 
>  tools/testing/selftests/fchmodat2/fchmodat2_test.c | 166 ++++++++++++++-------
>  1 file changed, 112 insertions(+), 54 deletions(-)
> ---
> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> change-id: 20250711-selftests-fchmodat2-c30374c376f8
> 
> Best regards,
> --  
> Mark Brown <broonie@kernel.org>
> 

It make sense. Thanks!

Acked-by: Alexey Gladkov <legion@kernel.org>

-- 
Rgrds, legion