[LTP] Make ioctl_pidfd02 valgrind-compliant

[LTP] Make ioctl_pidfd02 valgrind-compliant

[Martin Cermak via ltp]

[-- Attachment #1: Type: text/plain, Size: 163 bytes --]

Hi folks,

attached is a ioctl_pidfd02 patch that improves the behavior of
this testcase under valgrind.  Please review and check in if
possible.

Thanks,
Martin


[-- Attachment #2: 0001-Make-ioctl_pidfd02-valgrind-compliant.patch --]
[-- Type: text/plain, Size: 3917 bytes --]

From 25c0e052aba25b0a86f844246f14aa17ad47ef1f Mon Sep 17 00:00:00 2001
From: Martin Cermak <mcermak@redhat.com>
Date: Wed, 15 Apr 2026 10:03:44 +0200
Subject: [PATCH] Make ioctl_pidfd02 valgrind-compliant

When ioctl_pidfd02 runs under valgrind, it fails to use clone3()
in lib/tst_clone.c:27, because valgrind doesn't implement it. So,
it resorts to a raw syscall a few lines later.

Manual page clone(2) says that In Linux 2.4 and earlier, clone()
does not take arguments parent_tid, tls, and child_tid.  That pre-
dates valgrind itself though.  In reasonably recent history, the
raw clone() syscall takes 5 args.

The ioctl_pidfd02.c testcase calls clone() with flags = CLONE_PIDFD
and others.  When that happens, the 4th clone() arg, parent_tidptr,
becomes mandatory, and valgrind checks its validity per PRE(sys_clone)
along the lines of coregrind/m_syswrap/syswrap-linux.c:831.  If this
arg isn't set, the raw syscall() will execute without failure, but
that arg contains garbage, and valgrind fails to check it:

ioctl_pidfd02.c:64: TPASS: WEXITSTATUS(info1->exit_code) == 100 (100)
==2227408== Syscall param clone(parent_tidptr) points to unaddressable byte(s)
==2227408==    at 0x496215D: syscall (syscall.S:38)
==2227408==    by 0x420E73: tst_clone (tst_clone.c:41)
==2227408==    by 0x4135E9: safe_clone (tst_test.c:601)
==2227408==    by 0x405094: run (ioctl_pidfd02.c:33)
==2227408==    by 0x416AE4: run_tests (tst_test.c:1730)
==2227408==    by 0x416D6E: testrun (tst_test.c:1796)
==2227408==    by 0x417427: fork_testrun (tst_test.c:1936)
==2227408==    by 0x417A8A: tst_run_tcases (tst_test.c:2079)
==2227408==    by 0x404E9A: main (tst_test.h:755)
==2227408==  Address 0xffffffffffffffa0 is not stack'd, malloc'd or (recently) free'd

This patch sets the parent_tidptr explicitly, making valgrind happy.
With this update, ioctl_pidfd02 tests fine on all the rhel supported
arches both with and without valgrind, e.g.:

el10 s390x # valgrind --exit-on-first-error=yes --error-exitcode=-3 --quiet ./testcases/kernel/syscalls/ioctl/ioctl_pidfd02
tst_kconfig.c:90: TINFO: Parsing kernel config '/lib/modules/6.12.0-218.el10.s390x/build/.config'
tst_buffers.c:57: TINFO: Test is using guarded buffers
tst_tmpdir.c:308: TINFO: Using /tmp/LTP_iocGIJIgS as tmpdir (xfs filesystem)
tst_test.c:2059: TINFO: LTP version: 20260130
tst_test.c:2062: TINFO: Tested kernel: 6.12.0-218.el10.s390x #1 SMP Tue Mar 31 07:51:26 EDT 2026 s390x
tst_kconfig.c:90: TINFO: Parsing kernel config '/lib/modules/6.12.0-218.el10.s390x/build/.config'
tst_test.c:1887: TINFO: Overall timeout per run is 0h 00m 30s
ioctl_pidfd02.c:49: TPASS: info0->mask & PIDFD_INFO_EXIT == 0 (0)
ioctl_pidfd02.c:50: TPASS: info0->exit_code == 0 (0)
ioctl_pidfd02.c:61: TPASS: info1->mask & PIDFD_INFO_EXIT == PIDFD_INFO_EXIT (8)
ioctl_pidfd02.c:62: TPASS: info1->exit_code == status (25600)
ioctl_pidfd02.c:64: TPASS: WEXITSTATUS(info1->exit_code) == 100 (100)
ioctl_pidfd02.c:49: TPASS: info0->mask & PIDFD_INFO_EXIT == 0 (0)
ioctl_pidfd02.c:50: TPASS: info0->exit_code == 0 (0)
ioctl_pidfd02.c:61: TPASS: info1->mask & PIDFD_INFO_EXIT == PIDFD_INFO_EXIT (8)
ioctl_pidfd02.c:62: TPASS: info1->exit_code == status (25600)
ioctl_pidfd02.c:64: TPASS: WEXITSTATUS(info1->exit_code) == 100 (100)

Summary:
passed   10
failed   0
broken   0
skipped  0
warnings 0
el10 s390x #
---
 lib/tst_clone.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/tst_clone.c b/lib/tst_clone.c
index 8638052e2..d14f8699f 100644
--- a/lib/tst_clone.c
+++ b/lib/tst_clone.c
@@ -36,9 +36,9 @@ pid_t tst_clone(const struct tst_clone_args *tst_args)
 	flags = args.exit_signal | args.flags;
 
 #ifdef __s390x__
-	pid = syscall(__NR_clone, NULL, flags);
+	pid = syscall(__NR_clone, NULL, flags, args.pidfd);
 #else
-	pid = syscall(__NR_clone, flags, NULL);
+	pid = syscall(__NR_clone, flags, NULL, args.pidfd);
 #endif
 
 	if (pid == -1)
-- 
2.53.0


[-- Attachment #3: Type: text/plain, Size: 60 bytes --]


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

Re: [LTP] Make ioctl_pidfd02 valgrind-compliant

[linuxtestproject.agent]

Hi Martin,

On Wed, 15 Apr 2026 10:03:44 +0200, Martin Cermak wrote:
> Make ioctl_pidfd02 valgrind-compliant

[...]

> This patch sets the parent_tidptr explicitly, making valgrind happy.

This is a bug fix; please add a Fixes: tag referencing the commit that
originally introduced the raw clone() fallback without the parent_tidptr
argument.

---
Note:

Our agent completed the review of the patch. 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] Make ioctl_pidfd02 valgrind-compliant

[Cyril Hrubis]

Hi!
>  #ifdef __s390x__
> -	pid = syscall(__NR_clone, NULL, flags);
> +	pid = syscall(__NR_clone, NULL, flags, args.pidfd);
>  #else
> -	pid = syscall(__NR_clone, flags, NULL);
> +	pid = syscall(__NR_clone, flags, NULL, args.pidfd);
>  #endif

Aren't we missing more that that?

Looking at kernel/fork.c:

#ifdef __ARCH_WANT_SYS_CLONE
#ifdef CONFIG_CLONE_BACKWARDS
SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp,
                 int __user *, parent_tidptr,
                 unsigned long, tls,
                 int __user *, child_tidptr)
#elif defined(CONFIG_CLONE_BACKWARDS2)
SYSCALL_DEFINE5(clone, unsigned long, newsp, unsigned long, clone_flags,
                 int __user *, parent_tidptr,
                 int __user *, child_tidptr,
                 unsigned long, tls)
#elif defined(CONFIG_CLONE_BACKWARDS3)
SYSCALL_DEFINE6(clone, unsigned long, clone_flags, unsigned long, newsp,
                int, stack_size,
                int __user *, parent_tidptr,
                int __user *, child_tidptr,
                unsigned long, tls)
#else
SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp,
                 int __user *, parent_tidptr,
                 int __user *, child_tidptr,
                 unsigned long, tls)
