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

Petr Vorel <[email protected]> Tue, 28 Jul 2026 15:35:56 +0200
Newsgroups org.kernel.vger.linux-integrity,it.linux.lists.ltp
Message-ID <20260728133556.GA1128467@pevik>
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?

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

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

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

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

> 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

> Pre-existing issues, not introduced by this patch:

> - "log" is created in the test working directory by
>   check_policy_writable() and load_ima_policy() and is never removed.
False positive - file is removed during cleanup of the temporary directory
because all tests use TST_NEEDS_TMPDIR=1.

> - $IMA_POLICY is expanded unquoted throughout ima_setup.sh.
False positive - /sys/kernel/security/ima/policy does not need to be quoted.

Kind regards,
Petr

> ---
> Note:

> 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