* [PATCH 1/2] ima_violations.sh: Wait for ima_mmap to exit
@ 2026-04-28 16:10 Petr Vorel
2026-04-28 16:10 ` [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints Petr Vorel
2026-04-30 9:03 ` [PATCH 1/2] ima_violations.sh: Wait for ima_mmap to exit Cyril Hrubis
0 siblings, 2 replies; 6+ messages in thread
From: Petr Vorel @ 2026-04-28 16:10 UTC (permalink / raw)
To: ltp
Cc: Petr Vorel, Cyril Hrubis, Avinesh Kumar, Wei Gao, Mimi Zohar,
linux-integrity, Martin Doucha
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints
2026-04-28 16:10 [PATCH 1/2] ima_violations.sh: Wait for ima_mmap to exit Petr Vorel
@ 2026-04-28 16:10 ` Petr Vorel
2026-04-29 11:15 ` [LTP] " Martin Doucha
2026-04-30 9:16 ` Cyril Hrubis
2026-04-30 9:03 ` [PATCH 1/2] ima_violations.sh: Wait for ima_mmap to exit Cyril Hrubis
1 sibling, 2 replies; 6+ messages in thread
From: Petr Vorel @ 2026-04-28 16:10 UTC (permalink / raw)
To: ltp
Cc: Petr Vorel, Cyril Hrubis, Avinesh Kumar, Wei Gao, Mimi Zohar,
linux-integrity
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints
2026-04-28 16:10 ` [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints Petr Vorel
@ 2026-04-29 11:15 ` Martin Doucha
2026-04-29 12:00 ` Petr Vorel
2026-04-30 9:16 ` Cyril Hrubis
1 sibling, 1 reply; 6+ messages in thread
From: Martin Doucha @ 2026-04-29 11:15 UTC (permalink / raw)
To: Petr Vorel, ltp; +Cc: linux-integrity
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints
2026-04-29 11:15 ` [LTP] " Martin Doucha
@ 2026-04-29 12:00 ` Petr Vorel
0 siblings, 0 replies; 6+ messages in thread
From: Petr Vorel @ 2026-04-29 12:00 UTC (permalink / raw)
To: Martin Doucha; +Cc: ltp, linux-integrity, Mimi Zohar
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints
2026-04-28 16:10 ` [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints Petr Vorel
2026-04-29 11:15 ` [LTP] " Martin Doucha
@ 2026-04-30 9:16 ` Cyril Hrubis
1 sibling, 0 replies; 6+ messages in thread
From: Cyril Hrubis @ 2026-04-30 9:16 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp, Avinesh Kumar, Wei Gao, Mimi Zohar, linux-integrity
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] ima_violations.sh: Wait for ima_mmap to exit
2026-04-28 16:10 [PATCH 1/2] ima_violations.sh: Wait for ima_mmap to exit Petr Vorel
2026-04-28 16:10 ` [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints Petr Vorel
@ 2026-04-30 9:03 ` Cyril Hrubis
1 sibling, 0 replies; 6+ messages in thread
From: Cyril Hrubis @ 2026-04-30 9:03 UTC (permalink / raw)
To: Petr Vorel
Cc: ltp, Avinesh Kumar, Wei Gao, Mimi Zohar, linux-integrity,
Martin Doucha
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-04-30 9:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-28 16:10 [PATCH 1/2] ima_violations.sh: Wait for ima_mmap to exit Petr Vorel
2026-04-28 16:10 ` [PATCH 2/2] ima_violations.sh: ima_mmap.c: Replace sleep with checkpoints Petr Vorel
2026-04-29 11:15 ` [LTP] " Martin Doucha
2026-04-29 12:00 ` Petr Vorel
2026-04-30 9:16 ` Cyril Hrubis
2026-04-30 9:03 ` [PATCH 1/2] ima_violations.sh: Wait for ima_mmap to exit Cyril Hrubis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox