Re: [LTP] lapi: fallback fchroot() parameters

[linuxtestproject.agent@gmail.com]

Hi Andrea,

On 2026-10-20, Andrea Cervesato wrote:
> lapi: fallback fchroot() parameters

--- [PATCH 5/14] ---

> diff --git a/testcases/kernel/syscalls/fchroot/fchroot04.c b/testcases/kernel/syscalls/fchroot/fchroot04.c
> +	TST_EXP_PASS(tst_syscall(__NR_fchroot, FD_FAILFS_ROOT, 0),
> +		"fchroot() with the FD_FAILFS_ROOT sentinel");

High: Stop the child if this expectation fails; TST_EXP_PASS records a
failure but continues execution. For example, root without CAP_SYS_CHROOT
and without no_new_privs gets EPERM, then mkdir("/foo") can create a
directory in the real root that tmpdir cleanup never removes. Check
TST_PASS and exit before executing the absolute-path operations.

--- [PATCH 6/14] ---

> diff --git a/testcases/kernel/syscalls/fchroot/fchroot05.c b/testcases/kernel/syscalls/fchroot/fchroot05.c
> +	len = readlinkat(procfd, "self/root", buf, sizeof(buf) - 1);
> +	if (len < 0) {
> +		tst_res(TFAIL | TTERRNO, "readlinkat() of /proc/self/root");

Low: TTERRNO still reads the preceding fstatat() error from TST_ERR,
not readlinkat()'s errno. Wrap readlinkat() in TEST() or
TST_EXP_POSITIVE() so a failure reports the actual error.

--- [PATCH 8/14] ---

> diff --git a/testcases/kernel/syscalls/fchroot/fchroot07.c b/testcases/kernel/syscalls/fchroot/fchroot07.c
> +	SAFE_WAIT(NULL);

High: A successfully executed helper can fail in tst_reinit() while
reopening its absolute IPC pathname under failfs, before it can publish
TFAIL through shared memory. SAFE_WAIT(NULL) discards that nonzero exit,
leaving only the earlier fchroot TPASS and a false pass. Use
tst_reap_children() to retain child-failure detection.

> +	if (tst_get_path(CHILD_EXEC, child_path, sizeof(child_path)))
> +		tst_brk(TCONF, "Couldn't find %s in $PATH", CHILD_EXEC);

Medium: Make child_path absolute before entering failfs. Despite its API
comment, tst_get_path() returns ./fchroot07_child when the helper is found
through an empty or relative PATH component, including the empty component
inserted by the framework when running from the build directory. This
tests relative execution and, for a dynamic binary, its interpreter lookup
rather than the claimed absolute executable lookup.

--- [PATCH 12/14] ---

> diff --git a/testcases/kernel/syscalls/fchroot/fchroot11.c b/testcases/kernel/syscalls/fchroot/fchroot11.c
> +	.needs_kconfigs = (const char *[]) {
> +		"CONFIG_USER_NS=y",
> +		NULL,
> +	}

Medium: Gate only the user-namespace subcase, not the entire test.
Unprivileged failfs entry with no_new_privs works without CONFIG_USER_NS,
but this descriptor skips its success and lookup checks as well. Keep
those checks enabled and report TCONF only for the unavailable namespace
subcase.

Verdict - Needs revision

Pre-existing issue: include/tst_fs.h documents tst_get_path() as returning
an absolute path, but lib/get_path.c preserves relative PATH entries.

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

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