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

[Mimi Zohar <zohar@linux.ibm.com>]

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").

> 
> > 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.

> 
> > 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.

Mimi

> 
> > Should test2 report TCONF (or split the two assertions) when
> > CONFIG_IMA_WRITE_POLICY is not available?
> 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
> 
>