From: Petr Vorel <pvorel@suse.cz>
To: linuxtestproject.agent@gmail.com
Cc: ltp@lists.linux.it, Mimi Zohar <zohar@linux.ibm.com>,
linux-integrity@vger.kernel.org
Subject: Re: ima_setup.sh: Fix check_policy_writable() for kernel < 4.5
Date: Tue, 28 Jul 2026 15:35:56 +0200 [thread overview]
Message-ID: <20260728133556.GA1128467@pevik> (raw)
In-Reply-To: <20260728125815.4069-1-linuxtestproject.agent@gmail.com>
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
next prev parent reply other threads:[~2026-07-28 13:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-28 11:42 [PATCH 1/1] ima_setup.sh: Fix check_policy_writable() for kernel < 4.5 Petr Vorel
[not found] ` <20260728125815.4069-1-linuxtestproject.agent@gmail.com>
2026-07-28 13:35 ` Petr Vorel [this message]
2026-07-29 6:54 ` [LTP] " Andrea Cervesato
2026-07-29 17:55 ` Mimi Zohar
2026-08-03 13:19 ` Petr Vorel
2026-08-07 18:49 ` Mimi Zohar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260728133556.GA1128467@pevik \
--to=pvorel@suse.cz \
--cc=linux-integrity@vger.kernel.org \
--cc=linuxtestproject.agent@gmail.com \
--cc=ltp@lists.linux.it \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox