Re: [LTP] tst_clone: Fix child detection on sparc

Re: [LTP] tst_clone: Fix child detection on sparc

[linuxtestproject.agent]

Hi Anders,

On Mon Sep 28 23:27:13 2026 +0200, Anders Roxell <anders.roxell@linaro.org> wrote:
> tst_clone: Fix child detection on sparc

> +#ifdef __sparc__
> +	pid_t parent_pid = getpid();
> +#endif

Use tst_getpid() instead of getpid(). glibc versions prior to 2.25 cache
getpid(), which returns a stale PID if tst_clone() is called from a child
process created by a direct clone syscall.

> +out:
> +#ifdef __sparc__
> +	if (getpid() != parent_pid)
> +		pid = 0;
> +#endif

Calling getpid() in the child returns the cached parent PID on glibc < 2.25
because the raw clone syscall bypasses glibc, causing child detection to fail.
Use if (pid == parent_pid) pid = 0; instead, or use tst_getpid().

Verdict - Needs revision

---
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

Re: [LTP] tst_clone: Fix child detection on sparc

[linuxtestproject.agent]

Hi Anders,

On Tue Sep 29 10:42:53 2026 +0200, Anders Roxell wrote:
> tst_clone: Fix child detection on sparc

> @@ -20,6 +20,9 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
>  	};
>  	int flags;
>  	pid_t pid = -1;
> +#ifdef __sparc__
> +	pid_t parent_pid = tst_getpid();
> +#endif

On SPARC, clone3 already returns 0 in the child, making this call redundant
when clone3 succeeds; move it into the clone fallback path right before
syscall(__NR_clone, ...). Additionally, the SPARC kernel returns the
caller's TID (current->pid) in %o0 to the child, so use tst_gettid()
instead of tst_getpid() to support multithreaded callers.

Verdict - Needs revision

---
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

[LTP] [PATCH v3] tst_clone: Fix child detection on sparc

[Anders Roxell]

On sparc the clone syscall returns the caller's thread id in the child,
not 0. So the child thinks it is the parent.

Compare with tst_gettid() in the clone fallback. clone3 returns 0, but
sparc only has it since v7.0.

Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
v2 -> v3: use tst_gettid(), the kernel returns the caller's thread id
to the child. And only get it in the clone fallback, as the AI review
suggested.

v1 -> v2: fixed the AI review comments, use tst_getpid() since glibc
before 2.25 caches getpid(), and only check after clone, so the goto
is gone.

 lib/tst_clone.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/tst_clone.c b/lib/tst_clone.c
index 296cd40ac0a0..a04c4042653b 100644
--- a/lib/tst_clone.c
+++ b/lib/tst_clone.c
@@ -35,6 +35,10 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
 
 	flags = args.exit_signal | args.flags;
 
+#ifdef __sparc__
+	pid_t tid = tst_gettid();
+#endif
+
 #ifdef __s390x__
 	pid = syscall(__NR_clone, NULL, flags, args.pidfd, NULL, NULL);
 #else
@@ -44,5 +48,10 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
 	if (pid == -1)
 		return -2;
 
+#ifdef __sparc__
+	if (pid == tid)
+		pid = 0;
+#endif
+
 	return pid;
 }
-- 
2.53.0


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

Re: [LTP] [PATCH v3] tst_clone: Fix child detection on sparc

[Petr Vorel]

Hi Anders,

> On sparc the clone syscall returns the caller's thread id in the child,
> not 0. So the child thinks it is the parent.

> Compare with tst_gettid() in the clone fallback. clone3 returns 0, but
> sparc only has it since v7.0.

> Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
> ---
> v2 -> v3: use tst_gettid(), the kernel returns the caller's thread id
> to the child. And only get it in the clone fallback, as the AI review
> suggested.

> v1 -> v2: fixed the AI review comments, use tst_getpid() since glibc
> before 2.25 caches getpid(), and only check after clone, so the goto
> is gone.

>  lib/tst_clone.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

> diff --git a/lib/tst_clone.c b/lib/tst_clone.c
> index 296cd40ac0a0..a04c4042653b 100644
> --- a/lib/tst_clone.c
> +++ b/lib/tst_clone.c
> @@ -35,6 +35,10 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)

>  	flags = args.exit_signal | args.flags;

> +#ifdef __sparc__
> +	pid_t tid = tst_gettid();

