Re: [LTP] [PATCH 2/2] ima_selinux.sh: Detect SELinux before loading policy

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

> Hi!

> On 2/17/25 14:08, Petr Vorel wrote:
> > Adding TST_SETUP_EARLY to run test specific setup before loading policy.
> > That allows to avoid loading IMA policy (which usually request reboot)
> > if the test would be skipped anyway.

> > Fixes: aac97cca96 ("ima_setup.sh: Allow to load predefined policy")
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> >   testcases/kernel/security/integrity/ima/tests/ima_selinux.sh | 3 ++-
> >   testcases/kernel/security/integrity/ima/tests/ima_setup.sh   | 2 ++
> >   2 files changed, 4 insertions(+), 1 deletion(-)

> > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
> > index 97c5d64ec5..577f7c2aca 100755
> > --- a/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
> > +++ b/testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
> > @@ -13,9 +13,10 @@
> >   TST_NEEDS_CMDS="awk cut grep tail"
> >   TST_CNT=2
> > -TST_SETUP="setup"
> >   TST_MIN_KVER="5.12"
> > +TST_SETUP_EARLY="setup"
> > +
> >   REQUIRED_POLICY_CONTENT='selinux.policy'
> >   setup()
> > diff --git a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> > index 1f1c267c4b..2a9f64978e 100644
> > --- a/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> > +++ b/testcases/kernel/security/integrity/ima/tests/ima_setup.sh
> > @@ -265,6 +265,8 @@ ima_setup()
> >   		cd "$TST_MNTPOINT"
> >   	fi
> > +	[ -n "$TST_SETUP_EARLY" ] && $TST_SETUP_EARLY
> > +
> Why not doing ". ima_setup.sh" at the end of setup() ?

I do not understand what you mean. How would it help?

Also since 04021637f4 ("tst_test.sh: Cleanup getopts usage")
shell library is required to be run at the end of a shell test,
just before tst_run.

FYI at least for some tests (these with $REQUIRED_BUILTIN_POLICY, e.g.
evm_overlay.sh) it makes sense to check /proc/cmdline content (e.g. run
load_ima_policy()) before the test specific setup to quit early enough. But OTOH
it's best to postpone loading the policy after other checks are done (e.g. here
check for SELinux), because loading policy often requires reboot. Maybe best
would be to do $REQUIRED_BUILTIN_POLICY check *before running the setup and
keep loading the policy *after* the setup. Maybe I should postpone loading
(see the diff below).

Kind regards,
Petr

> >   	if ! verify_ima_policy; then
> >   		load_ima_policy
> >   	fi
> Andrea

diff --git testcases/kernel/security/integrity/ima/tests/ima_selinux.sh testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
index c8eb80b641..97c5d64ec5 100755
--- testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
+++ testcases/kernel/security/integrity/ima/tests/ima_selinux.sh
@@ -13,10 +13,9 @@
 
 TST_NEEDS_CMDS="awk cut grep tail"
 TST_CNT=2
+TST_SETUP="setup"
 TST_MIN_KVER="5.12"
 
-IMA_SETUP_EARLY="setup"
-
 REQUIRED_POLICY_CONTENT='selinux.policy'
 
 setup()
diff --git testcases/kernel/security/integrity/ima/tests/ima_setup.sh testcases/kernel/security/integrity/ima/tests/ima_setup.sh
index befaf7f73c..f7ec5aeb35 100644
--- testcases/kernel/security/integrity/ima/tests/ima_setup.sh
+++ testcases/kernel/security/integrity/ima/tests/ima_setup.sh
@@ -245,6 +245,8 @@ load_ima_policy()
 
 ima_setup()
 {
+	local load_policy
+
 	SECURITYFS="$(mount_helper securityfs $SYSFS/kernel/security)"
 
 	IMA_DIR="$SECURITYFS/ima"
@@ -265,13 +267,15 @@ ima_setup()
 		cd "$TST_MNTPOINT"
 	fi
 
-	[ -n "$IMA_SETUP_EARLY" ] && $IMA_SETUP_EARLY
+	verify_ima_policy
+	load_policy=$?
+
+	[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER
 
-	if ! verify_ima_policy; then
+	if [ $load_policy -ne 0 ]; then
 		load_ima_policy
 	fi
 
-	[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER
 }
 
 ima_cleanup()