[LTP] [PATCH 1/2] ima_violations.sh: Wait for ima_mmap to exit

[LTP] [PATCH 1/2] ima_violations.sh: Wait for ima_mmap to exit

[Petr Vorel]

This fixes a race when even 2 sec sleep is not enough (f26399583e was
a wrong approach).

While at it, check for ima_mmap exit code (missing since ever).

Fixes: f26399583e ("ima/{ima_measurements,ima_violations}.sh: Avoid running on tmpfs")
Fixes: 3a8efbcc46 ("This patch adds Integrity Measurement Architecture(IMA) testing support ...")
Reported-by: Martin Doucha <mdoucha@suse.cz>
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .../kernel/security/integrity/ima/tests/ima_violations.sh | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
index 13cc9d804d..0c03c30786 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
@@ -157,6 +157,8 @@ test2()
 
 test3()
 {
+	local pid
+
 	tst_res TINFO "verify open_writers using mmapped files"
 
 	local search="open_writers"
@@ -168,6 +170,7 @@ test3()
 	echo 'testing testing' > $FILE
 
 	ima_mmap -f $FILE &
+	pid=$!
 	# wait for violations appear in logs
 	tst_sleep 1s
 
@@ -177,7 +180,10 @@ test3()
 	validate $num_violations $count $search
 
 	# wait for ima_mmap to exit, so we can umount
-	tst_sleep 2s
+	wait $pid
+	if [ $? -ne 0 ]; then
+		tst_brk TBROK "failed to execute ima_mmap"
+	fi
 }
 
 test4()
-- 
2.54.0


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