Unfortunately, we haven't realized that this will not work in the PID namespace
(CLONE_NEWPID). Agent's finding (run locally):

	- In the child, the old sparc clone puts current->pid into %o0
	  (arch/sparc/kernel/process_64.c:634, process_32.c:365). That is the
	  parent's global thread id.
	- tst_gettid() returns the thread id as seen inside the namespace
	  (kernel/sys.c:1007).
	- Inside a namespace the two numbers differ, so the child takes the parent's
	  code path.

And concrete way how this can be manifested is e.g.
testcases/kernel/containers/pidns/pidns05.c:

	pidns05: child_func() runs as pid 1 in a new namespace and calls
	SAFE_CLONE(). The grandchild sees a number other than 1, behaves as the
	parent and sits in pause() until the checkpoint times out.

Sounds reasonable, could you please verify it?

Agent suggests to check the second return value in %o1 instead (1 in the child,
0 in the parent), as glibc's sparc fork/clone wrappers do).

Looking into glibc sources there is indeed clone() sparc specific assembly:
sysdeps/unix/sysv/linux/sparc/sparc32/clone.S
sysdeps/unix/sysv/linux/sparc/sparc64/clone.S

Agent even suggested assembly which has similar fragments, but of course we'd
strive to avoid using it unless really necessary.

#ifdef __sparc__
/*
 * On sparc, clone() returns the parent's global thread id in %o0 in the
 * child, which cannot be compared with gettid() inside a pid namespace.
 * The kernel sets %o1 to 1 in the child and 0 in the parent, but syscall()
 * drops %o1, hence inline assembly.
 */
static pid_t sparc_clone(unsigned long flags, unsigned long pidfd)
{
	register long g1 __asm__("g1") = __NR_clone;
	register long o0 __asm__("o0") = flags;
	register long o1 __asm__("o1") = 0;
	register long o2 __asm__("o2") = pidfd;
	register long o3 __asm__("o3") = 0;
	register long o4 __asm__("o4") = 0;

	__asm__ __volatile__(
# ifdef __arch64__
		"ta 0x6d\n\t"
		"bcc,pt %%xcc, 1f\n\t"
# else
		"ta 0x10\n\t"
		"bcc 1f\n\t"
# endif
		" mov 0, %%g1\n\t"
		"mov 1, %%g1\n"
		"1:"
		: "+r" (g1), "+r" (o0), "+r" (o1), "+r" (o2), "+r" (o3), "+r" (o4)
		:
		: "cc", "memory");

	if (g1) {
		errno = o0;
		return -1;
	}

	return o1 ? 0 : o0;
}
#endif

Regard that use of tst_gettid() instead of tst_getpid() suggested for v2 by
agent [1]:

	the SPARC kernel returns the caller's TID (current->pid) in %o0 to the
	child, so use tst_gettid() instead of tst_getpid() to support multithreaded
	callers.

Sure, LTP has tests which use threads, but many of them use
pthread_create() instead of SAFE_CLONE()/tst_clone() (which you're trying to
fix).

IMHO there is only testcases/kernel/syscalls/fanotify/fanotify21.c, but even
that test is safe, because SAFE_CLONE() is call for a different test case than
pthread usage.

But of course it does not harm to have it.

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/20260929111636.7777-1-linuxtestproject.agent@gmail.com/

> +#endif
> +
>  #ifdef __s390x__
>  	pid = syscall(__NR_clone, NULL, flags, args.pidfd, NULL, NULL);
>  #else
> @@ -44,5 +48,10 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
>  	if (pid == -1)
>  		return -2;

> +#ifdef __sparc__
> +	if (pid == tid)
> +		pid = 0;
> +#endif
> +
>  	return pid;
>  }

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

Re: [LTP] tst_clone: Fix child detection on sparc

[linuxtestproject.agent]

Hi Anders,

On Wed, 30 Sep 2026, Anders Roxell wrote:
> tst_clone: Fix child detection on sparc

> +#ifdef __sparc__
> +	if (pid == tid)
> +		pid = 0;
> +#endif

High severity: SPARC copy_thread() returns the caller's global TID, but
tst_gettid() returns its namespace-local TID. In pidns05's nested
CLONE_NEWPID path, tid is 1 while pid is the caller's global TID, so the
child still takes the parent branch and the test waits until timeout.
Use a SPARC syscall wrapper that reads the %o1 child indicator instead
of comparing identifiers from different PID namespaces.

Verdict - Needs revision

---
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