#endif
{
        struct kernel_clone_args args = {
                .flags          = (lower_32_bits(clone_flags) & ~CSIGNAL),
                .pidfd          = parent_tidptr,
                .child_tid      = child_tidptr,
                .parent_tid     = parent_tidptr,
                .exit_signal    = (lower_32_bits(clone_flags) & CSIGNAL),
                .stack          = newsp,
                .tls            = tls,
        };

        return kernel_clone(&args);
}
#endif

there is at least child_tidptr and tls as well.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] Make ioctl_pidfd02 valgrind-compliant

[Martin Cermak via ltp]

Hi Cyril,

On  Wed  2026-04-15  13:34 , Cyril Hrubis wrote:
> >  #ifdef __s390x__
> > -	pid = syscall(__NR_clone, NULL, flags);
> > +	pid = syscall(__NR_clone, NULL, flags, args.pidfd);
> >  #else
> > -	pid = syscall(__NR_clone, flags, NULL);
> > +	pid = syscall(__NR_clone, flags, NULL, args.pidfd);
> >  #endif
> 
> Aren't we missing more that that?
> 
> Looking at kernel/fork.c:
> 
> #ifdef __ARCH_WANT_SYS_CLONE
> #ifdef CONFIG_CLONE_BACKWARDS
> SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp,
>                  int __user *, parent_tidptr,
>                  unsigned long, tls,
>                  int __user *, child_tidptr)
> #elif defined(CONFIG_CLONE_BACKWARDS2)
> SYSCALL_DEFINE5(clone, unsigned long, newsp, unsigned long, clone_flags,
>                  int __user *, parent_tidptr,
>                  int __user *, child_tidptr,
>                  unsigned long, tls)
> #elif defined(CONFIG_CLONE_BACKWARDS3)
> SYSCALL_DEFINE6(clone, unsigned long, clone_flags, unsigned long, newsp,
>                 int, stack_size,
>                 int __user *, parent_tidptr,
>                 int __user *, child_tidptr,
>                 unsigned long, tls)
> #else
> SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp,
>                  int __user *, parent_tidptr,
>                  int __user *, child_tidptr,
>                  unsigned long, tls)
> #endif
> {
>         struct kernel_clone_args args = {
>                 .flags          = (lower_32_bits(clone_flags) & ~CSIGNAL),
>                 .pidfd          = parent_tidptr,
>                 .child_tid      = child_tidptr,
>                 .parent_tid     = parent_tidptr,
>                 .exit_signal    = (lower_32_bits(clone_flags) & CSIGNAL),
>                 .stack          = newsp,
>                 .tls            = tls,
>         };
> 
>         return kernel_clone(&args);
> }
> #endif
> 
> there is at least child_tidptr and tls as well.

I've updated my patch (and commit message) accordingly, and sent
it via git send-email per your suggestion - as a separate email.
Please check.

Cheers,
Martin


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