[LTP] [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints

[Petr Vorel]

Using checkpoints is a proper way in LTP new API [1] to avoid races and
waste of time.  It reduces 3 sec sleep in ima_mmap.c and 1 sec sleep in
ima_violations.sh with just checkpoints.

NOTE: tst_reinit() is really needed instead of .needs_checkpoints = 1
as documented in Shell-Test-API.asciidoc.

[1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests

Fixes: 0e4cbf753f ("security/ima: Rewrite tests into new API + fixes")
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/security/integrity/ima/src/ima_mmap.c     | 7 ++++---
 .../kernel/security/integrity/ima/tests/ima_violations.sh  | 6 +++++-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/security/integrity/ima/src/ima_mmap.c b/testcases/kernel/security/integrity/ima/src/ima_mmap.c
index 8596809ef4..09b22fd4f4 100644
--- a/testcases/kernel/security/integrity/ima/src/ima_mmap.c
+++ b/testcases/kernel/security/integrity/ima/src/ima_mmap.c
@@ -9,7 +9,6 @@
 
 #include "tst_test.h"
 
-#define SLEEP_AFTER_CLOSE 3
 #define MMAPSIZE 1024
 
 static char *filename;
@@ -35,8 +34,10 @@ static void run(void)
 	file = SAFE_MMAP(NULL, MMAPSIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
 	SAFE_CLOSE(fd);
 
-	tst_res(TINFO, "sleep %ds", SLEEP_AFTER_CLOSE);
-	sleep(SLEEP_AFTER_CLOSE);
+	tst_reinit();
+	TST_CHECKPOINT_WAIT(0);
+	/* keep running until ima_violations.sh open and close file */
+	TST_CHECKPOINT_WAKE_AND_WAIT(0);
 
 	tst_res(TPASS, "test completed");
 }
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
index 0c03c30786..d7dcd077b4 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
@@ -8,6 +8,7 @@
 # test[4-6] test 6.15 commit 5b3cd801155f ("ima: limit the number of open-writers integrity violations")
 # test[7-8] test 6.15 commit a414016218ca ("ima: limit the number of ToMToU integrity violations")
 
+TST_NEEDS_CHECKPOINTS=1
 TST_SETUP="setup"
 TST_CLEANUP="cleanup"
 TST_CNT=8
@@ -171,12 +172,15 @@ test3()
 
 	ima_mmap -f $FILE &
 	pid=$!
+
 	# wait for violations appear in logs
-	tst_sleep 1s
+	TST_CHECKPOINT_WAKE_AND_WAIT 0
 
 	open_file_read
 	close_file_read
 
+	TST_CHECKPOINT_WAKE 0
+
 	validate $num_violations $count $search
 
 	# wait for ima_mmap to exit, so we can umount
-- 
2.54.0


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

Re: [LTP] [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints

[Martin Doucha]

Hi,
for both patches:

Reviewed-by: Martin Doucha <mdoucha@suse.cz>

On 4/28/26 18:10, Petr Vorel wrote:
> Using checkpoints is a proper way in LTP new API [1] to avoid races and
> waste of time.  It reduces 3 sec sleep in ima_mmap.c and 1 sec sleep in
> ima_violations.sh with just checkpoints.
> 
> NOTE: tst_reinit() is really needed instead of .needs_checkpoints = 1
> as documented in Shell-Test-API.asciidoc.
> 
> [1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests
> 
> Fixes: 0e4cbf753f ("security/ima: Rewrite tests into new API + fixes")
> Suggested-by: Cyril Hrubis <chrubis@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>   testcases/kernel/security/integrity/ima/src/ima_mmap.c     | 7 ++++---
>   .../kernel/security/integrity/ima/tests/ima_violations.sh  | 6 +++++-
>   2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/src/ima_mmap.c b/testcases/kernel/security/integrity/ima/src/ima_mmap.c
> index 8596809ef4..09b22fd4f4 100644
> --- a/testcases/kernel/security/integrity/ima/src/ima_mmap.c
> +++ b/testcases/kernel/security/integrity/ima/src/ima_mmap.c
> @@ -9,7 +9,6 @@
>   
>   #include "tst_test.h"
>   
> -#define SLEEP_AFTER_CLOSE 3
>   #define MMAPSIZE 1024
>   
>   static char *filename;
> @@ -35,8 +34,10 @@ static void run(void)
>   	file = SAFE_MMAP(NULL, MMAPSIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
>   	SAFE_CLOSE(fd);
>   
> -	tst_res(TINFO, "sleep %ds", SLEEP_AFTER_CLOSE);
> -	sleep(SLEEP_AFTER_CLOSE);
> +	tst_reinit();
> +	TST_CHECKPOINT_WAIT(0);
> +	/* keep running until ima_violations.sh open and close file */
> +	TST_CHECKPOINT_WAKE_AND_WAIT(0);
>   
>   	tst_res(TPASS, "test completed");
>   }
> diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> index 0c03c30786..d7dcd077b4 100755
> --- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> +++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> @@ -8,6 +8,7 @@
>   # test[4-6] test 6.15 commit 5b3cd801155f ("ima: limit the number of open-writers integrity violations")
>   # test[7-8] test 6.15 commit a414016218ca ("ima: limit the number of ToMToU integrity violations")
>   
> +TST_NEEDS_CHECKPOINTS=1
>   TST_SETUP="setup"
>   TST_CLEANUP="cleanup"
>   TST_CNT=8
> @@ -171,12 +172,15 @@ test3()
>   
>   	ima_mmap -f $FILE &
>   	pid=$!
> +
>   	# wait for violations appear in logs
> -	tst_sleep 1s
> +	TST_CHECKPOINT_WAKE_AND_WAIT 0
>   
>   	open_file_read
>   	close_file_read
>   
> +	TST_CHECKPOINT_WAKE 0
> +
>   	validate $num_violations $count $search
>   
>   	# wait for ima_mmap to exit, so we can umount


-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

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

Re: [LTP] [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints

[Petr Vorel]

Hi Martin, all,

> Hi,
> for both patches:

> Reviewed-by: Martin Doucha <mdoucha@suse.cz>

Thanks for review, patchset merged.

Kind regards,
Petr

> On 4/28/26 18:10, Petr Vorel wrote:
> > Using checkpoints is a proper way in LTP new API [1] to avoid races and
> > waste of time.  It reduces 3 sec sleep in ima_mmap.c and 1 sec sleep in
> > ima_violations.sh with just checkpoints.

> > NOTE: tst_reinit() is really needed instead of .needs_checkpoints = 1
> > as documented in Shell-Test-API.asciidoc.

> > [1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests

> > Fixes: 0e4cbf753f ("security/ima: Rewrite tests into new API + fixes")
> > Suggested-by: Cyril Hrubis <chrubis@suse.cz>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> >   testcases/kernel/security/integrity/ima/src/ima_mmap.c     | 7 ++++---
> >   .../kernel/security/integrity/ima/tests/ima_violations.sh  | 6 +++++-
> >   2 files changed, 9 insertions(+), 4 deletions(-)

> > diff --git a/testcases/kernel/security/integrity/ima/src/ima_mmap.c b/testcases/kernel/security/integrity/ima/src/ima_mmap.c
> > index 8596809ef4..09b22fd4f4 100644
> > --- a/testcases/kernel/security/integrity/ima/src/ima_mmap.c
> > +++ b/testcases/kernel/security/integrity/ima/src/ima_mmap.c
> > @@ -9,7 +9,6 @@
> >   #include "tst_test.h"
> > -#define SLEEP_AFTER_CLOSE 3
> >   #define MMAPSIZE 1024
> >   static char *filename;
> > @@ -35,8 +34,10 @@ static void run(void)
> >   	file = SAFE_MMAP(NULL, MMAPSIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> >   	SAFE_CLOSE(fd);
> > -	tst_res(TINFO, "sleep %ds", SLEEP_AFTER_CLOSE);
> > -	sleep(SLEEP_AFTER_CLOSE);
> > +	tst_reinit();
> > +	TST_CHECKPOINT_WAIT(0);
> > +	/* keep running until ima_violations.sh open and close file */
> > +	TST_CHECKPOINT_WAKE_AND_WAIT(0);
> >   	tst_res(TPASS, "test completed");
> >   }
> > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> > index 0c03c30786..d7dcd077b4 100755
> > --- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> > +++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
> > @@ -8,6 +8,7 @@
> >   # test[4-6] test 6.15 commit 5b3cd801155f ("ima: limit the number of open-writers integrity violations")
> >   # test[7-8] test 6.15 commit a414016218ca ("ima: limit the number of ToMToU integrity violations")
> > +TST_NEEDS_CHECKPOINTS=1
> >   TST_SETUP="setup"
> >   TST_CLEANUP="cleanup"
> >   TST_CNT=8
> > @@ -171,12 +172,15 @@ test3()
> >   	ima_mmap -f $FILE &
> >   	pid=$!
> > +
> >   	# wait for violations appear in logs
> > -	tst_sleep 1s
> > +	TST_CHECKPOINT_WAKE_AND_WAIT 0
> >   	open_file_read
> >   	close_file_read
> > +	TST_CHECKPOINT_WAKE 0
> > +
> >   	validate $num_violations $count $search
> >   	# wait for ima_mmap to exit, so we can umount

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

Re: [LTP] [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints

[Cyril Hrubis]

Hi!
> Using checkpoints is a proper way in LTP new API [1] to avoid races and
> waste of time.  It reduces 3 sec sleep in ima_mmap.c and 1 sec sleep in
> ima_violations.sh with just checkpoints.
> 
> NOTE: tst_reinit() is really needed instead of .needs_checkpoints = 1
> as documented in Shell-Test-API.asciidoc.
> 
> [1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests
> 
> Fixes: 0e4cbf753f ("security/ima: Rewrite tests into new API + fixes")
> Suggested-by: Cyril Hrubis <chrubis@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  testcases/kernel/security/integrity/ima/src/ima_mmap.c     | 7 ++++---
>  .../kernel/security/integrity/ima/tests/ima_violations.sh  | 6 +++++-
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/testcases/kernel/security/integrity/ima/src/ima_mmap.c b/testcases/kernel/security/integrity/ima/src/ima_mmap.c
> index 8596809ef4..09b22fd4f4 100644
> --- a/testcases/kernel/security/integrity/ima/src/ima_mmap.c
> +++ b/testcases/kernel/security/integrity/ima/src/ima_mmap.c
> @@ -9,7 +9,6 @@
>  
>  #include "tst_test.h"
>  
> -#define SLEEP_AFTER_CLOSE 3
>  #define MMAPSIZE 1024
>  
>  static char *filename;
> @@ -35,8 +34,10 @@ static void run(void)
>  	file = SAFE_MMAP(NULL, MMAPSIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
>  	SAFE_CLOSE(fd);
>  
> -	tst_res(TINFO, "sleep %ds", SLEEP_AFTER_CLOSE);
> -	sleep(SLEEP_AFTER_CLOSE);
> +	tst_reinit();
> +	TST_CHECKPOINT_WAIT(0);
> +	/* keep running until ima_violations.sh open and close file */
> +	TST_CHECKPOINT_WAKE_AND_WAIT(0);
>  
>  	tst_res(TPASS, "test completed");
>  }

C helpers that call tst_reinit() should implement main(). These are not
complete tests, just helpers. The main problem is that if you add
tst_reinit() to a source that defines tst_test structure you are
initializing the test library for a second time, which overwrites the
some of the already initialized variables.

We probably want:

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 26f6510a0..971a184f2 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -196,6 +196,9 @@ void tst_reinit(void)
        size_t size = getpagesize();
        int fd;

+       if (ipc)
+               tst_brk(TBROK, "Test library already initialized!");
+
        if (!path)
                tst_brk(TBROK, IPC_ENV_VAR" is not defined");


So that this kind of mistake is caught early.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] ima_violations.sh: Wait for ima_mmap to exit

[linuxtestproject.agent]

Hi Petr,

On Tue, Apr 28, 2026 at 18:10:33 +0200, Petr Vorel wrote:
> ima_violations.sh: Wait for ima_mmap to exit
> ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints

Both patches look good.

Reviewed-by: LTP AI Reviewer <ltp-ai@noreply.github.com>

Pre-existing issues noticed in the surrounding code (not introduced
by this patch):

- ima_mmap.c:2-8 — no doc comment describing the test; only a copyright block present
- ima_violations.sh — missing # --- doc and # --- env blocks
- ima_mmap.c:18-25 — cleanup() does not set file = NULL after SAFE_MUNMAP

---
Note:

Our agent completed the review of the patch. The full review can be
found at: https://github.com/linux-test-project/ltp-agent/actions/runs/25066130804

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] [PATCH 1/2] ima_violations.sh: Wait for ima_mmap to exit

[Cyril Hrubis]

Hi!
>  test3()
>  {
> +	local pid
> +
>  	tst_res TINFO "verify open_writers using mmapped files"
>  
>  	local search="open_writers"
> @@ -168,6 +170,7 @@ test3()
>  	echo 'testing testing' > $FILE
>  
>  	ima_mmap -f $FILE &
> +	pid=$!
>  	# wait for violations appear in logs
>  	tst_sleep 1s
>  
> @@ -177,7 +180,10 @@ test3()
>  	validate $num_violations $count $search
>  
>  	# wait for ima_mmap to exit, so we can umount
> -	tst_sleep 2s
> +	wait $pid

You can simplify this just by doing 'wait' that will wait for all
background jobs (we have only one here).

Either way:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

> +	if [ $? -ne 0 ]; then
> +		tst_brk TBROK "failed to execute ima_mmap"
> +	fi
>  }
>  
>  test4()
> -- 
> 2.54.0
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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