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