Re: ima_setup.sh: Fix check_policy_writable() for kernel < 4.5

Petr Vorel <[email protected]> Mon, 3 Aug 2026 15:19:42 +0200
Newsgroups gmane.linux.ltp
Message-ID <20260803131942.GB246496@pevik>
Hi Mimi,

first, thanks for your feedback!

> On Tue, 2026-07-28 at 15:35 +0200, Petr Vorel wrote:
> > Hi Mimi,

> > once you have time, I'd appreciate your comments. Thanks!

> > > Hi Petr,

> > > On Tue, Jul 28, 2026 at 01:42:24PM +0200, Petr Vorel wrote:
> > > > ima_setup.sh: Fix check_policy_writable() for kernel < 4.5

> > > > -	# workaround for kernels < v4.18 without fix
> > > > +
> > > > +	# Workaround for kernels < v4.18 without fix
> > > >  	# ffb122de9a60b ("ima: Reflect correct permissions for policy")
> > > > -	echo "" 2> log > $IMA_POLICY
> > > > -	grep -q "Device or resource busy" log && return 1
> > > > +	# Require >= 4.5 to write multiple times via CONFIG_IMA_WRITE_POLICY
> > > > +	# 38d859f991f3 ("IMA: policy can now be updated multiple times")

> > > Is CONFIG_IMA_WRITE_POLICY really the reason for the 4.5 boundary here?

> Yes


> > > On >= 4.5 ima_release_policy() runs ima_check_policy(), which returns
> > > -EINVAL when ima_temp_rules is empty. The empty write therefore sets
> > > valid_policy = 0, ima_delete_rules() is called and IMA_FS_BUSY is
> > > cleared, so the probe stays non-destructive even when
> > > CONFIG_IMA_WRITE_POLICY=n.

> > Well, CONFIG_IMA_WRITE_POLICY did not exist in kernel < 4.5 (not sure if that's
> > obvious from the comment I added).

> Perhaps prefix the commit message with something like:

> IMA allows the builtin policy to be replaced with a custom policy just once. 
> Support for appending additional policy rules to the custom policy
> (CONFIG_IMA_WRITE_POLICY) was subsequently added in linux 4.5. Refer to commit 
> 38d859f991f3 ("IMA: policy can now be updated multiple times").

+1

> > > What makes < 4.5 different is the absence of ima_check_policy(): there
> > Yes, that's true that ima_check_policy() was added in v4.5.

> Agreed.

> > > the empty write is committed and securityfs_remove() drops the policy
> > > file. Would it be clearer to state that, and to keep the v4.18
> > > permission workaround in a separate paragraph, since the two comments
> > > describe unrelated things?

> > > > +	if tst_kvcmp -ge 4.5; then
> > > > +		echo "" 2> log > $IMA_POLICY
> > > > +		grep -q "Device or resource busy" log && return 1
> > > > +	fi
> > > >  	return 0

> > > With this, require_policy_writable() no longer TCONFs on < 4.5, so
> > > ima_policy.sh test2 ("verify that policy file is not opened concurrently
> > > and able to loaded multiple times") now runs there.

> > Hm, maybe test2() does not really makes sense to run on < v4.5. But test1()
> > certainly does. And echo "" > $IMA_POLICY really disables policy in v4.4,
> > likely due one of these from v4.5-rc1:
> > * 38d859f991f3 ("IMA: policy can now be updated multiple times")
> > * 0112721df4ed ("IMA: policy can be updated zero times")
> > @Mimi WDYT?

> Writing an invalid policy resulted in never being able to transition from a
> builtin policy to a custom policy, similar to writing an empty policy. Commit
> 0112721df4ed ("IMA: policy can be updated zero times") addressed the invalid
> policy case.

Thanks for the explanation.

> > > On those kernels one loader gets -EBUSY at open, the other succeeds and
> > > the policy file is then removed on release. That hits

> > > 	elif [ $rc1 -eq 0 ] || [ $rc2 -eq 0 ]; then
> > > 		tst_res TPASS "policy was loaded just by one process and able to loaded multiple times"

> > > so the test reports that the policy can be loaded multiple times while
> > > only the concurrency half of the assertion was exercised, and loading
> > > twice is not possible before 4.5.
> > That sounds correct and should be fixed.

> Correct, IMA originally permitted transitioning from a builtin policy to a
> custom policy.  Support for extending the custom IMA policy was added in 4.5.

Do you agree that test2 in ima_policy.sh should be skipped on kernel < 4.5? If
you consider useful running the test also on these old kernels I could just
correct the commit message for older kernel or simply ignore that fact (just add
a comment to the test that it's irrelevant for < 4.5. (I know 4.4 is not
supported any more, in fact it's just for SLES and possibly other LTS distros
still supporting this old kernel, but I still appreciate your opinion.)

> Mimi


> > > Should test2 report TCONF (or split the two assertions) when
> > > CONFIG_IMA_WRITE_POLICY is not available?

FYI we could sooner or later start using kernel config (e.g. look into
CONFIG_IMA_WRITE_POLICY and CONFIG_IMA_READ_POLICY as many LTP tests and now
even LTP library require kernel config (we resisted long time add kernel config
as a hard dependency, but finally Cyril added support in 2018).

But more important for me would be to migrate LTP IMA tests to shell loader [1]
or even rewrite to C API. We have deprecated tst_test.sh API some time ago.

Kind regards,
Petr

[1] https://linux-test-project.readthedocs.io/en/latest/developers/api_shell_tests.html

> > With requiring test2 to be run on kernel >= v4.5 (I'll add it to v2) everything
> > should work even with this patch (unmodified behavior on >= v4.5, avoid write
> > policy on < 4.5 when doing the check for ima_policy.sh test1 and for other IMA
> > tests).

> > > Verdict - Needs revision

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