[LTP] Avoid syscall param ioctl(generic) points to uninitialised byte(s)

[LTP] Avoid syscall param ioctl(generic) points to uninitialised byte(s)

[Martin Cermak via ltp]

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

Hi folks,

one more nit re uninitialised bytes attached.

Thanks,
Martin


[-- Attachment #2: 0001-Avoid-syscall-param-ioctl-generic-points-to-uninitia.patch --]
[-- Type: text/plain, Size: 1189 bytes --]

From b81924101f441d7f2103197c8b81a38e8920fda4 Mon Sep 17 00:00:00 2001
From: Super User <root@vm-10-0-185-15.hosted.upshift.rdu2.redhat.com>
Date: Wed, 20 Jan 2038 01:08:34 -0500
Subject: [PATCH] Avoid syscall param ioctl(generic) points to uninitialised
 byte(s)

lib/tst_clocks.c: fix uninitialized memory in run()  Reproducible
                  using userfaultfd01.

Signed-off-by: Martin Cermak <mcermak@redhat.com>
---
 testcases/kernel/syscalls/userfaultfd/userfaultfd01.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
index 7368d3863..d24766e7c 100644
--- a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
+++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
@@ -92,8 +92,8 @@ static void *handle_thread(void)
 static void run(unsigned int i)
 {
 	pthread_t thr;
-	struct uffdio_api uffdio_api = {};
-	struct uffdio_register uffdio_register;
+	struct uffdio_api uffdio_api = {0, };
+	struct uffdio_register uffdio_register = {0, };
 	struct tcase *tc = &tcases[i];
 
 	if (tc->kver == AFTER_5_11 && kver == BEFORE_5_11)
-- 
2.52.0


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


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

Re: [LTP] Avoid syscall param ioctl(generic) points to uninitialised byte(s)

[linuxtestproject.agent]

Hi Martin,

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.

On Wed, 20 Jan 2038, Martin Cermak wrote:
> Avoid syscall param ioctl(generic) points to uninitialised byte(s)

The subject is a raw Valgrind diagnostic string, not a commit subject.
Name the component and the fix, e.g.:
  userfaultfd01: initialize uffdio_register to avoid uninitialized bytes

> lib/tst_clocks.c: fix uninitialized memory in run()  Reproducible
>                   using userfaultfd01.

The file named here (lib/tst_clocks.c) is not the file changed by this
patch; the actual change is in
testcases/kernel/syscalls/userfaultfd/userfaultfd01.c. Please correct
this and explain why uffdio_register was uninitialized (the ioctls field
is not set before UFFDIO_REGISTER is called).

Regards,
LTP AI Reviewer

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

Re: [LTP] Avoid syscall param ioctl(generic) points to uninitialised byte(s)

[Cyril Hrubis]

Hi!
> From b81924101f441d7f2103197c8b81a38e8920fda4 Mon Sep 17 00:00:00 2001
> From: Super User <root@vm-10-0-185-15.hosted.upshift.rdu2.redhat.com>
> Date: Wed, 20 Jan 2038 01:08:34 -0500
> Subject: [PATCH] Avoid syscall param ioctl(generic) points to uninitialised
>  byte(s)
> 
> lib/tst_clocks.c: fix uninitialized memory in run()  Reproducible
>                   using userfaultfd01.

This description does not seem to correspond to the actual change.

> Signed-off-by: Martin Cermak <mcermak@redhat.com>
> ---
>  testcases/kernel/syscalls/userfaultfd/userfaultfd01.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
> index 7368d3863..d24766e7c 100644
> --- a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
> +++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
> @@ -92,8 +92,8 @@ static void *handle_thread(void)
>  static void run(unsigned int i)
>  {
>  	pthread_t thr;
> -	struct uffdio_api uffdio_api = {};
> -	struct uffdio_register uffdio_register;
> +	struct uffdio_api uffdio_api = {0, };
> +	struct uffdio_register uffdio_register = {0, };

Given that we were using = {}; in the code before can we keep using that
and for all instances?

Also I assume the problem is the ioctls field in the uffdio_api
structure that is not initialized beacuse is supposed to be set by
kernel before returning into userspace. Should valgrind warn about such
cases?


-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] Avoid syscall param ioctl(generic) points to uninitialised byte(s)

[Martin Cermak via ltp]

Hi Cyril,

On  Wed  2026-04-15  12:18 , Cyril Hrubis wrote:
> Hi!
> > From b81924101f441d7f2103197c8b81a38e8920fda4 Mon Sep 17 00:00:00 2001
> > From: Super User <root@vm-10-0-185-15.hosted.upshift.rdu2.redhat.com>
> > Date: Wed, 20 Jan 2038 01:08:34 -0500
> > Subject: [PATCH] Avoid syscall param ioctl(generic) points to uninitialised
> >  byte(s)
> > 
> > lib/tst_clocks.c: fix uninitialized memory in run()  Reproducible
> >                   using userfaultfd01.
> 
> This description does not seem to correspond to the actual change.

Oh, right, this is a true mistake in my proposed patch.  Sorry about that.

> 
> > Signed-off-by: Martin Cermak <mcermak@redhat.com>
> > ---
> >  testcases/kernel/syscalls/userfaultfd/userfaultfd01.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
> > index 7368d3863..d24766e7c 100644
> > --- a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
> > +++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
> > @@ -92,8 +92,8 @@ static void *handle_thread(void)
> >  static void run(unsigned int i)
> >  {
> >  	pthread_t thr;
> > -	struct uffdio_api uffdio_api = {};
> > -	struct uffdio_register uffdio_register;
> > +	struct uffdio_api uffdio_api = {0, };
> > +	struct uffdio_register uffdio_register = {0, };
> 
> Given that we were using = {}; in the code before can we keep using that
> and for all instances?

I saw both initializers in the LTP source, see `$ grep -rIn  '= {};'` and
`$ grep -rIn  '= { 0, };'`.  The latter one is compliant with older standards.
But to keep the coding style within this file, I've updated the initializer.

> Also I assume the problem is the ioctls field in the uffdio_api
> structure that is not initialized beacuse is supposed to be set by
> kernel before returning into userspace. Should valgrind warn about such
> cases?

You are right that zeroing just this uffdio_register.ioctls = 0; is suffi-
cient to to avoid the valgrind complaint.  Zeroing whole the struct seems
syntactically more concise and also correct.

Here is how Valgrind complains when uffdio_register.ioctls isn't zero inited:

el10 x86_64 # echo $T
/root/ltp/testcases/kernel/syscalls/userfaultfd/userfaultfd01
el10 x86_64 # 
el10 x86_64 # 
el10 x86_64 # 
el10 x86_64 # $T
tst_test.c:2059: TINFO: LTP version: 20260130
tst_test.c:2062: TINFO: Tested kernel: 6.12.0-218.el10.x86_64 #1 SMP PREEMPT_DYNAMIC Tue Mar 31 08:07:36 EDT 2026 x86_64
tst_kconfig.c:90: TINFO: Parsing kernel config '/lib/modules/6.12.0-218.el10.x86_64/build/.config'
tst_test.c:1887: TINFO: Overall timeout per run is 0h 25m 00s
userfaultfd01.c:120: TPASS: Pagefault handled!
userfaultfd01.c:120: TPASS: Pagefault handled!

Summary:
passed   2
failed   0
broken   0
skipped  0
warnings 0
el10 x86_64 # valgrind $T
tst_test.c:2059: TINFO: LTP version: 20260130
tst_test.c:2062: TINFO: Tested kernel: 6.12.0-218.el10.x86_64 #1 SMP PREEMPT_DYNAMIC Tue Mar 31 08:07:36 EDT 2026 x86_64
tst_kconfig.c:90: TINFO: Parsing kernel config '/lib/modules/6.12.0-218.el10.x86_64/build/.config'
tst_test.c:1887: TINFO: Overall timeout per run is 0h 25m 00s
==2747289== Syscall param ioctl(generic) points to uninitialised byte(s)
==2747289==    at 0x4966D1F: ioctl (ioctl.c:36)
==2747289==    by 0x40534C: run (userfaultfd01.c:113)
==2747289==    by 0x41164C: run_tests (tst_test.c:1730)
==2747289==    by 0x41164C: testrun (tst_test.c:1796)
==2747289==    by 0x41164C: fork_testrun (tst_test.c:1936)
==2747289==    by 0x413B74: tst_run_tcases (tst_test.c:2079)
==2747289==    by 0x404E8D: main (tst_test.h:755)
==2747289==  Address 0x1ffefff98a is on thread 1's stack
==2747289==  in frame #1, created by run (userfaultfd01.c:93)
==2747289== 
==2747289== 
==2747289== Exit program on first error (--exit-on-first-error=yes)
tst_test.c:1953: TBROK: Child returned with 253

Summary:
passed   0
failed   0
broken   1
skipped  0
warnings 0
el10 x86_64 # 


Here is the updated patch (inline):


---------------------------------8<------------------------------------
$ cat 0001-userfaultfd01.c-Zero-initialize-the-uffdio_register-.patch 
From e1ed7e74551bfa51204e1f45fe0b3bb64f21bcae Mon Sep 17 00:00:00 2001
From: Martin Cermak <mcermak@redhat.com>
Date: Thu, 16 Apr 2026 11:15:38 +0200
Subject: [PATCH] userfaultfd01.c: Zero-initialize the uffdio_register struct

Without this patch, valgrind complains when running userfaultfd01:

el10 x86_64 # valgrind testcases/kernel/syscalls/userfaultfd/userfaultfd01
tst_test.c:2059: TINFO: LTP version: 20260130
tst_test.c:2062: TINFO: Tested kernel: 6.12.0-218.el10.x86_64 #1 SMP PREEMPT_DYNAMIC Tue Mar 31 08:07:36 EDT 2026 x86_64
tst_kconfig.c:90: TINFO: Parsing kernel config '/lib/modules/6.12.0-218.el10.x86_64/build/.config'
tst_test.c:1887: TINFO: Overall timeout per run is 0h 25m 00s
==2748329== Syscall param ioctl(generic) points to uninitialised byte(s)
==2748329==    at 0x4966D1F: ioctl (ioctl.c:36)
==2748329==    by 0x40534C: run (userfaultfd01.c:113)
==2748329==    by 0x41164C: run_tests (tst_test.c:1730)
==2748329==    by 0x41164C: testrun (tst_test.c:1796)
==2748329==    by 0x41164C: fork_testrun (tst_test.c:1936)
==2748329==    by 0x413B74: tst_run_tcases (tst_test.c:2079)
==2748329==    by 0x404E8D: main (tst_test.h:755)
==2748329==  Address 0x1ffefff98a is on thread 1's stack
==2748329==  in frame #1, created by run (userfaultfd01.c:93)
==2748329==
==2748329==
==2748329== Exit program on first error (--exit-on-first-error=yes)
tst_test.c:1953: TBROK: Child returned with 253

It would be sufficient to zero out the uffdio_register.ioctls member.
This patch inits whole the struct before use.
---
 testcases/kernel/syscalls/userfaultfd/userfaultfd01.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
index 7368d3863..bd955ffdd 100644
--- a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
+++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
@@ -93,7 +93,7 @@ static void run(unsigned int i)
 {
        pthread_t thr;
        struct uffdio_api uffdio_api = {};
-       struct uffdio_register uffdio_register;
+       struct uffdio_register uffdio_register {};
        struct tcase *tc = &tcases[i];
 
        if (tc->kver == AFTER_5_11 && kver == BEFORE_5_11)
-- 
2.53.0

$ 
---------------------------------8<------------------------------------


Cheers,
Martin


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

Re: [LTP] Avoid syscall param ioctl(generic) points to uninitialised byte(s)

[Cyril Hrubis]

Hi!
> > > diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
> > > index 7368d3863..d24766e7c 100644
> > > --- a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
> > > +++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
> > > @@ -92,8 +92,8 @@ static void *handle_thread(void)
> > >  static void run(unsigned int i)
> > >  {
> > >  	pthread_t thr;
> > > -	struct uffdio_api uffdio_api = {};
> > > -	struct uffdio_register uffdio_register;
> > > +	struct uffdio_api uffdio_api = {0, };
> > > +	struct uffdio_register uffdio_register = {0, };
> > 
> > Given that we were using = {}; in the code before can we keep using that
> > and for all instances?
> 
> I saw both initializers in the LTP source, see `$ grep -rIn  '= {};'` and
> `$ grep -rIn  '= { 0, };'`.  The latter one is compliant with older standards.
> But to keep the coding style within this file, I've updated the initializer.
> 
> > Also I assume the problem is the ioctls field in the uffdio_api
> > structure that is not initialized beacuse is supposed to be set by
> > kernel before returning into userspace. Should valgrind warn about such
> > cases?
> 
> You are right that zeroing just this uffdio_register.ioctls = 0; is suffi-
> cient to to avoid the valgrind complaint.  Zeroing whole the struct seems
> syntactically more concise and also correct.

That's not what I was trying to point out. Some fields of structures
passed to ioctls are output-only. The question is if valgrind should
warn if we do not set output only field in such structure?

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] Avoid syscall param ioctl(generic) points to uninitialised byte(s)

[Martin Cermak via ltp]

On  Thu  2026-04-16  12:07 , Cyril Hrubis wrote:
> Hi!
> > > > diff --git a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
> > > > index 7368d3863..d24766e7c 100644
> > > > --- a/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
> > > > +++ b/testcases/kernel/syscalls/userfaultfd/userfaultfd01.c
> > > > @@ -92,8 +92,8 @@ static void *handle_thread(void)
> > > >  static void run(unsigned int i)
> > > >  {
> > > >  	pthread_t thr;
> > > > -	struct uffdio_api uffdio_api = {};
> > > > -	struct uffdio_register uffdio_register;
> > > > +	struct uffdio_api uffdio_api = {0, };
> > > > +	struct uffdio_register uffdio_register = {0, };
> > > 
> > > Given that we were using = {}; in the code before can we keep using that
> > > and for all instances?
> > 
> > I saw both initializers in the LTP source, see `$ grep -rIn  '= {};'` and
> > `$ grep -rIn  '= { 0, };'`.  The latter one is compliant with older standards.
> > But to keep the coding style within this file, I've updated the initializer.
> > 
> > > Also I assume the problem is the ioctls field in the uffdio_api
> > > structure that is not initialized beacuse is supposed to be set by
> > > kernel before returning into userspace. Should valgrind warn about such
> > > cases?
> > 
> > You are right that zeroing just this uffdio_register.ioctls = 0; is suffi-
> > cient to to avoid the valgrind complaint.  Zeroing whole the struct seems
> > syntactically more concise and also correct.
> 
> That's not what I was trying to point out. Some fields of structures
> passed to ioctls are output-only. The question is if valgrind should
> warn if we do not set output only field in such structure?

You are very right.  Curent version of valgrind doesn't recognize
UFFDIO_REGISTER, and treats this ioctl as unknown ioctl via
ML_(PRE_unknown_ioctl)(tid, ARG2, ARG3);  Good point, thanks!

Martin


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