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
WARNING: multiple messages have this Message-ID (diff)
From: Petr Vorel <pvorel@suse.cz>
To: linuxtestproject.agent@gmail.com
Cc: linux-integrity@vger.kernel.org, ltp@lists.linux.it
Subject: Re: [LTP] 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
--
Mailing list info: https://lists.linux.it/listinfo/ltp
next prev parent reply other threads:[~2026-07-28 13:36 UTC|newest]
Thread overview: 5+ 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
2026-07-28 11:42 ` [LTP] " Petr Vorel
2026-07-28 12:58 ` [LTP] " linuxtestproject.agent
2026-07-28 13:35 ` Petr Vorel [this message]
2026-07-28 13:35 ` Petr Vorel
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.