* Re: [PATCH] ima_violations.sh: Another fix of condition evaluation
From: Mimi Zohar @ 2025-12-12 3:42 UTC (permalink / raw)
To: Petr Vorel, ltp; +Cc: linux-integrity, Martin Doucha
In-Reply-To: <20251211111046.87297-1-pvorel@suse.cz>
On Thu, 2025-12-11 at 12:10 +0100, Petr Vorel wrote:
> c0c35509f9 was not enough to fix evaluation against empty
> $expected_violations:
>
> ima_violations 1 TINFO: verify open writers violation
> /opt/ltp/testcases/bin/ima_violations.sh: line 96: [: 0: unary operator expected
>
> Therefore split checks into two if.
>
> Also improvements (readability)
> * shorten line length with saving subtraction into variable
> * evaluate empty variable with ${:-}
>
> Fixes: 726ed71905 ("ima_violations.sh: Update validate() to support multiple violations")
> Reported-by: Martin Doucha <mdoucha@suse.cz>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
Thanks, Martin, Petr. LGTM
> ---
> NOTE: this was found on old SLES 4.4 based kernel which does not log
> validations. But missing validations might be just a Secure Boot related
> setup problem:
>
> $ mokutil --sb-state
> Secure Boot: EFI variables not supported on SUT
>
> Events are logged when Secure Boot is off:
> $ mokutil --sb-state
> SecureBoot disabled
>
> Or maybe violations worked differently on the old kernel (I remember
> only 6.15 change).
Violations only occur when there are policy rules containing "func=FILE_CHECK"
defined. The secure boot mode should only affects the arch specific policies,
which do not include "func=FILE_CHECK" rules. There is a slight difference
between the builtin the original "ima_tcb" and newer "ima_policy=tcb" policies,
which might affect violations.
--
thanks,
Mimi
^ permalink raw reply
* Re: [RFC][PATCH] ima: Add support for staging measurements for deletion
From: steven chen @ 2025-12-11 23:38 UTC (permalink / raw)
To: Roberto Sassu, Gregory Lumen
Cc: corbet, zohar, dmitry.kasatkin, eric.snowberg, paul, jmorris,
serge, linux-doc, linux-kernel, linux-integrity,
linux-security-module, nramas, Roberto Sassu, steven chen
In-Reply-To: <2f550d4cd860022e990d1de62049df85a6a86df8.camel@huaweicloud.com>
On 12/11/2025 6:50 AM, Roberto Sassu wrote:
> On Thu, 2025-12-11 at 10:56 +0100, Roberto Sassu wrote:
>> On Wed, 2025-12-10 at 11:12 -0800, Gregory Lumen wrote:
>>> Roberto,
>>>
>>> The proposed approach appears to be workable. However, if our primary goal
>>> here is to enable UM to free kernel memory consumed by the IMA log with an
>>> absolute minimum of kernel functionality/change, then I would argue that
>>> the proposed Stage-then-delete approach still represents unnecessary
>>> complexity when compared to a trim-to-N solution. Specifically:
> The benefit of the Stage-then-delete is that you don't need to scan the
> IMA measurements list in advance to determine what to trim, you just
> trim everything by swapping list head (very fast) and then you can read
> and delete the measurements out of the hot path.
>
> [...]
Trim N entries proposal can do the same speed as staged proposal or
faster than staged proposal.
Steven
>>> - There exists a potential UM measurement-loss race condition introduced
>>> by the staging functionality that would not exist with a trim-to-N
>>> approach. (Occurs if a kexec call occurs after a UM agent has staged
>>> measurements for deletion, but has not completed copying them to
>>> userspace). This could be avoided by persisting staged measurements across
>>> kexec calls at the cost of making the proposed change larger.
>> The solution is to coordinate the staging with kexec in user space.
> To avoid requiring coordination in user space, I will try to see if I
> could improve my patch to prepend the staged entries to the current
> measurement list, before serializing them for kexec().
Trim N entries proposal does not need this at all.
Steven
> Roberto
^ permalink raw reply
* Re: xfs/ima: Regression caching i_version
From: Jeff Layton @ 2025-12-11 22:50 UTC (permalink / raw)
To: Frederick Lawler
Cc: Christian Brauner, Darrick J. Wong, Josef Bacik, Carlos Maiolino,
linux-xfs, linux-kernel, linux-security-module, linux-integrity,
Roberto Sassu, kernel-team
In-Reply-To: <aTtF3BPUcd9crhAm@CMGLRV3>
On Thu, 2025-12-11 at 16:29 -0600, Frederick Lawler wrote:
> On Fri, Dec 12, 2025 at 06:41:00AM +0900, Jeff Layton wrote:
> > On Thu, 2025-12-11 at 15:12 -0600, Frederick Lawler wrote:
> > > On Fri, Dec 12, 2025 at 05:55:45AM +0900, Jeff Layton wrote:
> > > > On Thu, 2025-12-11 at 14:29 -0600, Frederick Lawler wrote:
> > > > > Hi Jeff,
> > > > >
> > > > > While testing 6.18, I think I found a regression with
> > > > > commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps") since 6.13
> > > > > where IMA is no longer able to properly cache i_version when we overlay
> > > > > tmpfs on top of XFS. Each measurement diff check in function
> > > > > process_measurement() reports that the i_version is
> > > > > always set to zero for iint->real_inode.version.
> > > > >
> > > > > The function ima_collect_measurement() is looking to extract the version
> > > > > from the cookie on next measurement to cache i_version.
> > > > >
> > > > > I'm unclear from the commit description what the right approach here is:
> > > > > update in IMA land by checking for time changes, or do
> > > > > something else such as adding the cookie back.
> > > > >
> > > > >
> > > >
> > > > What we probably want to do is switch to using the ctime to manufacture
> > > > a change attribute when STATX_CHANGE_ATTRIBUTE is not set in the statx
> > > > reply.
> > > >
> > > > IIRC, IMA doesn't need to persist these values across reboot, so
> > > > something like this (completely untested) might work, but it may be
> > > > better to lift nfsd4_change_attribute() into a common header and use
> > > > the same mechanism for both:
> > >
> > > I agree lifting nfsd4_change_attribute(), if anything else, a consistent
> > > place to fetch the i_version from. Am I correct in my understanding that
> > > the XOR on the times will cancel out and result in just the i_version?
> >
> > No. I was just using the XOR to mix the tv_sec and tv_nsec fields
> > together in a way that (hopefully) wouldn't generate collisions. It's
> > quite not as robust as what nfsd4_change_attribute() does, but might be
> > sane enough for IMA.
> >
> > > IMA is calling into inode_eq_iversion() to perform the comparison
> > > between the cached value and inode.i_version.
> >
> > That just looks at the i_version field directly without going through -
> > > getattr, so that would need to be switched over as well. Could
> > integrity_inode_attrs_changed() use vfs_getattr_nosec() and compare the
> > result?
>
> That makes sense to me. I'll look through it a bit more, roll a patch, and
> see what the IMA folks have to say (unless they comment here first).
>
Great, thanks. Looks like ima_check_last_writer() might also need a
similar change.
I'm not sure, but It's possible that vfs_getattr_nosec() may not be
callable from all the contexts where integrity_inode_attrs_changed() is
called. If that is the case, then you may just need to convert IMA back
to accessing the i_version field directly instead of going through
vfs_getattr_nosec().
That's less than ideal though, as it will limit the filesystems where
IMA can be implemented. You'll need to come up with a way to deal with
the fact that XFS's i_version field can change on atime updates.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: xfs/ima: Regression caching i_version
From: Frederick Lawler @ 2025-12-11 22:29 UTC (permalink / raw)
To: Jeff Layton
Cc: Christian Brauner, Darrick J. Wong, Josef Bacik, Carlos Maiolino,
linux-xfs, linux-kernel, linux-security-module, linux-integrity,
Roberto Sassu, kernel-team
In-Reply-To: <fab68913274be1cb2a629372eafd52205a51b74e.camel@kernel.org>
On Fri, Dec 12, 2025 at 06:41:00AM +0900, Jeff Layton wrote:
> On Thu, 2025-12-11 at 15:12 -0600, Frederick Lawler wrote:
> > On Fri, Dec 12, 2025 at 05:55:45AM +0900, Jeff Layton wrote:
> > > On Thu, 2025-12-11 at 14:29 -0600, Frederick Lawler wrote:
> > > > Hi Jeff,
> > > >
> > > > While testing 6.18, I think I found a regression with
> > > > commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps") since 6.13
> > > > where IMA is no longer able to properly cache i_version when we overlay
> > > > tmpfs on top of XFS. Each measurement diff check in function
> > > > process_measurement() reports that the i_version is
> > > > always set to zero for iint->real_inode.version.
> > > >
> > > > The function ima_collect_measurement() is looking to extract the version
> > > > from the cookie on next measurement to cache i_version.
> > > >
> > > > I'm unclear from the commit description what the right approach here is:
> > > > update in IMA land by checking for time changes, or do
> > > > something else such as adding the cookie back.
> > > >
> > > >
> > >
> > > What we probably want to do is switch to using the ctime to manufacture
> > > a change attribute when STATX_CHANGE_ATTRIBUTE is not set in the statx
> > > reply.
> > >
> > > IIRC, IMA doesn't need to persist these values across reboot, so
> > > something like this (completely untested) might work, but it may be
> > > better to lift nfsd4_change_attribute() into a common header and use
> > > the same mechanism for both:
> >
> > I agree lifting nfsd4_change_attribute(), if anything else, a consistent
> > place to fetch the i_version from. Am I correct in my understanding that
> > the XOR on the times will cancel out and result in just the i_version?
>
> No. I was just using the XOR to mix the tv_sec and tv_nsec fields
> together in a way that (hopefully) wouldn't generate collisions. It's
> quite not as robust as what nfsd4_change_attribute() does, but might be
> sane enough for IMA.
>
> > IMA is calling into inode_eq_iversion() to perform the comparison
> > between the cached value and inode.i_version.
>
> That just looks at the i_version field directly without going through -
> >getattr, so that would need to be switched over as well. Could
> integrity_inode_attrs_changed() use vfs_getattr_nosec() and compare the
> result?
That makes sense to me. I'll look through it a bit more, roll a patch, and
see what the IMA folks have to say (unless they comment here first).
Thanks Jeff
>
>
> > >
> > > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > > index c35ea613c9f8..5a71845f579e 100644
> > > --- a/security/integrity/ima/ima_api.c
> > > +++ b/security/integrity/ima/ima_api.c
> > > @@ -272,10 +272,14 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > > * to an initial measurement/appraisal/audit, but was modified to
> > > * assume the file changed.
> > > */
> > > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > > + result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE | STATX_CTIME,
> > > AT_STATX_SYNC_AS_STAT);
> > > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > > - i_version = stat.change_cookie;
> > > + if (!result) {
> > > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > > + i_version = stat.change_cookie;
> > > + else if (stat.result_mask & STATX_CTIME)
> > > + i_version = stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> > > + }
> > > hash.hdr.algo = algo;
> > > hash.hdr.length = hash_digest_size[algo];
> > >
>
> --
> Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: [RFC][PATCH] ima: Add support for staging measurements for deletion
From: steven chen @ 2025-12-11 22:06 UTC (permalink / raw)
To: Roberto Sassu, corbet, zohar, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, nramas, Roberto Sassu, steven chen
In-Reply-To: <b701686d-212e-4152-9db9-0c56f21e1fdc@linux.microsoft.com>
On 12/11/2025 11:20 AM, steven chen wrote:
> On 12/11/2025 2:18 AM, Roberto Sassu wrote:
>> On Wed, 2025-12-10 at 16:03 -0800, steven chen wrote:
>>> On 12/9/2025 2:17 AM, Roberto Sassu wrote:
>>>> From: Roberto Sassu <roberto.sassu@huawei.com>
>>>>
>>>> Introduce the ability of staging the entire of the IMA measurement
>>>> list, or
>>>> a portion, for deletion. Staging means moving the current content
>>>> of the
>>>> measurement list to a separate location, and allowing users to read
>>>> and
>>>> delete it. This causes the measurement list to be atomically truncated
>>>> before new measurements can be added. Staging can be done only once
>>>> at a
>>>> time.
>>>>
>>>> User space is responsible to concatenate the staged IMA
>>>> measurements list
>>>> portions following the temporal order in which the operations were
>>>> done,
>>>> together with the current measurement list. Then, it can send the
>>>> collected
>>>> data to the remote verifiers.
>>>>
>>>> The benefit of this solution is the ability to free precious kernel
>>>> memory,
>>>> in exchange of delegating user space to reconstruct the full
>>>> measurement
>>>> list from the chunks. No trust needs to be given to user space,
>>>> since the
>>>> integrity of the measurement list is protected by the TPM.
>>>>
>>>> By default, staging the measurements list for deletion does not
>>>> alter the
>>>> hash table. When staging is done, IMA is still able to detect
>>>> collisions on
>>>> the staged and later deleted measurement entries, by keeping the entry
>>>> digests (only template data are freed).
>>>>
>>>> However, since during the measurements list serialization only the
>>>> SHA1
>>>> digest is passed, and since there are no template data to
>>>> recalculate the
>>>> other digests from, the hash table is currently not populated with
>>>> digests
>>>> from staged/deleted entries after kexec().
>>>>
>>>> Introduce the new kernel option ima_flush_htable to decide whether
>>>> or not
>>>> the digests of staged measurement entries are flushed from the hash
>>>> table.
>>>>
>>>> Then, introduce ascii_runtime_measurements_staged_<algo> and
>>>> binary_runtime_measurement_staged_<algo> interfaces to stage/delete
>>>> the
>>>> measurements. Use 'echo A > <IMA interface>' and 'echo D > <IMA
>>>> interface>'
>>>> to respectively stage and delete the entire measurements list. Use
>>>> 'echo N > <IMA interface>', with N between 1 and ULONG_MAX, to
>>>> stage the
>>>> selected portion of the measurements list.
>>>>
>>>> The ima_measure_users counter (protected by the ima_measure_lock
>>>> mutex) has
>>>> been introduced to protect access to the measurement list and the
>>>> staged
>>>> part. The open method of all the measurement interfaces has been
>>>> extended
>>>> to allow only one writer at a time or, in alternative, multiple
>>>> readers.
>>>> The write permission is used to stage/delete the measurements, the
>>>> read
>>>> permission to read them. Write requires also the CAP_SYS_ADMIN
>>>> capability.
>>> Hi Roberto,
>>>
>>> I released version 2 of trim N entries patch as bellow:
>>>
>>> [PATCH v2 0/1] Trim N entries of IMA event logs
>>> <https://lore.kernel.org/linux-integrity/20251210235314.3341-1-chenste@linux.microsoft.com/T/#t>
>>>
>>>
>>> I adapted some of your idea and I think trim N has following
>>> advantages:
>>> 1: less measurement list hold time than your current implementation
>>> 2. operation much simple for user space
>>> 3. less kernel code change
>>> 4. no potential issue as Gregory mentioned.
>> Please have a look at:
>>
>> https://marc.info/?l=linux-integrity&m=176545085325473&w=2
>>
>> and let me know if I'm missing something.
>>
>> Thanks
>>
>> Roberto
>
> Hi Roberto,
>
> what does this staging solution do that's not achieved by trim N
> entries solution?
>
> You did not address all my comments and your other idea make things
> more complex.
The following are steps for both proposals:
the steps for trim N solution:
1. User space reads list without lock
2. User space decides to trim N entries and send command to kernel
3. Kernel will lock the list use the same or less time as
staged solution use(we can improve this together)
the steps for staged N solution:
1. User space reads list without lock
2. User space stages list with lock
3. User space decides to trim N entries and send command to kernel
4. Kernel trim staged list (staged list may not empty after trim)
5. kexec save the staged list during soft reboot
6. kexec restore the staged list during soft reboot
>
> Also, Trim N solution is simple and will bring following two good points:
> easy for future IMA development
will be easier for future "Kexec Measurement List Passing" project
> easy for code maintenance
>
> Could you also add your comments on the trim N solution?
>
> Thanks,
>
> Steven
>
>>
>>> Thanks,
>>>
>>> Steven
^ permalink raw reply
* Re: xfs/ima: Regression caching i_version
From: Jeff Layton @ 2025-12-11 21:41 UTC (permalink / raw)
To: Frederick Lawler
Cc: Christian Brauner, Darrick J. Wong, Josef Bacik, Carlos Maiolino,
linux-xfs, linux-kernel, linux-security-module, linux-integrity,
Roberto Sassu, kernel-team
In-Reply-To: <aTszzVJkIqBpYLst@CMGLRV3>
On Thu, 2025-12-11 at 15:12 -0600, Frederick Lawler wrote:
> On Fri, Dec 12, 2025 at 05:55:45AM +0900, Jeff Layton wrote:
> > On Thu, 2025-12-11 at 14:29 -0600, Frederick Lawler wrote:
> > > Hi Jeff,
> > >
> > > While testing 6.18, I think I found a regression with
> > > commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps") since 6.13
> > > where IMA is no longer able to properly cache i_version when we overlay
> > > tmpfs on top of XFS. Each measurement diff check in function
> > > process_measurement() reports that the i_version is
> > > always set to zero for iint->real_inode.version.
> > >
> > > The function ima_collect_measurement() is looking to extract the version
> > > from the cookie on next measurement to cache i_version.
> > >
> > > I'm unclear from the commit description what the right approach here is:
> > > update in IMA land by checking for time changes, or do
> > > something else such as adding the cookie back.
> > >
> > >
> >
> > What we probably want to do is switch to using the ctime to manufacture
> > a change attribute when STATX_CHANGE_ATTRIBUTE is not set in the statx
> > reply.
> >
> > IIRC, IMA doesn't need to persist these values across reboot, so
> > something like this (completely untested) might work, but it may be
> > better to lift nfsd4_change_attribute() into a common header and use
> > the same mechanism for both:
>
> I agree lifting nfsd4_change_attribute(), if anything else, a consistent
> place to fetch the i_version from. Am I correct in my understanding that
> the XOR on the times will cancel out and result in just the i_version?
No. I was just using the XOR to mix the tv_sec and tv_nsec fields
together in a way that (hopefully) wouldn't generate collisions. It's
quite not as robust as what nfsd4_change_attribute() does, but might be
sane enough for IMA.
> IMA is calling into inode_eq_iversion() to perform the comparison
> between the cached value and inode.i_version.
That just looks at the i_version field directly without going through -
>getattr, so that would need to be switched over as well. Could
integrity_inode_attrs_changed() use vfs_getattr_nosec() and compare the
result?
> >
> > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > index c35ea613c9f8..5a71845f579e 100644
> > --- a/security/integrity/ima/ima_api.c
> > +++ b/security/integrity/ima/ima_api.c
> > @@ -272,10 +272,14 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > * to an initial measurement/appraisal/audit, but was modified to
> > * assume the file changed.
> > */
> > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > + result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT);
> > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > - i_version = stat.change_cookie;
> > + if (!result) {
> > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > + i_version = stat.change_cookie;
> > + else if (stat.result_mask & STATX_CTIME)
> > + i_version = stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> > + }
> > hash.hdr.algo = algo;
> > hash.hdr.length = hash_digest_size[algo];
> >
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply
* Re: xfs/ima: Regression caching i_version
From: Frederick Lawler @ 2025-12-11 21:12 UTC (permalink / raw)
To: Jeff Layton
Cc: Christian Brauner, Darrick J. Wong, Josef Bacik, Carlos Maiolino,
linux-xfs, linux-kernel, linux-security-module, linux-integrity,
Roberto Sassu, kernel-team
In-Reply-To: <2b193b5ccd696420196ae9059f83dcc8b3f06473.camel@kernel.org>
On Fri, Dec 12, 2025 at 05:55:45AM +0900, Jeff Layton wrote:
> On Thu, 2025-12-11 at 14:29 -0600, Frederick Lawler wrote:
> > Hi Jeff,
> >
> > While testing 6.18, I think I found a regression with
> > commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps") since 6.13
> > where IMA is no longer able to properly cache i_version when we overlay
> > tmpfs on top of XFS. Each measurement diff check in function
> > process_measurement() reports that the i_version is
> > always set to zero for iint->real_inode.version.
> >
> > The function ima_collect_measurement() is looking to extract the version
> > from the cookie on next measurement to cache i_version.
> >
> > I'm unclear from the commit description what the right approach here is:
> > update in IMA land by checking for time changes, or do
> > something else such as adding the cookie back.
> >
> >
>
> What we probably want to do is switch to using the ctime to manufacture
> a change attribute when STATX_CHANGE_ATTRIBUTE is not set in the statx
> reply.
>
> IIRC, IMA doesn't need to persist these values across reboot, so
> something like this (completely untested) might work, but it may be
> better to lift nfsd4_change_attribute() into a common header and use
> the same mechanism for both:
I agree lifting nfsd4_change_attribute(), if anything else, a consistent
place to fetch the i_version from. Am I correct in my understanding that
the XOR on the times will cancel out and result in just the i_version?
IMA is calling into inode_eq_iversion() to perform the comparison
between the cached value and inode.i_version.
>
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c35ea613c9f8..5a71845f579e 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -272,10 +272,14 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> * to an initial measurement/appraisal/audit, but was modified to
> * assume the file changed.
> */
> - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> + result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT);
> - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> - i_version = stat.change_cookie;
> + if (!result) {
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + i_version = stat.change_cookie;
> + else if (stat.result_mask & STATX_CTIME)
> + i_version = stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
> + }
> hash.hdr.algo = algo;
> hash.hdr.length = hash_digest_size[algo];
>
^ permalink raw reply
* Re: xfs/ima: Regression caching i_version
From: Jeff Layton @ 2025-12-11 20:55 UTC (permalink / raw)
To: Frederick Lawler
Cc: Christian Brauner, Darrick J. Wong, Josef Bacik, Carlos Maiolino,
linux-xfs, linux-kernel, linux-security-module, linux-integrity,
Roberto Sassu, kernel-team
In-Reply-To: <aTspr4_h9IU4EyrR@CMGLRV3>
On Thu, 2025-12-11 at 14:29 -0600, Frederick Lawler wrote:
> Hi Jeff,
>
> While testing 6.18, I think I found a regression with
> commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps") since 6.13
> where IMA is no longer able to properly cache i_version when we overlay
> tmpfs on top of XFS. Each measurement diff check in function
> process_measurement() reports that the i_version is
> always set to zero for iint->real_inode.version.
>
> The function ima_collect_measurement() is looking to extract the version
> from the cookie on next measurement to cache i_version.
>
> I'm unclear from the commit description what the right approach here is:
> update in IMA land by checking for time changes, or do
> something else such as adding the cookie back.
>
>
What we probably want to do is switch to using the ctime to manufacture
a change attribute when STATX_CHANGE_ATTRIBUTE is not set in the statx
reply.
IIRC, IMA doesn't need to persist these values across reboot, so
something like this (completely untested) might work, but it may be
better to lift nfsd4_change_attribute() into a common header and use
the same mechanism for both:
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c35ea613c9f8..5a71845f579e 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -272,10 +272,14 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
* to an initial measurement/appraisal/audit, but was modified to
* assume the file changed.
*/
- result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
+ result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE | STATX_CTIME,
AT_STATX_SYNC_AS_STAT);
- if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
- i_version = stat.change_cookie;
+ if (!result) {
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ i_version = stat.change_cookie;
+ else if (stat.result_mask & STATX_CTIME)
+ i_version = stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
+ }
hash.hdr.algo = algo;
hash.hdr.length = hash_digest_size[algo];
^ permalink raw reply related
* xfs/ima: Regression caching i_version
From: Frederick Lawler @ 2025-12-11 20:29 UTC (permalink / raw)
To: Jeff Layton
Cc: Christian Brauner, Darrick J. Wong, Josef Bacik, Carlos Maiolino,
linux-xfs, linux-kernel, linux-security-module, linux-integrity,
Roberto Sassu, kernel-team
Hi Jeff,
While testing 6.18, I think I found a regression with
commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps") since 6.13
where IMA is no longer able to properly cache i_version when we overlay
tmpfs on top of XFS. Each measurement diff check in function
process_measurement() reports that the i_version is
always set to zero for iint->real_inode.version.
The function ima_collect_measurement() is looking to extract the version
from the cookie on next measurement to cache i_version.
I'm unclear from the commit description what the right approach here is:
update in IMA land by checking for time changes, or do
something else such as adding the cookie back.
Thanks,
Fred
^ permalink raw reply
* Re: [PATCH v1 17/17] tpm/tpm_ftpm_tee: Make use of tee bus methods
From: Jarkko Sakkinen @ 2025-12-11 20:22 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jens Wiklander, Peter Huewe, Jason Gunthorpe, Sumit Garg, op-tee,
linux-integrity, linux-kernel
In-Reply-To: <eaf8216e8a6c3dabce5a82be5765d67c66318791.1765472125.git.u.kleine-koenig@baylibre.com>
On Thu, Dec 11, 2025 at 06:15:11PM +0100, Uwe Kleine-König wrote:
> The tee bus got dedicated callbacks for probe and remove.
> Make use of these. This fixes a runtime warning about the driver needing
> to be converted to the bus methods.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> drivers/char/tpm/tpm_ftpm_tee.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_ftpm_tee.c b/drivers/char/tpm/tpm_ftpm_tee.c
> index e5fbc70b0eca..20294d1953a3 100644
> --- a/drivers/char/tpm/tpm_ftpm_tee.c
> +++ b/drivers/char/tpm/tpm_ftpm_tee.c
> @@ -169,7 +169,7 @@ static int ftpm_tee_match(struct tee_ioctl_version_data *ver, const void *data)
> * Return:
> * On success, 0. On failure, -errno.
> */
> -static int ftpm_tee_probe(struct device *dev)
> +static int ftpm_tee_probe_generic(struct device *dev)
> {
> int rc;
> struct tpm_chip *chip;
> @@ -251,11 +251,18 @@ static int ftpm_tee_probe(struct device *dev)
> return rc;
> }
>
> +static int ftpm_tee_probe(struct tee_client_device *tcdev)
> +{
> + struct device *dev = &tcdev->dev;
> +
> + return ftpm_tee_probe_generic(dev);
> +}
> +
> static int ftpm_plat_tee_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
>
> - return ftpm_tee_probe(dev);
> + return ftpm_tee_probe_generic(dev);
> }
>
> /**
> @@ -265,7 +272,7 @@ static int ftpm_plat_tee_probe(struct platform_device *pdev)
> * Return:
> * 0 always.
> */
> -static int ftpm_tee_remove(struct device *dev)
> +static void ftpm_tee_remove_generic(struct device *dev)
> {
> struct ftpm_tee_private *pvt_data = dev_get_drvdata(dev);
>
> @@ -285,15 +292,20 @@ static int ftpm_tee_remove(struct device *dev)
> tee_client_close_context(pvt_data->ctx);
>
> /* memory allocated with devm_kzalloc() is freed automatically */
> +}
>
> - return 0;
> +static void ftpm_tee_remove(struct tee_client_device *tcdev)
> +{
> + struct device *dev = &tcdev->dev;
> +
> + ftpm_tee_remove_generic(dev);
> }
>
> static void ftpm_plat_tee_remove(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
>
> - ftpm_tee_remove(dev);
> + ftpm_tee_remove_generic(dev);
> }
>
> /**
> @@ -335,11 +347,11 @@ static const struct tee_client_device_id optee_ftpm_id_table[] = {
> MODULE_DEVICE_TABLE(tee, optee_ftpm_id_table);
>
> static struct tee_client_driver ftpm_tee_driver = {
> + .probe = ftpm_tee_probe,
> + .remove = ftpm_tee_remove,
> .id_table = optee_ftpm_id_table,
> .driver = {
> .name = "optee-ftpm",
> - .probe = ftpm_tee_probe,
> - .remove = ftpm_tee_remove,
> },
> };
>
> --
> 2.47.3
>
And also for this:
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v1 17/17] tpm/tpm_ftpm_tee: Make use of tee bus methods
From: Jarkko Sakkinen @ 2025-12-11 20:22 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jens Wiklander, Peter Huewe, Jason Gunthorpe, Sumit Garg, op-tee,
linux-integrity, linux-kernel
In-Reply-To: <eaf8216e8a6c3dabce5a82be5765d67c66318791.1765472125.git.u.kleine-koenig@baylibre.com>
On Thu, Dec 11, 2025 at 06:15:11PM +0100, Uwe Kleine-König wrote:
> The tee bus got dedicated callbacks for probe and remove.
> Make use of these. This fixes a runtime warning about the driver needing
> to be converted to the bus methods.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> ---
> drivers/char/tpm/tpm_ftpm_tee.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_ftpm_tee.c b/drivers/char/tpm/tpm_ftpm_tee.c
> index e5fbc70b0eca..20294d1953a3 100644
> --- a/drivers/char/tpm/tpm_ftpm_tee.c
> +++ b/drivers/char/tpm/tpm_ftpm_tee.c
> @@ -169,7 +169,7 @@ static int ftpm_tee_match(struct tee_ioctl_version_data *ver, const void *data)
> * Return:
> * On success, 0. On failure, -errno.
> */
> -static int ftpm_tee_probe(struct device *dev)
> +static int ftpm_tee_probe_generic(struct device *dev)
> {
> int rc;
> struct tpm_chip *chip;
> @@ -251,11 +251,18 @@ static int ftpm_tee_probe(struct device *dev)
> return rc;
> }
>
> +static int ftpm_tee_probe(struct tee_client_device *tcdev)
> +{
> + struct device *dev = &tcdev->dev;
> +
> + return ftpm_tee_probe_generic(dev);
> +}
> +
> static int ftpm_plat_tee_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
>
> - return ftpm_tee_probe(dev);
> + return ftpm_tee_probe_generic(dev);
> }
>
> /**
> @@ -265,7 +272,7 @@ static int ftpm_plat_tee_probe(struct platform_device *pdev)
> * Return:
> * 0 always.
> */
> -static int ftpm_tee_remove(struct device *dev)
> +static void ftpm_tee_remove_generic(struct device *dev)
> {
> struct ftpm_tee_private *pvt_data = dev_get_drvdata(dev);
>
> @@ -285,15 +292,20 @@ static int ftpm_tee_remove(struct device *dev)
> tee_client_close_context(pvt_data->ctx);
>
> /* memory allocated with devm_kzalloc() is freed automatically */
> +}
>
> - return 0;
> +static void ftpm_tee_remove(struct tee_client_device *tcdev)
> +{
> + struct device *dev = &tcdev->dev;
> +
> + ftpm_tee_remove_generic(dev);
> }
>
> static void ftpm_plat_tee_remove(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
>
> - ftpm_tee_remove(dev);
> + ftpm_tee_remove_generic(dev);
> }
>
> /**
> @@ -335,11 +347,11 @@ static const struct tee_client_device_id optee_ftpm_id_table[] = {
> MODULE_DEVICE_TABLE(tee, optee_ftpm_id_table);
>
> static struct tee_client_driver ftpm_tee_driver = {
> + .probe = ftpm_tee_probe,
> + .remove = ftpm_tee_remove,
> .id_table = optee_ftpm_id_table,
> .driver = {
> .name = "optee-ftpm",
> - .probe = ftpm_tee_probe,
> - .remove = ftpm_tee_remove,
> },
> };
>
> --
> 2.47.3
>
I checked trusted key patches and ftpm patches. I don't have anything
against merging them.
So for those four patches:
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [RFC][PATCH] ima: Add support for staging measurements for deletion
From: steven chen @ 2025-12-11 19:47 UTC (permalink / raw)
To: Roberto Sassu, Gregory Lumen
Cc: corbet, zohar, dmitry.kasatkin, eric.snowberg, paul, jmorris,
serge, linux-doc, linux-kernel, linux-integrity,
linux-security-module, nramas, Roberto Sassu, steven chen
In-Reply-To: <d7418d0afa696b8da67e4f25fd0dc1b9d6fd908f.camel@huaweicloud.com>
On 12/11/2025 1:56 AM, Roberto Sassu wrote:
> On Wed, 2025-12-10 at 11:12 -0800, Gregory Lumen wrote:
>> Roberto,
>>
>> The proposed approach appears to be workable. However, if our primary goal
>> here is to enable UM to free kernel memory consumed by the IMA log with an
>> absolute minimum of kernel functionality/change, then I would argue that
>> the proposed Stage-then-delete approach still represents unnecessary
>> complexity when compared to a trim-to-N solution. Specifically:
>>
>> - Any functional benefit offered through the introduction of a staged
>> measurement list could be equally achieved in UM with a trim-to-N solution
>> coupled with the proposed ima_measure_users counter for access locking.
> Ok, let's quantify the complexity of each solution. Let's assume that
> the IMA measurements list has M entries and you want to trim at N < M.
>
> Staging:
>
> 1st. trim at N
>
> (kernel)
> 1. list lock (write side) -> list replace (swap the heads) -> list unlock
> 2. read M -> file (file contains 0..M)
> 3. for each 0..M -> delete entry
>
> (user)
> 1. for each 0..N in file -> replay PCR
> 2. trim at N (keep N + 1..M)
>
>
> 2nd. trim at O
>
> (kernel)
> 1. list lock -> list replace (swap the heads) -> list unlock
> 2. read P -> file (file contains N + 1..P)
> 3. for each M + 1..P -> delete entry
>
> (user)
> 1. for each N + 1..O in file -> replay PCR
> 2. trim at O (keep O + 1..P)
>
>
>
> Trimming:
>
> 1st. trim at N
>
> (kernel)
> 1. list lock (read side) -> for each 0..M -> read in file (file now contains 0..M) -> list unlock
>
> (user)
> 1. for each 0..N -> replay PCR
> 2. discard N + 1..M
>
> (kernel)
>
> 1. list lock (write side) -> for each 0..N -> trim -> list unlock
>
>
> 2nd. trim at O
>
> (kernel)
> 1. list lock (read side) -> for each N + 1..P -> read in file (file now contains N + 1..P) -> list unlock
>
> (user)
> 1. for each N + 1..O -> replay PCR
> 2. discard O + 1..P
>
> (kernel)
>
> 1. list lock (write side) -> for each N + 1..O -> trim -> list unlock
>
>
> You can try to optimize it a bit by prematurely ending the reading
> before M and P, and by replaying the PCR on a partial buffer.
>
>
> But still:
>
> I just swap list heads in the hot path (still need to do the same for
> the hash table, postponed to later), and do the free later once there
> is no contention with new measurements.
>
> In your case you are taking the lock and walking the list two times,
> once as a reader and once as a writer, and discarding measurements in
> user space that you already have.
>
> I think your solution is more complex.
This is not the case, please check the released version 2 of trim N
entries patch as bellow:
[PATCH v2 0/1] Trim N entries of IMA event logs
<https://lore.kernel.org/linux-integrity/20251210235314.3341-1-chenste@linux.microsoft.com/T/#t>
The following are the steps for trim N solution:
User space reads list without lock
User space decides to trim N entries and send command to kernel
Kernel will lock the list use the same or less time as staged
solution use
All work done.
>
>> - There exists a potential UM measurement-loss race condition introduced
>> by the staging functionality that would not exist with a trim-to-N
>> approach. (Occurs if a kexec call occurs after a UM agent has staged
>> measurements for deletion, but has not completed copying them to
>> userspace). This could be avoided by persisting staged measurements across
>> kexec calls at the cost of making the proposed change larger.
> The solution is to coordinate the staging with kexec in user space.
>
>
> Roberto
^ permalink raw reply
* Re: [RFC][PATCH] ima: Add support for staging measurements for deletion
From: steven chen @ 2025-12-11 19:20 UTC (permalink / raw)
To: Roberto Sassu, corbet, zohar, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, nramas, Roberto Sassu
In-Reply-To: <d6ef2d61a2c31c0ae46741b6bd78f38bc02e6141.camel@huaweicloud.com>
On 12/11/2025 2:18 AM, Roberto Sassu wrote:
> On Wed, 2025-12-10 at 16:03 -0800, steven chen wrote:
>> On 12/9/2025 2:17 AM, Roberto Sassu wrote:
>>> From: Roberto Sassu <roberto.sassu@huawei.com>
>>>
>>> Introduce the ability of staging the entire of the IMA measurement list, or
>>> a portion, for deletion. Staging means moving the current content of the
>>> measurement list to a separate location, and allowing users to read and
>>> delete it. This causes the measurement list to be atomically truncated
>>> before new measurements can be added. Staging can be done only once at a
>>> time.
>>>
>>> User space is responsible to concatenate the staged IMA measurements list
>>> portions following the temporal order in which the operations were done,
>>> together with the current measurement list. Then, it can send the collected
>>> data to the remote verifiers.
>>>
>>> The benefit of this solution is the ability to free precious kernel memory,
>>> in exchange of delegating user space to reconstruct the full measurement
>>> list from the chunks. No trust needs to be given to user space, since the
>>> integrity of the measurement list is protected by the TPM.
>>>
>>> By default, staging the measurements list for deletion does not alter the
>>> hash table. When staging is done, IMA is still able to detect collisions on
>>> the staged and later deleted measurement entries, by keeping the entry
>>> digests (only template data are freed).
>>>
>>> However, since during the measurements list serialization only the SHA1
>>> digest is passed, and since there are no template data to recalculate the
>>> other digests from, the hash table is currently not populated with digests
>>> from staged/deleted entries after kexec().
>>>
>>> Introduce the new kernel option ima_flush_htable to decide whether or not
>>> the digests of staged measurement entries are flushed from the hash table.
>>>
>>> Then, introduce ascii_runtime_measurements_staged_<algo> and
>>> binary_runtime_measurement_staged_<algo> interfaces to stage/delete the
>>> measurements. Use 'echo A > <IMA interface>' and 'echo D > <IMA interface>'
>>> to respectively stage and delete the entire measurements list. Use
>>> 'echo N > <IMA interface>', with N between 1 and ULONG_MAX, to stage the
>>> selected portion of the measurements list.
>>>
>>> The ima_measure_users counter (protected by the ima_measure_lock mutex) has
>>> been introduced to protect access to the measurement list and the staged
>>> part. The open method of all the measurement interfaces has been extended
>>> to allow only one writer at a time or, in alternative, multiple readers.
>>> The write permission is used to stage/delete the measurements, the read
>>> permission to read them. Write requires also the CAP_SYS_ADMIN capability.
>> Hi Roberto,
>>
>> I released version 2 of trim N entries patch as bellow:
>>
>> [PATCH v2 0/1] Trim N entries of IMA event logs
>> <https://lore.kernel.org/linux-integrity/20251210235314.3341-1-chenste@linux.microsoft.com/T/#t>
>>
>> I adapted some of your idea and I think trim N has following advantages:
>> 1: less measurement list hold time than your current implementation
>> 2. operation much simple for user space
>> 3. less kernel code change
>> 4. no potential issue as Gregory mentioned.
> Please have a look at:
>
> https://marc.info/?l=linux-integrity&m=176545085325473&w=2
>
> and let me know if I'm missing something.
>
> Thanks
>
> Roberto
Hi Roberto,
what does this staging solution do that's not achieved by trim N entries
solution?
You did not address all my comments and your other idea make things more
complex.
Also, Trim N solution is simple and will bring following two good points:
easy for future IMA development
easy for code maintenance
Could you also add your comments on the trim N solution?
Thanks,
Steven
>
>> Thanks,
>>
>> Steven
>>
>>> Finally, introduce the _notrim version of the run-time measurements count
>>> and the binary measurements list size, to display them in the kexec-related
>>> critical data records.
>>>
>>> Note: This code derives from the Alt-IMA Huawei project, and is being
>>> released under the dual license model (GPL-2.0 OR MIT).
>>>
>>> Link: https://github.com/linux-integrity/linux/issues/1
>>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>>> ---
>>> .../admin-guide/kernel-parameters.txt | 4 +
>>> security/integrity/ima/ima.h | 10 +-
>>> security/integrity/ima/ima_fs.c | 222 +++++++++++++++++-
>>> security/integrity/ima/ima_kexec.c | 13 +-
>>> security/integrity/ima/ima_queue.c | 111 ++++++++-
>>> 5 files changed, 340 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>> index 6c42061ca20e..355d8930e3ac 100644
>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>> @@ -2215,6 +2215,10 @@
>>> Use the canonical format for the binary runtime
>>> measurements, instead of host native format.
>>>
>>> + ima_flush_htable [IMA]
>>> + Flush the measurement list hash table when staging all
>>> + or a part of it for deletion.
>>> +
>>> ima_hash= [IMA]
>>> Format: { md5 | sha1 | rmd160 | sha256 | sha384
>>> | sha512 | ... }
>>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>>> index e3d71d8d56e3..d7aa4a0f79b1 100644
>>> --- a/security/integrity/ima/ima.h
>>> +++ b/security/integrity/ima/ima.h
>>> @@ -117,6 +117,8 @@ struct ima_queue_entry {
>>> struct ima_template_entry *entry;
>>> };
>>> extern struct list_head ima_measurements; /* list of all measurements */
>>> +extern struct list_head ima_measurements_staged; /* list of staged meas. */
>>> +extern bool ima_measurements_staged_exist; /* If there are staged meas. */
>>>
>>> /* Some details preceding the binary serialized measurement list */
>>> struct ima_kexec_hdr {
>>> @@ -281,10 +283,12 @@ struct ima_template_desc *ima_template_desc_current(void);
>>> struct ima_template_desc *ima_template_desc_buf(void);
>>> struct ima_template_desc *lookup_template_desc(const char *name);
>>> bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
>>> +int ima_queue_stage(unsigned long req_value);
>>> +int ima_queue_delete_staged(void);
>>> int ima_restore_measurement_entry(struct ima_template_entry *entry);
>>> int ima_restore_measurement_list(loff_t bufsize, void *buf);
>>> int ima_measurements_show(struct seq_file *m, void *v);
>>> -unsigned long ima_get_binary_runtime_size(void);
>>> +unsigned long ima_get_binary_runtime_size(bool notrim);
>>> int ima_init_template(void);
>>> void ima_init_template_list(void);
>>> int __init ima_init_digests(void);
>>> @@ -298,11 +302,13 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
>>> extern spinlock_t ima_queue_lock;
>>>
>>> struct ima_h_table {
>>> - atomic_long_t len; /* number of stored measurements in the list */
>>> + atomic_long_t len; /* current num of stored meas. in the list */
>>> + atomic_long_t len_notrim; /* total num of stored meas. in the list */
>>> atomic_long_t violations;
>>> struct hlist_head queue[IMA_MEASURE_HTABLE_SIZE];
>>> };
>>> extern struct ima_h_table ima_htable;
>>> +extern struct mutex ima_extend_list_mutex;
>>>
>>> static inline unsigned int ima_hash_key(u8 *digest)
>>> {
>>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
>>> index 87045b09f120..321c98ae0e55 100644
>>> --- a/security/integrity/ima/ima_fs.c
>>> +++ b/security/integrity/ima/ima_fs.c
>>> @@ -24,7 +24,12 @@
>>>
>>> #include "ima.h"
>>>
>>> +/* Requests: ('A', [1, ULONG_MAX])\n (stage all/N) or D\n (delete staged) */
>>> +#define STAGED_REQ_LENGTH 21
>>> +
>>> static DEFINE_MUTEX(ima_write_mutex);
>>> +static DEFINE_MUTEX(ima_measure_lock);
>>> +static long ima_measure_users;
>>>
>>> bool ima_canonical_fmt;
>>> static int __init default_canonical_fmt_setup(char *str)
>>> @@ -74,14 +79,15 @@ static const struct file_operations ima_measurements_count_ops = {
>>> };
>>>
>>> /* returns pointer to hlist_node */
>>> -static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
>>> +static void *_ima_measurements_start(struct seq_file *m, loff_t *pos,
>>> + struct list_head *head)
>>> {
>>> loff_t l = *pos;
>>> struct ima_queue_entry *qe;
>>>
>>> /* we need a lock since pos could point beyond last element */
>>> rcu_read_lock();
>>> - list_for_each_entry_rcu(qe, &ima_measurements, later) {
>>> + list_for_each_entry_rcu(qe, head, later) {
>>> if (!l--) {
>>> rcu_read_unlock();
>>> return qe;
>>> @@ -91,7 +97,18 @@ static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
>>> return NULL;
>>> }
>>>
>>> -static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
>>> +static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
>>> +{
>>> + return _ima_measurements_start(m, pos, &ima_measurements);
>>> +}
>>> +
>>> +static void *ima_measurements_staged_start(struct seq_file *m, loff_t *pos)
>>> +{
>>> + return _ima_measurements_start(m, pos, &ima_measurements_staged);
>>> +}
>>> +
>>> +static void *_ima_measurements_next(struct seq_file *m, void *v, loff_t *pos,
>>> + struct list_head *head)
>>> {
>>> struct ima_queue_entry *qe = v;
>>>
>>> @@ -103,7 +120,18 @@ static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
>>> rcu_read_unlock();
>>> (*pos)++;
>>>
>>> - return (&qe->later == &ima_measurements) ? NULL : qe;
>>> + return (&qe->later == head) ? NULL : qe;
>>> +}
>>> +
>>> +static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
>>> +{
>>> + return _ima_measurements_next(m, v, pos, &ima_measurements);
>>> +}
>>> +
>>> +static void *ima_measurements_staged_next(struct seq_file *m, void *v,
>>> + loff_t *pos)
>>> +{
>>> + return _ima_measurements_next(m, v, pos, &ima_measurements_staged);
>>> }
>>>
>>> static void ima_measurements_stop(struct seq_file *m, void *v)
>>> @@ -202,16 +230,138 @@ static const struct seq_operations ima_measurments_seqops = {
>>> .show = ima_measurements_show
>>> };
>>>
>>> +static int _ima_measurements_open(struct inode *inode, struct file *file,
>>> + const struct seq_operations *seq_ops)
>>> +{
>>> + bool write = !!(file->f_mode & FMODE_WRITE);
>>> + int ret;
>>> +
>>> + if (write && !capable(CAP_SYS_ADMIN))
>>> + return -EPERM;
>>> +
>>> + mutex_lock(&ima_measure_lock);
>>> + if ((write && ima_measure_users != 0) ||
>>> + (!write && ima_measure_users < 0)) {
>>> + mutex_unlock(&ima_measure_lock);
>>> + return -EBUSY;
>>> + }
>>> +
>>> + ret = seq_open(file, seq_ops);
>>> + if (ret < 0) {
>>> + mutex_unlock(&ima_measure_lock);
>>> + return ret;
>>> + }
>>> +
>>> + if (write)
>>> + ima_measure_users--;
>>> + else
>>> + ima_measure_users++;
>>> +
>>> + mutex_unlock(&ima_measure_lock);
>>> + return ret;
>>> +}
>>> +
>>> static int ima_measurements_open(struct inode *inode, struct file *file)
>>> {
>>> - return seq_open(file, &ima_measurments_seqops);
>>> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
>>> +}
>>> +
>>> +static int ima_measurements_release(struct inode *inode, struct file *file)
>>> +{
>>> + bool write = !!(file->f_mode & FMODE_WRITE);
>>> + int ret;
>>> +
>>> + mutex_lock(&ima_measure_lock);
>>> + ret = seq_release(inode, file);
>>> + if (!ret) {
>>> + if (write)
>>> + ima_measure_users++;
>>> + else
>>> + ima_measure_users--;
>>> + }
>>> +
>>> + mutex_unlock(&ima_measure_lock);
>>> + return ret;
>>> }
>>>
>>> static const struct file_operations ima_measurements_ops = {
>>> .open = ima_measurements_open,
>>> .read = seq_read,
>>> .llseek = seq_lseek,
>>> - .release = seq_release,
>>> + .release = ima_measurements_release,
>>> +};
>>> +
>>> +static const struct seq_operations ima_measurments_staged_seqops = {
>>> + .start = ima_measurements_staged_start,
>>> + .next = ima_measurements_staged_next,
>>> + .stop = ima_measurements_stop,
>>> + .show = ima_measurements_show
>>> +};
>>> +
>>> +static int ima_measurements_staged_open(struct inode *inode, struct file *file)
>>> +{
>>> + return _ima_measurements_open(inode, file,
>>> + &ima_measurments_staged_seqops);
>>> +}
>>> +
>>> +static ssize_t ima_measurements_staged_read(struct file *file, char __user *buf,
>>> + size_t size, loff_t *ppos)
>>> +{
>>> + if (!ima_measurements_staged_exist)
>>> + return -ENOENT;
>>> +
>>> + return seq_read(file, buf, size, ppos);
>>> +}
>>> +
>>> +static ssize_t ima_measurements_staged_write(struct file *file,
>>> + const char __user *buf,
>>> + size_t datalen, loff_t *ppos)
>>> +{
>>> + char req[STAGED_REQ_LENGTH], *req_ptr = req;
>>> + unsigned long req_value;
>>> + int ret;
>>> +
>>> + if (*ppos > 0 || datalen < 2 || datalen > STAGED_REQ_LENGTH)
>>> + return -EINVAL;
>>> +
>>> + ret = copy_from_user(req, buf, datalen);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + if (strsep(&req_ptr, "\n") == NULL)
>>> + return -EINVAL;
>>> +
>>> + switch (req[0]) {
>>> + case 'A':
>>> + if (datalen != 2 || req[1] != '\0')
>>> + return -EINVAL;
>>> +
>>> + ret = ima_queue_stage(ULONG_MAX);
>>> + break;
>>> + case 'D':
>>> + if (datalen != 2 || req[1] != '\0')
>>> + return -EINVAL;
>>> +
>>> + ret = ima_queue_delete_staged();
>>> + break;
>>> + default:
>>> + ret = kstrtoul(req, 0, &req_value);
>>> + if (!ret)
>>> + ret = ima_queue_stage(req_value);
>>> + }
>>> +
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + return datalen;
>>> +}
>>> +
>>> +static const struct file_operations ima_measurements_staged_ops = {
>>> + .open = ima_measurements_staged_open,
>>> + .read = ima_measurements_staged_read,
>>> + .write = ima_measurements_staged_write,
>>> + .llseek = seq_lseek,
>>> + .release = ima_measurements_release,
>>> };
>>>
>>> void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
>>> @@ -279,14 +429,37 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
>>>
>>> static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
>>> {
>>> - return seq_open(file, &ima_ascii_measurements_seqops);
>>> + return _ima_measurements_open(inode, file,
>>> + &ima_ascii_measurements_seqops);
>>> }
>>>
>>> static const struct file_operations ima_ascii_measurements_ops = {
>>> .open = ima_ascii_measurements_open,
>>> .read = seq_read,
>>> .llseek = seq_lseek,
>>> - .release = seq_release,
>>> + .release = ima_measurements_release,
>>> +};
>>> +
>>> +static const struct seq_operations ima_ascii_measurements_staged_seqops = {
>>> + .start = ima_measurements_staged_start,
>>> + .next = ima_measurements_staged_next,
>>> + .stop = ima_measurements_stop,
>>> + .show = ima_ascii_measurements_show
>>> +};
>>> +
>>> +static int ima_ascii_measurements_staged_open(struct inode *inode,
>>> + struct file *file)
>>> +{
>>> + return _ima_measurements_open(inode, file,
>>> + &ima_ascii_measurements_staged_seqops);
>>> +}
>>> +
>>> +static const struct file_operations ima_ascii_measurements_staged_ops = {
>>> + .open = ima_ascii_measurements_staged_open,
>>> + .read = ima_measurements_staged_read,
>>> + .write = ima_measurements_staged_write,
>>> + .llseek = seq_lseek,
>>> + .release = ima_measurements_release,
>>> };
>>>
>>> static ssize_t ima_read_policy(char *path)
>>> @@ -419,6 +592,25 @@ static int __init create_securityfs_measurement_lists(void)
>>> &ima_measurements_ops);
>>> if (IS_ERR(dentry))
>>> return PTR_ERR(dentry);
>>> +
>>> + sprintf(file_name, "ascii_runtime_measurements_staged_%s",
>>> + hash_algo_name[algo]);
>>> + dentry = securityfs_create_file(file_name,
>>> + S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
>>> + ima_dir, (void *)(uintptr_t)i,
>>> + &ima_ascii_measurements_staged_ops);
>>> + if (IS_ERR(dentry))
>>> + return PTR_ERR(dentry);
>>> +
>>> + sprintf(file_name, "binary_runtime_measurements_staged_%s",
>>> + hash_algo_name[algo]);
>>> + dentry = securityfs_create_file(file_name,
>>> + S_IRUSR | S_IRGRP |
>>> + S_IWUSR | S_IWGRP,
>>> + ima_dir, (void *)(uintptr_t)i,
>>> + &ima_measurements_staged_ops);
>>> + if (IS_ERR(dentry))
>>> + return PTR_ERR(dentry);
>>> }
>>>
>>> return 0;
>>> @@ -528,6 +720,20 @@ int __init ima_fs_init(void)
>>> goto out;
>>> }
>>>
>>> + dentry = securityfs_create_symlink("binary_runtime_measurements_staged",
>>> + ima_dir, "binary_runtime_measurements_staged_sha1", NULL);
>>> + if (IS_ERR(dentry)) {
>>> + ret = PTR_ERR(dentry);
>>> + goto out;
>>> + }
>>> +
>>> + dentry = securityfs_create_symlink("ascii_runtime_measurements_staged",
>>> + ima_dir, "ascii_runtime_measurements_staged_sha1", NULL);
>>> + if (IS_ERR(dentry)) {
>>> + ret = PTR_ERR(dentry);
>>> + goto out;
>>> + }
>>> +
>>> dentry = securityfs_create_file("runtime_measurements_count",
>>> S_IRUSR | S_IRGRP, ima_dir, NULL,
>>> &ima_measurements_count_ops);
>>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
>>> index 7362f68f2d8b..23a20300da7b 100644
>>> --- a/security/integrity/ima/ima_kexec.c
>>> +++ b/security/integrity/ima/ima_kexec.c
>>> @@ -40,8 +40,8 @@ void ima_measure_kexec_event(const char *event_name)
>>> long len;
>>> int n;
>>>
>>> - buf_size = ima_get_binary_runtime_size();
>>> - len = atomic_long_read(&ima_htable.len);
>>> + buf_size = ima_get_binary_runtime_size(true);
>>> + len = atomic_long_read(&ima_htable.len_notrim);
>>>
>>> n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
>>> "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
>>> @@ -93,8 +93,10 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
>>>
>>> memset(&khdr, 0, sizeof(khdr));
>>> khdr.version = 1;
>>> - /* This is an append-only list, no need to hold the RCU read lock */
>>> - list_for_each_entry_rcu(qe, &ima_measurements, later, true) {
>>> +
>>> + /* It can race with ima_queue_stage(). */
>>> + mutex_lock(&ima_extend_list_mutex);
>>> + list_for_each_entry(qe, &ima_measurements, later) {
>>> if (ima_kexec_file.count < ima_kexec_file.size) {
>>> khdr.count++;
>>> ima_measurements_show(&ima_kexec_file, qe);
>>> @@ -103,6 +105,7 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
>>> break;
>>> }
>>> }
>>> + mutex_unlock(&ima_extend_list_mutex);
>>>
>>> /*
>>> * fill in reserved space with some buffer details
>>> @@ -157,7 +160,7 @@ void ima_add_kexec_buffer(struct kimage *image)
>>> else
>>> extra_memory = CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB * 1024;
>>>
>>> - binary_runtime_size = ima_get_binary_runtime_size() + extra_memory;
>>> + binary_runtime_size = ima_get_binary_runtime_size(false) + extra_memory;
>>>
>>> if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
>>> kexec_segment_size = ULONG_MAX;
>>> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
>>> index 590637e81ad1..868f216ac343 100644
>>> --- a/security/integrity/ima/ima_queue.c
>>> +++ b/security/integrity/ima/ima_queue.c
>>> @@ -22,19 +22,32 @@
>>>
>>> #define AUDIT_CAUSE_LEN_MAX 32
>>>
>>> +bool ima_flush_htable;
>>> +static int __init ima_flush_htable_setup(char *str)
>>> +{
>>> + ima_flush_htable = true;
>>> + return 1;
>>> +}
>>> +__setup("ima_flush_htable", ima_flush_htable_setup);
>>> +
>>> /* pre-allocated array of tpm_digest structures to extend a PCR */
>>> static struct tpm_digest *digests;
>>>
>>> LIST_HEAD(ima_measurements); /* list of all measurements */
>>> +LIST_HEAD(ima_measurements_staged); /* list of staged measurements */
>>> +bool ima_measurements_staged_exist; /* If there are staged measurements */
>>> #ifdef CONFIG_IMA_KEXEC
>>> static unsigned long binary_runtime_size;
>>> +static unsigned long binary_runtime_size_notrim;
>>> #else
>>> static unsigned long binary_runtime_size = ULONG_MAX;
>>> +static unsigned long binary_runtime_size_notrim = ULONG_MAX;
>>> #endif
>>>
>>> /* key: inode (before secure-hashing a file) */
>>> struct ima_h_table ima_htable = {
>>> .len = ATOMIC_LONG_INIT(0),
>>> + .len_notrim = ATOMIC_LONG_INIT(0),
>>> .violations = ATOMIC_LONG_INIT(0),
>>> .queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
>>> };
>>> @@ -43,7 +56,7 @@ struct ima_h_table ima_htable = {
>>> * and extending the TPM PCR aggregate. Since tpm_extend can take
>>> * long (and the tpm driver uses a mutex), we can't use the spinlock.
>>> */
>>> -static DEFINE_MUTEX(ima_extend_list_mutex);
>>> +DEFINE_MUTEX(ima_extend_list_mutex);
>>>
>>> /*
>>> * Used internally by the kernel to suspend measurements.
>>> @@ -114,15 +127,19 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
>>> list_add_tail_rcu(&qe->later, &ima_measurements);
>>>
>>> atomic_long_inc(&ima_htable.len);
>>> + atomic_long_inc(&ima_htable.len_notrim);
>>> if (update_htable) {
>>> key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
>>> hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
>>> }
>>>
>>> - if (binary_runtime_size != ULONG_MAX) {
>>> + if (binary_runtime_size_notrim != ULONG_MAX) {
>>> int size;
>>>
>>> size = get_binary_runtime_size(entry);
>>> + binary_runtime_size_notrim =
>>> + (binary_runtime_size_notrim < ULONG_MAX - size) ?
>>> + binary_runtime_size_notrim + size : ULONG_MAX;
>>> binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
>>> binary_runtime_size + size : ULONG_MAX;
>>> }
>>> @@ -134,12 +151,18 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
>>> * entire binary_runtime_measurement list, including the ima_kexec_hdr
>>> * structure.
>>> */
>>> -unsigned long ima_get_binary_runtime_size(void)
>>> +unsigned long ima_get_binary_runtime_size(bool notrim)
>>> {
>>> - if (binary_runtime_size >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
>>> + unsigned long *val;
>>> +
>>> + mutex_lock(&ima_extend_list_mutex);
>>> + val = (notrim) ? &binary_runtime_size_notrim : &binary_runtime_size;
>>> + mutex_unlock(&ima_extend_list_mutex);
>>> +
>>> + if (*val >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
>>> return ULONG_MAX;
>>> else
>>> - return binary_runtime_size + sizeof(struct ima_kexec_hdr);
>>> + return *val + sizeof(struct ima_kexec_hdr);
>>> }
>>>
>>> static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
>>> @@ -220,6 +243,84 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>>> return result;
>>> }
>>>
>>> +int ima_queue_stage(unsigned long req_value)
>>> +{
>>> + unsigned long req_value_copy = req_value, to_remove = 0;
>>> + struct ima_queue_entry *qe;
>>> +
>>> + if (ima_measurements_staged_exist)
>>> + return -EEXIST;
>>> +
>>> + mutex_lock(&ima_extend_list_mutex);
>>> + if (list_empty(&ima_measurements)) {
>>> + mutex_unlock(&ima_extend_list_mutex);
>>> + return -ENOENT;
>>> + }
>>> +
>>> + if (req_value == ULONG_MAX) {
>>> + list_replace(&ima_measurements, &ima_measurements_staged);
>>> + INIT_LIST_HEAD(&ima_measurements);
>>> + atomic_long_set(&ima_htable.len, 0);
>>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>>> + binary_runtime_size = 0;
>>> + } else {
>>> + list_for_each_entry(qe, &ima_measurements, later) {
>>> + to_remove += get_binary_runtime_size(qe->entry);
>>> + if (--req_value_copy == 0)
>>> + break;
>>> + }
>>> +
>>> + if (req_value_copy > 0) {
>>> + mutex_unlock(&ima_extend_list_mutex);
>>> + return -ENOENT;
>>> + }
>>> +
>>> + __list_cut_position(&ima_measurements_staged, &ima_measurements,
>>> + &qe->later);
>>> + atomic_long_sub(req_value, &ima_htable.len);
>>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>>> + binary_runtime_size -= to_remove;
>>> + }
>>> +
>>> + if (ima_flush_htable) {
>>> + list_for_each_entry(qe, &ima_measurements_staged, later)
>>> + /* It can race with ima_lookup_digest_entry(). */
>>> + hlist_del_rcu(&qe->hnext);
>>> + }
>>> +
>>> + mutex_unlock(&ima_extend_list_mutex);
>>> + ima_measurements_staged_exist = true;
>>> + return 0;
>>> +}
>>> +
>>> +int ima_queue_delete_staged(void)
>>> +{
>>> + struct ima_queue_entry *qe, *qe_tmp;
>>> + unsigned int i;
>>> +
>>> + if (!ima_measurements_staged_exist)
>>> + return -ENOENT;
>>> +
>>> + list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_staged, later) {
>>> + for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
>>> + kfree(qe->entry->template_data[i].data);
>>> + qe->entry->template_data[i].data = NULL;
>>> + qe->entry->template_data[i].len = 0;
>>> + }
>>> +
>>> + list_del(&qe->later);
>>> +
>>> + if (ima_flush_htable) {
>>> + kfree(qe->entry->digests);
>>> + kfree(qe->entry);
>>> + kfree(qe);
>>> + }
>>> + }
>>> +
>>> + ima_measurements_staged_exist = false;
>>> + return 0;
>>> +}
>>> +
>>> int ima_restore_measurement_entry(struct ima_template_entry *entry)
>>> {
>>> int result = 0;
^ permalink raw reply
* Re: [PATCH V2 1/1] IMA event log trimming
From: steven chen @ 2025-12-11 18:41 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve,
gregorylumen, nramas, sushring, linux-doc, steven chen
In-Reply-To: <20251210235314.3341-2-chenste@linux.microsoft.com>
On 12/10/2025 3:53 PM, steven chen wrote:
> This patch is for trimming N entries of the IMA event logs. It will also
> cleaning the hash table if ima_flush_htable is set.
>
> It provides a userspace interface ima_trim_log that can be used to input
> number N to let kernel to trim N entries of IMA event logs. When read
> this interface, it returns number of entries trimmed last time.
>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> ---
> .../admin-guide/kernel-parameters.txt | 4 +
> security/integrity/ima/ima.h | 2 +
> security/integrity/ima/ima_fs.c | 175 +++++++++++++++++-
> security/integrity/ima/ima_queue.c | 64 +++++++
> 4 files changed, 241 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index e92c0056e4e0..cd1a1d0bf0e2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2197,6 +2197,10 @@
> Use the canonical format for the binary runtime
> measurements, instead of host native format.
>
> + ima_flush_htable [IMA]
> + Flush the measurement list hash table when trim all
> + or a part of it for deletion.
> +
> ima_hash= [IMA]
> Format: { md5 | sha1 | rmd160 | sha256 | sha384
> | sha512 | ... }
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index e3d71d8d56e3..ab0e30ee25ea 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -246,8 +246,10 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
>
> #ifdef CONFIG_IMA_KEXEC
> void ima_measure_kexec_event(const char *event_name);
> +long ima_purge_event_log(long number_logs);
> #else
> static inline void ima_measure_kexec_event(const char *event_name) {}
> +static inline long ima_purge_event_log(long number_logs) { return 0; }
> #endif
>
> /*
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 87045b09f120..410f7d03c43f 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -21,6 +21,9 @@
> #include <linux/rcupdate.h>
> #include <linux/parser.h>
> #include <linux/vmalloc.h>
> +#include <linux/ktime.h>
> +#include <linux/timekeeping.h>
> +#include <linux/ima.h>
>
> #include "ima.h"
>
> @@ -38,6 +41,14 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
>
> static int valid_policy = 1;
>
> +#define IMA_LOG_TRIM_REQ_LENGTH 11
> +#define IMA_LOG_TRIM_EVENT_LEN 256
> +
> +static long trimcount;
> +/* mutex protects atomicity of trimming measurement list requests */
> +static DEFINE_MUTEX(ima_measure_lock);
> +static long ima_measure_users;
> +
> static ssize_t ima_show_htable_value(char __user *buf, size_t count,
> loff_t *ppos, atomic_long_t *val)
> {
> @@ -202,16 +213,65 @@ static const struct seq_operations ima_measurments_seqops = {
> .show = ima_measurements_show
> };
>
> +static int _ima_measurements_open(struct inode *inode, struct file *file,
> + const struct seq_operations *seq_ops)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> + int ret;
> +
> + if (write && !capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + mutex_lock(&ima_measure_lock);
> + if ((write && ima_measure_users != 0) ||
> + (!write && ima_measure_users < 0)) {
> + mutex_unlock(&ima_measure_lock);
> + return -EBUSY;
> + }
> +
> + ret = seq_open(file, seq_ops);
> + if (ret < 0) {
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> + }
> +
> + if (write)
> + ima_measure_users--;
> + else
> + ima_measure_users++;
> +
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> +}
> +
> static int ima_measurements_open(struct inode *inode, struct file *file)
> {
> - return seq_open(file, &ima_measurments_seqops);
> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
> +}
> +
> +static int ima_measurements_release(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> + int ret;
> +
> + mutex_lock(&ima_measure_lock);
> + ret = seq_release(inode, file);
> + if (!ret) {
> + if (write)
> + ima_measure_users++;
> + else
> + ima_measure_users--;
> + }
> +
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> }
>
> static const struct file_operations ima_measurements_ops = {
> .open = ima_measurements_open,
> .read = seq_read,
> .llseek = seq_lseek,
> - .release = seq_release,
> + .release = ima_measurements_release,
> };
>
> void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
> @@ -279,14 +339,111 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
>
> static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
> {
> - return seq_open(file, &ima_ascii_measurements_seqops);
> + return _ima_measurements_open(inode, file, &ima_ascii_measurements_seqops);
> }
>
> static const struct file_operations ima_ascii_measurements_ops = {
> .open = ima_ascii_measurements_open,
> .read = seq_read,
> .llseek = seq_lseek,
> - .release = seq_release,
> + .release = ima_measurements_release,
> +};
> +
> +static void ima_measure_trim_event(const long number_logs)
> +{
> + char ima_log_trim_event[IMA_LOG_TRIM_EVENT_LEN];
> + struct timespec64 ts;
> + u64 time_ns;
> + int n;
> +
> + ktime_get_real_ts64(&ts);
> + time_ns = (u64)ts.tv_sec * 1000000000ULL + ts.tv_nsec;
> + n = scnprintf(ima_log_trim_event, IMA_LOG_TRIM_EVENT_LEN,
> + "time=%llu; log trim this time=%lu;",
> + time_ns, number_logs);
> +
> + ima_measure_critical_data("ima_log_trim", "trim ima event logs", ima_log_trim_event, n, false, NULL, 0);
> +}
> +
> +static int ima_log_trim_open(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> +
> + if (!write && capable(CAP_SYS_ADMIN))
> + return 0;
> + else if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
> +}
> +
> +static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
> +{
> + char tmpbuf[IMA_LOG_TRIM_REQ_LENGTH]; /* greater than largest 'long' string value */
> + ssize_t len;
> +
> + len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
> + return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
> +}
> +
> +static ssize_t ima_log_trim_write(struct file *file,
> + const char __user *buf, size_t datalen, loff_t *ppos)
> +{
> + unsigned char req[IMA_LOG_TRIM_REQ_LENGTH];
> + long count, n;
> + int ret;
> +
> + if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_LENGTH || datalen < 2) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + n = (int)datalen;
> +
> + ret = copy_from_user(req, buf, datalen);
> + if (ret < 0)
> + goto out;
> +
> + count = 0;
> + for (int i = 0; i < n; ++i) {
> + if (req[i] < '0' || req[i] > '9') {
> + ret = -EINVAL;
> + goto out;
> + }
> + count = count * 10 + req[i] - '0';
> + }
> + ret = ima_purge_event_log(count);
> +
> + if (ret < 0)
> + goto out;
> +
> + trimcount = ret;
> +
> + if (trimcount > 0)
> + ima_measure_trim_event(trimcount);
> +
> + ret = datalen;
> +out:
> + return ret;
> +}
> +
> +static int ima_log_trim_release(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> + if (!write && capable(CAP_SYS_ADMIN))
> + return 0;
> + else if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + return ima_measurements_release(inode, file);
> +}
> +
> +static const struct file_operations ima_log_trim_ops = {
> + .open = ima_log_trim_open,
> + .read = ima_log_trim_read,
> + .write = ima_log_trim_write,
> + .llseek = generic_file_llseek,
> + .release = ima_log_trim_release
> };
>
> static ssize_t ima_read_policy(char *path)
> @@ -528,6 +685,16 @@ int __init ima_fs_init(void)
> goto out;
> }
>
> + dentry = securityfs_create_file("ima_trim_log",
> + S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
> + ima_dir, NULL, &ima_log_trim_ops);
> + if (IS_ERR(dentry)) {
> + ret = PTR_ERR(dentry);
> + goto out;
> + }
> +
> + trimcount = 0;
> +
> dentry = securityfs_create_file("runtime_measurements_count",
> S_IRUSR | S_IRGRP, ima_dir, NULL,
> &ima_measurements_count_ops);
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 590637e81ad1..77ab52469727 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -22,6 +22,14 @@
>
> #define AUDIT_CAUSE_LEN_MAX 32
>
> +bool ima_flush_htable;
> +static int __init ima_flush_htable_setup(char *str)
> +{
> + ima_flush_htable = true;
> + return 1;
> +}
> +__setup("ima_flush_htable", ima_flush_htable_setup);
> +
> /* pre-allocated array of tpm_digest structures to extend a PCR */
> static struct tpm_digest *digests;
>
> @@ -220,6 +228,62 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> return result;
> }
>
> +/* Delete the IMA event logs */
> +long ima_purge_event_log(long number_logs)
> +{
> + struct ima_queue_entry *qe, *qe_tmp;
> + LIST_HEAD(ima_measurements_staged);
> + unsigned int i;
> + long cur = number_logs;
> +
> + if (number_logs <= 0)
> + return number_logs;
> +
> + mutex_lock(&ima_extend_list_mutex);
> +
> +
> + list_for_each_entry(qe, &ima_measurements, later) {
> + if (--number_logs == 0)
> + break;
> + }
> +
> + if (number_logs > 0) {
> + mutex_unlock(&ima_extend_list_mutex);
> + return -ENOENT;
> + }
> +
> + __list_cut_position(&ima_measurements_staged, &ima_measurements,
> + &qe->later);
> + atomic_long_sub(cur, &ima_htable.len);
> +
> + if (!IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE) && ima_flush_htable) {
> + list_for_each_entry(qe, &ima_measurements_staged, later)
> + /* It can race with ima_lookup_digest_entry(). */
> + hlist_del_rcu(&qe->hnext);
> + }
If the h table can be staged during the locking period and deleted after
unlocking, the time
the list is held will be reduced.
I will work on this, and any suggestions are greatly appreciated.
Thanks,
Steven
> +
> + mutex_unlock(&ima_extend_list_mutex);
> +
> +
> + list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_staged, later) {
> + for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
> + kfree(qe->entry->template_data[i].data);
> + qe->entry->template_data[i].data = NULL;
> + qe->entry->template_data[i].len = 0;
> + }
> +
> + list_del(&qe->later);
> +
> + if (ima_flush_htable) {
> + kfree(qe->entry->digests);
> + kfree(qe->entry);
> + kfree(qe);
> + }
> + }
> +
> + return cur;
> +}
> +
> int ima_restore_measurement_entry(struct ima_template_entry *entry)
> {
> int result = 0;
^ permalink raw reply
* Re: [PATCH V2 1/1] IMA event log trimming
From: steven chen @ 2025-12-11 18:20 UTC (permalink / raw)
To: Roberto Sassu, linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve,
gregorylumen, nramas, sushring, linux-doc, steven chen
In-Reply-To: <561786c2505cae94913809ac0cf3d916e1a50a68.camel@huaweicloud.com>
On 12/11/2025 2:14 AM, Roberto Sassu wrote:
> On Wed, 2025-12-10 at 15:53 -0800, steven chen wrote:
>> This patch is for trimming N entries of the IMA event logs. It will also
>> cleaning the hash table if ima_flush_htable is set.
>>
>> It provides a userspace interface ima_trim_log that can be used to input
>> number N to let kernel to trim N entries of IMA event logs. When read
>> this interface, it returns number of entries trimmed last time.
> I was trying to find a common solution for both approaches, and instead
> you took part of my code and removed the part that you didn't like.
> This is not nice.
Hi Roberto,
The most important is one step trimming N entries and the N come from
user space. Everything
else are helping this target. We need to bring the best to the open
source community and
I acknowledged the source.
Thanks,
Steven
> Let's do the following. Please review my patch, and check if it makes
> you achieve your goal. Then, please send follow-up patches with the
> additional functionality you need (e.g. the new critical data
> measurement entry).
>
> Thanks
>
> Roberto
>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>> ---
>> .../admin-guide/kernel-parameters.txt | 4 +
>> security/integrity/ima/ima.h | 2 +
>> security/integrity/ima/ima_fs.c | 175 +++++++++++++++++-
>> security/integrity/ima/ima_queue.c | 64 +++++++
>> 4 files changed, 241 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index e92c0056e4e0..cd1a1d0bf0e2 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -2197,6 +2197,10 @@
>> Use the canonical format for the binary runtime
>> measurements, instead of host native format.
>>
>> + ima_flush_htable [IMA]
>> + Flush the measurement list hash table when trim all
>> + or a part of it for deletion.
>> +
>> ima_hash= [IMA]
>> Format: { md5 | sha1 | rmd160 | sha256 | sha384
>> | sha512 | ... }
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index e3d71d8d56e3..ab0e30ee25ea 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -246,8 +246,10 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
>>
>> #ifdef CONFIG_IMA_KEXEC
>> void ima_measure_kexec_event(const char *event_name);
>> +long ima_purge_event_log(long number_logs);
>> #else
>> static inline void ima_measure_kexec_event(const char *event_name) {}
>> +static inline long ima_purge_event_log(long number_logs) { return 0; }
>> #endif
>>
>> /*
>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
>> index 87045b09f120..410f7d03c43f 100644
>> --- a/security/integrity/ima/ima_fs.c
>> +++ b/security/integrity/ima/ima_fs.c
>> @@ -21,6 +21,9 @@
>> #include <linux/rcupdate.h>
>> #include <linux/parser.h>
>> #include <linux/vmalloc.h>
>> +#include <linux/ktime.h>
>> +#include <linux/timekeeping.h>
>> +#include <linux/ima.h>
>>
>> #include "ima.h"
>>
>> @@ -38,6 +41,14 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
>>
>> static int valid_policy = 1;
>>
>> +#define IMA_LOG_TRIM_REQ_LENGTH 11
>> +#define IMA_LOG_TRIM_EVENT_LEN 256
>> +
>> +static long trimcount;
>> +/* mutex protects atomicity of trimming measurement list requests */
>> +static DEFINE_MUTEX(ima_measure_lock);
>> +static long ima_measure_users;
>> +
>> static ssize_t ima_show_htable_value(char __user *buf, size_t count,
>> loff_t *ppos, atomic_long_t *val)
>> {
>> @@ -202,16 +213,65 @@ static const struct seq_operations ima_measurments_seqops = {
>> .show = ima_measurements_show
>> };
>>
>> +static int _ima_measurements_open(struct inode *inode, struct file *file,
>> + const struct seq_operations *seq_ops)
>> +{
>> + bool write = !!(file->f_mode & FMODE_WRITE);
>> + int ret;
>> +
>> + if (write && !capable(CAP_SYS_ADMIN))
>> + return -EPERM;
>> +
>> + mutex_lock(&ima_measure_lock);
>> + if ((write && ima_measure_users != 0) ||
>> + (!write && ima_measure_users < 0)) {
>> + mutex_unlock(&ima_measure_lock);
>> + return -EBUSY;
>> + }
>> +
>> + ret = seq_open(file, seq_ops);
>> + if (ret < 0) {
>> + mutex_unlock(&ima_measure_lock);
>> + return ret;
>> + }
>> +
>> + if (write)
>> + ima_measure_users--;
>> + else
>> + ima_measure_users++;
>> +
>> + mutex_unlock(&ima_measure_lock);
>> + return ret;
>> +}
>> +
>> static int ima_measurements_open(struct inode *inode, struct file *file)
>> {
>> - return seq_open(file, &ima_measurments_seqops);
>> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
>> +}
>> +
>> +static int ima_measurements_release(struct inode *inode, struct file *file)
>> +{
>> + bool write = !!(file->f_mode & FMODE_WRITE);
>> + int ret;
>> +
>> + mutex_lock(&ima_measure_lock);
>> + ret = seq_release(inode, file);
>> + if (!ret) {
>> + if (write)
>> + ima_measure_users++;
>> + else
>> + ima_measure_users--;
>> + }
>> +
>> + mutex_unlock(&ima_measure_lock);
>> + return ret;
>> }
>>
>> static const struct file_operations ima_measurements_ops = {
>> .open = ima_measurements_open,
>> .read = seq_read,
>> .llseek = seq_lseek,
>> - .release = seq_release,
>> + .release = ima_measurements_release,
>> };
>>
>> void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
>> @@ -279,14 +339,111 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
>>
>> static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
>> {
>> - return seq_open(file, &ima_ascii_measurements_seqops);
>> + return _ima_measurements_open(inode, file, &ima_ascii_measurements_seqops);
>> }
>>
>> static const struct file_operations ima_ascii_measurements_ops = {
>> .open = ima_ascii_measurements_open,
>> .read = seq_read,
>> .llseek = seq_lseek,
>> - .release = seq_release,
>> + .release = ima_measurements_release,
>> +};
>> +
>> +static void ima_measure_trim_event(const long number_logs)
>> +{
>> + char ima_log_trim_event[IMA_LOG_TRIM_EVENT_LEN];
>> + struct timespec64 ts;
>> + u64 time_ns;
>> + int n;
>> +
>> + ktime_get_real_ts64(&ts);
>> + time_ns = (u64)ts.tv_sec * 1000000000ULL + ts.tv_nsec;
>> + n = scnprintf(ima_log_trim_event, IMA_LOG_TRIM_EVENT_LEN,
>> + "time=%llu; log trim this time=%lu;",
>> + time_ns, number_logs);
>> +
>> + ima_measure_critical_data("ima_log_trim", "trim ima event logs", ima_log_trim_event, n, false, NULL, 0);
>> +}
>> +
>> +static int ima_log_trim_open(struct inode *inode, struct file *file)
>> +{
>> + bool write = !!(file->f_mode & FMODE_WRITE);
>> +
>> + if (!write && capable(CAP_SYS_ADMIN))
>> + return 0;
>> + else if (!capable(CAP_SYS_ADMIN))
>> + return -EPERM;
>> +
>> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
>> +}
>> +
>> +static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
>> +{
>> + char tmpbuf[IMA_LOG_TRIM_REQ_LENGTH]; /* greater than largest 'long' string value */
>> + ssize_t len;
>> +
>> + len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
>> + return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
>> +}
>> +
>> +static ssize_t ima_log_trim_write(struct file *file,
>> + const char __user *buf, size_t datalen, loff_t *ppos)
>> +{
>> + unsigned char req[IMA_LOG_TRIM_REQ_LENGTH];
>> + long count, n;
>> + int ret;
>> +
>> + if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_LENGTH || datalen < 2) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + n = (int)datalen;
>> +
>> + ret = copy_from_user(req, buf, datalen);
>> + if (ret < 0)
>> + goto out;
>> +
>> + count = 0;
>> + for (int i = 0; i < n; ++i) {
>> + if (req[i] < '0' || req[i] > '9') {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + count = count * 10 + req[i] - '0';
>> + }
>> + ret = ima_purge_event_log(count);
>> +
>> + if (ret < 0)
>> + goto out;
>> +
>> + trimcount = ret;
>> +
>> + if (trimcount > 0)
>> + ima_measure_trim_event(trimcount);
>> +
>> + ret = datalen;
>> +out:
>> + return ret;
>> +}
>> +
>> +static int ima_log_trim_release(struct inode *inode, struct file *file)
>> +{
>> + bool write = !!(file->f_mode & FMODE_WRITE);
>> + if (!write && capable(CAP_SYS_ADMIN))
>> + return 0;
>> + else if (!capable(CAP_SYS_ADMIN))
>> + return -EPERM;
>> +
>> + return ima_measurements_release(inode, file);
>> +}
>> +
>> +static const struct file_operations ima_log_trim_ops = {
>> + .open = ima_log_trim_open,
>> + .read = ima_log_trim_read,
>> + .write = ima_log_trim_write,
>> + .llseek = generic_file_llseek,
>> + .release = ima_log_trim_release
>> };
>>
>> static ssize_t ima_read_policy(char *path)
>> @@ -528,6 +685,16 @@ int __init ima_fs_init(void)
>> goto out;
>> }
>>
>> + dentry = securityfs_create_file("ima_trim_log",
>> + S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
>> + ima_dir, NULL, &ima_log_trim_ops);
>> + if (IS_ERR(dentry)) {
>> + ret = PTR_ERR(dentry);
>> + goto out;
>> + }
>> +
>> + trimcount = 0;
>> +
>> dentry = securityfs_create_file("runtime_measurements_count",
>> S_IRUSR | S_IRGRP, ima_dir, NULL,
>> &ima_measurements_count_ops);
>> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
>> index 590637e81ad1..77ab52469727 100644
>> --- a/security/integrity/ima/ima_queue.c
>> +++ b/security/integrity/ima/ima_queue.c
>> @@ -22,6 +22,14 @@
>>
>> #define AUDIT_CAUSE_LEN_MAX 32
>>
>> +bool ima_flush_htable;
>> +static int __init ima_flush_htable_setup(char *str)
>> +{
>> + ima_flush_htable = true;
>> + return 1;
>> +}
>> +__setup("ima_flush_htable", ima_flush_htable_setup);
>> +
>> /* pre-allocated array of tpm_digest structures to extend a PCR */
>> static struct tpm_digest *digests;
>>
>> @@ -220,6 +228,62 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>> return result;
>> }
>>
>> +/* Delete the IMA event logs */
>> +long ima_purge_event_log(long number_logs)
>> +{
>> + struct ima_queue_entry *qe, *qe_tmp;
>> + LIST_HEAD(ima_measurements_staged);
>> + unsigned int i;
>> + long cur = number_logs;
>> +
>> + if (number_logs <= 0)
>> + return number_logs;
>> +
>> + mutex_lock(&ima_extend_list_mutex);
>> +
>> +
>> + list_for_each_entry(qe, &ima_measurements, later) {
>> + if (--number_logs == 0)
>> + break;
>> + }
>> +
>> + if (number_logs > 0) {
>> + mutex_unlock(&ima_extend_list_mutex);
>> + return -ENOENT;
>> + }
>> +
>> + __list_cut_position(&ima_measurements_staged, &ima_measurements,
>> + &qe->later);
>> + atomic_long_sub(cur, &ima_htable.len);
>> +
>> + if (!IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE) && ima_flush_htable) {
>> + list_for_each_entry(qe, &ima_measurements_staged, later)
>> + /* It can race with ima_lookup_digest_entry(). */
>> + hlist_del_rcu(&qe->hnext);
>> + }
>> +
>> + mutex_unlock(&ima_extend_list_mutex);
>> +
>> +
>> + list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_staged, later) {
>> + for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
>> + kfree(qe->entry->template_data[i].data);
>> + qe->entry->template_data[i].data = NULL;
>> + qe->entry->template_data[i].len = 0;
>> + }
>> +
>> + list_del(&qe->later);
>> +
>> + if (ima_flush_htable) {
>> + kfree(qe->entry->digests);
>> + kfree(qe->entry);
>> + kfree(qe);
>> + }
>> + }
>> +
>> + return cur;
>> +}
>> +
>> int ima_restore_measurement_entry(struct ima_template_entry *entry)
>> {
>> int result = 0;
^ permalink raw reply
* Re: [RFC][PATCH] ima: Add support for staging measurements for deletion
From: steven chen @ 2025-12-11 18:06 UTC (permalink / raw)
To: Roberto Sassu, Gregory Lumen
Cc: corbet, zohar, dmitry.kasatkin, eric.snowberg, paul, jmorris,
serge, linux-doc, linux-kernel, linux-integrity,
linux-security-module, nramas, Roberto Sassu, steven chen
In-Reply-To: <75d8b82a2e493ca919926310c5f381221555d82d.camel@huaweicloud.com>
On 12/11/2025 7:24 AM, Roberto Sassu wrote:
> On Thu, 2025-12-11 at 15:50 +0100, Roberto Sassu wrote:
>> On Thu, 2025-12-11 at 10:56 +0100, Roberto Sassu wrote:
>>> On Wed, 2025-12-10 at 11:12 -0800, Gregory Lumen wrote:
>>>> Roberto,
>>>>
>>>> The proposed approach appears to be workable. However, if our primary goal
>>>> here is to enable UM to free kernel memory consumed by the IMA log with an
>>>> absolute minimum of kernel functionality/change, then I would argue that
>>>> the proposed Stage-then-delete approach still represents unnecessary
>>>> complexity when compared to a trim-to-N solution. Specifically:
>> The benefit of the Stage-then-delete is that you don't need to scan the
>> IMA measurements list in advance to determine what to trim, you just
>> trim everything by swapping list head (very fast) and then you can read
>> and delete the measurements out of the hot path.
> I forgot: I will also add in my patch the ability to stage and trim in
> one step, to satisfy your use case.
>
> Roberto
Hi Roberto,
The below is what you want in one step. I think anything more than this
does not bring any extra value.
I released version 2 of trim N entries patch as bellow:
[PATCH v2 0/1] Trim N entries of IMA event logs
<https://lore.kernel.org/linux-integrity/20251210235314.3341-1-chenste@linux.microsoft.com/T/#t>
Steven
>> [...]
>>
>>>> - There exists a potential UM measurement-loss race condition introduced
>>>> by the staging functionality that would not exist with a trim-to-N
>>>> approach. (Occurs if a kexec call occurs after a UM agent has staged
>>>> measurements for deletion, but has not completed copying them to
>>>> userspace). This could be avoided by persisting staged measurements across
>>>> kexec calls at the cost of making the proposed change larger.
>>> The solution is to coordinate the staging with kexec in user space.
>> To avoid requiring coordination in user space, I will try to see if I
>> could improve my patch to prepend the staged entries to the current
>> measurement list, before serializing them for kexec().
>>
>> Roberto
^ permalink raw reply
* [PATCH v1 17/17] tpm/tpm_ftpm_tee: Make use of tee bus methods
From: Uwe Kleine-König @ 2025-12-11 17:15 UTC (permalink / raw)
To: Jens Wiklander, Peter Huewe, Jarkko Sakkinen
Cc: Jason Gunthorpe, Sumit Garg, op-tee, linux-integrity,
linux-kernel
In-Reply-To: <cover.1765472125.git.u.kleine-koenig@baylibre.com>
The tee bus got dedicated callbacks for probe and remove.
Make use of these. This fixes a runtime warning about the driver needing
to be converted to the bus methods.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
drivers/char/tpm/tpm_ftpm_tee.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/char/tpm/tpm_ftpm_tee.c b/drivers/char/tpm/tpm_ftpm_tee.c
index e5fbc70b0eca..20294d1953a3 100644
--- a/drivers/char/tpm/tpm_ftpm_tee.c
+++ b/drivers/char/tpm/tpm_ftpm_tee.c
@@ -169,7 +169,7 @@ static int ftpm_tee_match(struct tee_ioctl_version_data *ver, const void *data)
* Return:
* On success, 0. On failure, -errno.
*/
-static int ftpm_tee_probe(struct device *dev)
+static int ftpm_tee_probe_generic(struct device *dev)
{
int rc;
struct tpm_chip *chip;
@@ -251,11 +251,18 @@ static int ftpm_tee_probe(struct device *dev)
return rc;
}
+static int ftpm_tee_probe(struct tee_client_device *tcdev)
+{
+ struct device *dev = &tcdev->dev;
+
+ return ftpm_tee_probe_generic(dev);
+}
+
static int ftpm_plat_tee_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- return ftpm_tee_probe(dev);
+ return ftpm_tee_probe_generic(dev);
}
/**
@@ -265,7 +272,7 @@ static int ftpm_plat_tee_probe(struct platform_device *pdev)
* Return:
* 0 always.
*/
-static int ftpm_tee_remove(struct device *dev)
+static void ftpm_tee_remove_generic(struct device *dev)
{
struct ftpm_tee_private *pvt_data = dev_get_drvdata(dev);
@@ -285,15 +292,20 @@ static int ftpm_tee_remove(struct device *dev)
tee_client_close_context(pvt_data->ctx);
/* memory allocated with devm_kzalloc() is freed automatically */
+}
- return 0;
+static void ftpm_tee_remove(struct tee_client_device *tcdev)
+{
+ struct device *dev = &tcdev->dev;
+
+ ftpm_tee_remove_generic(dev);
}
static void ftpm_plat_tee_remove(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- ftpm_tee_remove(dev);
+ ftpm_tee_remove_generic(dev);
}
/**
@@ -335,11 +347,11 @@ static const struct tee_client_device_id optee_ftpm_id_table[] = {
MODULE_DEVICE_TABLE(tee, optee_ftpm_id_table);
static struct tee_client_driver ftpm_tee_driver = {
+ .probe = ftpm_tee_probe,
+ .remove = ftpm_tee_remove,
.id_table = optee_ftpm_id_table,
.driver = {
.name = "optee-ftpm",
- .probe = ftpm_tee_probe,
- .remove = ftpm_tee_remove,
},
};
--
2.47.3
^ permalink raw reply related
* [PATCH v1 16/17] tpm/tpm_ftpm_tee: Make use of tee specific driver registration
From: Uwe Kleine-König @ 2025-12-11 17:15 UTC (permalink / raw)
To: Jens Wiklander, Peter Huewe, Jarkko Sakkinen
Cc: Jason Gunthorpe, Sumit Garg, op-tee, linux-integrity,
linux-kernel
In-Reply-To: <cover.1765472125.git.u.kleine-koenig@baylibre.com>
tee_client_driver_register() is typed more strongly and cares about
assigning the driver's bus. Similar for tee_client_driver_unregister().
Make use of these functions.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
drivers/char/tpm/tpm_ftpm_tee.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/char/tpm/tpm_ftpm_tee.c b/drivers/char/tpm/tpm_ftpm_tee.c
index 4e63c30aeaf1..e5fbc70b0eca 100644
--- a/drivers/char/tpm/tpm_ftpm_tee.c
+++ b/drivers/char/tpm/tpm_ftpm_tee.c
@@ -338,7 +338,6 @@ static struct tee_client_driver ftpm_tee_driver = {
.id_table = optee_ftpm_id_table,
.driver = {
.name = "optee-ftpm",
- .bus = &tee_bus_type,
.probe = ftpm_tee_probe,
.remove = ftpm_tee_remove,
},
@@ -352,7 +351,7 @@ static int __init ftpm_mod_init(void)
if (rc)
return rc;
- rc = driver_register(&ftpm_tee_driver.driver);
+ rc = tee_client_driver_register(&ftpm_tee_driver);
if (rc) {
platform_driver_unregister(&ftpm_tee_plat_driver);
return rc;
@@ -364,7 +363,7 @@ static int __init ftpm_mod_init(void)
static void __exit ftpm_mod_exit(void)
{
platform_driver_unregister(&ftpm_tee_plat_driver);
- driver_unregister(&ftpm_tee_driver.driver);
+ tee_client_driver_unregister(&ftpm_tee_driver);
}
module_init(ftpm_mod_init);
--
2.47.3
^ permalink raw reply related
* [PATCH v1 15/17] KEYS: trusted: Make use of tee bus methods
From: Uwe Kleine-König @ 2025-12-11 17:15 UTC (permalink / raw)
To: Jens Wiklander, Sumit Garg, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn
Cc: op-tee, linux-integrity, keyrings, linux-security-module,
linux-kernel
In-Reply-To: <cover.1765472125.git.u.kleine-koenig@baylibre.com>
The tee bus got dedicated callbacks for probe and remove.
Make use of these. This fixes a runtime warning about the driver needing
to be converted to the bus methods.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
security/keys/trusted-keys/trusted_tee.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
index 3cea9a377955..6e465c8bef5e 100644
--- a/security/keys/trusted-keys/trusted_tee.c
+++ b/security/keys/trusted-keys/trusted_tee.c
@@ -202,9 +202,9 @@ static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
return 0;
}
-static int trusted_key_probe(struct device *dev)
+static int trusted_key_probe(struct tee_client_device *rng_device)
{
- struct tee_client_device *rng_device = to_tee_client_device(dev);
+ struct device *dev = &rng_device->dev;
int ret;
struct tee_ioctl_open_session_arg sess_arg;
@@ -244,13 +244,11 @@ static int trusted_key_probe(struct device *dev)
return ret;
}
-static int trusted_key_remove(struct device *dev)
+static void trusted_key_remove(struct tee_client_device *dev)
{
unregister_key_type(&key_type_trusted);
tee_client_close_session(pvt_data.ctx, pvt_data.session_id);
tee_client_close_context(pvt_data.ctx);
-
- return 0;
}
static const struct tee_client_device_id trusted_key_id_table[] = {
@@ -261,11 +259,11 @@ static const struct tee_client_device_id trusted_key_id_table[] = {
MODULE_DEVICE_TABLE(tee, trusted_key_id_table);
static struct tee_client_driver trusted_key_driver = {
+ .probe = trusted_key_probe,
+ .remove = trusted_key_remove,
.id_table = trusted_key_id_table,
.driver = {
.name = DRIVER_NAME,
- .probe = trusted_key_probe,
- .remove = trusted_key_remove,
},
};
--
2.47.3
^ permalink raw reply related
* [PATCH v1 14/17] KEYS: trusted: Migrate to use tee specific driver registration function
From: Uwe Kleine-König @ 2025-12-11 17:15 UTC (permalink / raw)
To: Jens Wiklander, Sumit Garg, James Bottomley, Jarkko Sakkinen,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn
Cc: op-tee, linux-integrity, keyrings, linux-security-module,
linux-kernel
In-Reply-To: <cover.1765472125.git.u.kleine-koenig@baylibre.com>
The tee subsystem recently got a set of dedicated functions to register
(and unregister) a tee driver. Make use of them. These care for setting the
driver's bus (so the explicit assignment can be dropped) and the driver
owner (which is an improvement this driver benefits from).
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
---
security/keys/trusted-keys/trusted_tee.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
index aa3d477de6db..3cea9a377955 100644
--- a/security/keys/trusted-keys/trusted_tee.c
+++ b/security/keys/trusted-keys/trusted_tee.c
@@ -264,7 +264,6 @@ static struct tee_client_driver trusted_key_driver = {
.id_table = trusted_key_id_table,
.driver = {
.name = DRIVER_NAME,
- .bus = &tee_bus_type,
.probe = trusted_key_probe,
.remove = trusted_key_remove,
},
@@ -272,12 +271,12 @@ static struct tee_client_driver trusted_key_driver = {
static int trusted_tee_init(void)
{
- return driver_register(&trusted_key_driver.driver);
+ return tee_client_driver_register(&trusted_key_driver);
}
static void trusted_tee_exit(void)
{
- driver_unregister(&trusted_key_driver.driver);
+ tee_client_driver_unregister(&trusted_key_driver);
}
struct trusted_key_ops trusted_key_tee_ops = {
--
2.47.3
^ permalink raw reply related
* [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks
From: Uwe Kleine-König @ 2025-12-11 17:14 UTC (permalink / raw)
To: Jens Wiklander, Sumit Garg, Olivia Mackall, Herbert Xu,
Clément Léger, Alexandre Belloni, Ard Biesheuvel,
Maxime Coquelin, Alexandre Torgue, Sumit Garg, Ilias Apalodimas,
Jan Kiszka, Sudeep Holla, Christophe JAILLET, Michael Chan,
Pavan Chebbi, Rafał Miłecki, James Bottomley,
Jarkko Sakkinen, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, Peter Huewe
Cc: op-tee, linux-kernel, linux-crypto, linux-rtc, linux-efi,
linux-stm32, linux-arm-kernel, Cristian Marussi, arm-scmi, netdev,
linux-mips, linux-integrity, keyrings, linux-security-module,
Jason Gunthorpe
Hello,
the objective of this series is to make tee driver stop using callbacks
in struct device_driver. These were superseded by bus methods in 2006
(commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
methods.")) but nobody cared to convert all subsystems accordingly.
Here the tee drivers are converted. The first commit is somewhat
unrelated, but simplifies the conversion (and the drivers). It
introduces driver registration helpers that care about setting the bus
and owner. (The latter is missing in all drivers, so by using these
helpers the drivers become more correct.)
The patches #4 - #17 depend on the first two, so if they should be
applied to their respective subsystem trees these must contain the first
two patches first.
Note that after patch #2 is applied, unconverted drivers provoke a
warning in driver_register(), so it would be good for the user
experience if the whole series goes in during a single merge window. So
I guess an immutable branch containing the frist three patches that can
be merged into the other subsystem trees would be sensible.
After all patches are applied, tee_bus_type can be made private to
drivers/tee as it's not used in other places any more.
Best regards
Uwe
Uwe Kleine-König (17):
tee: Add some helpers to reduce boilerplate for tee client drivers
tee: Add probe, remove and shutdown bus callbacks to tee_client_driver
tee: Adapt documentation to cover recent additions
hwrng: optee - Make use of module_tee_client_driver()
hwrng: optee - Make use of tee bus methods
rtc: optee: Migrate to use tee specific driver registration function
rtc: optee: Make use of tee bus methods
efi: stmm: Make use of module_tee_client_driver()
efi: stmm: Make use of tee bus methods
firmware: arm_scmi: optee: Make use of module_tee_client_driver()
firmware: arm_scmi: Make use of tee bus methods
firmware: tee_bnxt: Make use of module_tee_client_driver()
firmware: tee_bnxt: Make use of tee bus methods
KEYS: trusted: Migrate to use tee specific driver registration
function
KEYS: trusted: Make use of tee bus methods
tpm/tpm_ftpm_tee: Make use of tee specific driver registration
tpm/tpm_ftpm_tee: Make use of tee bus methods
Documentation/driver-api/tee.rst | 18 +----
drivers/char/hw_random/optee-rng.c | 26 ++----
drivers/char/tpm/tpm_ftpm_tee.c | 31 +++++---
drivers/firmware/arm_scmi/transports/optee.c | 32 +++-----
drivers/firmware/broadcom/tee_bnxt_fw.c | 30 ++-----
drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++----
drivers/rtc/rtc-optee.c | 27 ++-----
drivers/tee/tee_core.c | 84 ++++++++++++++++++++
include/linux/tee_drv.h | 12 +++
security/keys/trusted-keys/trusted_tee.c | 17 ++--
10 files changed, 164 insertions(+), 138 deletions(-)
base-commit: 7d0a66e4bb9081d75c82ec4957c50034cb0ea449
--
2.47.3
^ permalink raw reply
* Re: [RFC][PATCH] ima: Add support for staging measurements for deletion
From: Roberto Sassu @ 2025-12-11 15:24 UTC (permalink / raw)
To: Gregory Lumen
Cc: corbet, zohar, dmitry.kasatkin, eric.snowberg, paul, jmorris,
serge, linux-doc, linux-kernel, linux-integrity,
linux-security-module, chenste, nramas, Roberto Sassu
In-Reply-To: <2f550d4cd860022e990d1de62049df85a6a86df8.camel@huaweicloud.com>
On Thu, 2025-12-11 at 15:50 +0100, Roberto Sassu wrote:
> On Thu, 2025-12-11 at 10:56 +0100, Roberto Sassu wrote:
> > On Wed, 2025-12-10 at 11:12 -0800, Gregory Lumen wrote:
> > > Roberto,
> > >
> > > The proposed approach appears to be workable. However, if our primary goal
> > > here is to enable UM to free kernel memory consumed by the IMA log with an
> > > absolute minimum of kernel functionality/change, then I would argue that
> > > the proposed Stage-then-delete approach still represents unnecessary
> > > complexity when compared to a trim-to-N solution. Specifically:
>
> The benefit of the Stage-then-delete is that you don't need to scan the
> IMA measurements list in advance to determine what to trim, you just
> trim everything by swapping list head (very fast) and then you can read
> and delete the measurements out of the hot path.
I forgot: I will also add in my patch the ability to stage and trim in
one step, to satisfy your use case.
Roberto
> [...]
>
> >
> > > - There exists a potential UM measurement-loss race condition introduced
> > > by the staging functionality that would not exist with a trim-to-N
> > > approach. (Occurs if a kexec call occurs after a UM agent has staged
> > > measurements for deletion, but has not completed copying them to
> > > userspace). This could be avoided by persisting staged measurements across
> > > kexec calls at the cost of making the proposed change larger.
> >
> > The solution is to coordinate the staging with kexec in user space.
>
> To avoid requiring coordination in user space, I will try to see if I
> could improve my patch to prepend the staged entries to the current
> measurement list, before serializing them for kexec().
>
> Roberto
^ permalink raw reply
* Re: [RFC][PATCH] ima: Add support for staging measurements for deletion
From: Roberto Sassu @ 2025-12-11 14:50 UTC (permalink / raw)
To: Gregory Lumen
Cc: corbet, zohar, dmitry.kasatkin, eric.snowberg, paul, jmorris,
serge, linux-doc, linux-kernel, linux-integrity,
linux-security-module, chenste, nramas, Roberto Sassu
In-Reply-To: <d7418d0afa696b8da67e4f25fd0dc1b9d6fd908f.camel@huaweicloud.com>
On Thu, 2025-12-11 at 10:56 +0100, Roberto Sassu wrote:
> On Wed, 2025-12-10 at 11:12 -0800, Gregory Lumen wrote:
> > Roberto,
> >
> > The proposed approach appears to be workable. However, if our primary goal
> > here is to enable UM to free kernel memory consumed by the IMA log with an
> > absolute minimum of kernel functionality/change, then I would argue that
> > the proposed Stage-then-delete approach still represents unnecessary
> > complexity when compared to a trim-to-N solution. Specifically:
The benefit of the Stage-then-delete is that you don't need to scan the
IMA measurements list in advance to determine what to trim, you just
trim everything by swapping list head (very fast) and then you can read
and delete the measurements out of the hot path.
[...]
>
> > - There exists a potential UM measurement-loss race condition introduced
> > by the staging functionality that would not exist with a trim-to-N
> > approach. (Occurs if a kexec call occurs after a UM agent has staged
> > measurements for deletion, but has not completed copying them to
> > userspace). This could be avoided by persisting staged measurements across
> > kexec calls at the cost of making the proposed change larger.
>
> The solution is to coordinate the staging with kexec in user space.
To avoid requiring coordination in user space, I will try to see if I
could improve my patch to prepend the staged entries to the current
measurement list, before serializing them for kexec().
Roberto
^ permalink raw reply
* [PATCH] ima_violations.sh: Another fix of condition evaluation
From: Petr Vorel @ 2025-12-11 11:10 UTC (permalink / raw)
To: ltp; +Cc: Petr Vorel, Mimi Zohar, linux-integrity, Martin Doucha
c0c35509f9 was not enough to fix evaluation against empty
$expected_violations:
ima_violations 1 TINFO: verify open writers violation
/opt/ltp/testcases/bin/ima_violations.sh: line 96: [: 0: unary operator expected
Therefore split checks into two if.
Also improvements (readability)
* shorten line length with saving subtraction into variable
* evaluate empty variable with ${:-}
Fixes: 726ed71905 ("ima_violations.sh: Update validate() to support multiple violations")
Reported-by: Martin Doucha <mdoucha@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
NOTE: this was found on old SLES 4.4 based kernel which does not log
validations. But missing validations might be just a Secure Boot related
setup problem:
$ mokutil --sb-state
Secure Boot: EFI variables not supported on SUT
Events are logged when Secure Boot is off:
$ mokutil --sb-state
SecureBoot disabled
Or maybe violations worked differently on the old kernel (I remember
only 6.15 change).
Kind regards,
Petr
.../integrity/ima/tests/ima_violations.sh | 21 ++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
index 1d2f1d9447..a8476e6b59 100755
--- a/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
+++ b/testcases/kernel/security/integrity/ima/tests/ima_violations.sh
@@ -87,23 +87,30 @@ validate()
local search="$3"
local expected_violations="$4"
local max_attempt=3
- local count2 i num_violations_new
+ local count2 diff i num_violations_new pass
for i in $(seq 1 $max_attempt); do
read num_violations_new < $IMA_VIOLATIONS
count2="$(get_count $search)"
- if [ -z "$expected_violations" -a $(($num_violations_new - $num_violations)) -gt 0 ] || \
- [ $(($num_violations_new - $num_violations)) -eq $expected_violations ]; then
- [ -z "$expected_violations" ] && expected_violations=1
+ diff=$(($num_violations_new - $num_violations))
+
+ if [ "$expected_violations" ] && [ $diff -eq $expected_violations ]; then
+ pass=1
+ fi
+ if [ -z "$expected_violations" ] && [ $diff -gt 0 ]; then
+ pass=1
+ fi
+
+ if [ "$pass" = 1 ]; then
if [ $count2 -gt $count ]; then
- tst_res TPASS "$expected_violations $search violation(s) added"
+ tst_res TPASS "${expected_violations:-1} $search violation(s) added"
return
else
tst_res TINFO "$search not found in $LOG ($i/$max_attempt attempt)..."
tst_sleep 1s
fi
- elif [ $(($num_violations_new - $num_violations)) -gt 0 ]; then
- tst_res $IMA_FAIL "$search too many violations added: $num_violations_new - $num_violations"
+ elif [ $diff -gt 0 ]; then
+ tst_res $IMA_FAIL "$search too many violations added: $diff ($num_violations_new - $num_violations)"
return
else
tst_res $IMA_FAIL "$search violation not added"
--
2.51.0
^ permalink raw reply related
* Re: [LTP] [PATCH v4 0/4] tst_runas.c, ima_{conditionals, measurements}.sh enhancements
From: Andrea Cervesato @ 2025-12-11 10:37 UTC (permalink / raw)
To: Petr Vorel, ltp; +Cc: selinux, linux-integrity
In-Reply-To: <20251209185557.20598-1-pvorel@suse.cz>
Hi Petr,
thanks for adding this tool. We will probably need to update
documentation as well after this patch will be merged.
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox