[PATCH v2 0/2] ima: minimize open-writers and ToMToU violations

[PATCH v2 0/2] ima: minimize open-writers and ToMToU violations

[Mimi Zohar]

Each time a file in policy, that is already opened for write, is opened
for read, an open-writers integrity violation audit message is emitted
and a violation record is added to the IMA measurement list.

Similarly each time a file in policy, that is already opened for read,
is opened for write, a Time-of-Measure-Time-of-Use (ToMToU) integrity
violation audit message is emitted and a violation record is added to
the IMA measurement list.

As there is no benefit in having multiple open-writers or ToMToU
violations for the same file open in the audit log and IMA measurement
list, minimize them.

Minimizing open-writer violations results in a single open-writers
violation being emitted until all writers are closed no matter the
number of subsequent file open readers (or writers).

Minimizing ToMToU violations results in a single ToMToU violation being
emitted for all subsequent file open writers, until another in policy
file open reader.

Since the IMA_MUST_MEASURE atomic flag is only used for tracking ToMToU
violations, rename the atomic flag to IMA_MAY_EMIT_TOMTOU.

Define a new atomic flag named IMA_EMITTED_OPENWRITERS to minimize
open-writer violations.

Mimi Zohar (2):
  ima: limit the number of open-writers integrity violations
  ima: limit the number of ToMToU integrity violations

 security/integrity/ima/ima.h      |  3 ++-
 security/integrity/ima/ima_main.c | 18 +++++++++++++-----
 2 files changed, 15 insertions(+), 6 deletions(-)

--
2.48.1

[PATCH v2 1/2] ima: limit the number of open-writers integrity violations

[Mimi Zohar]

Each time a file in policy, that is already opened for write, is opened
for read, an open-writers integrity violation audit message is emitted
and a violation record is added to the IMA measurement list. This
occurs even if an open-writers violation has already been recorded.

Limit the number of open-writers integrity violations for an existing
file open for write to one.  After the existing file open for write
closes (__fput), subsequent open-writers integrity violations may be
emitted.

Tested-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
Changelog v2:
- Based on Roberto's review, renamed the atomic flag name, updated the
patch description.
- Added Petr's and Stefan's tags.

Changelog v1:
- Basesd on Stefan's RFC comments, updated the patch description and code:
removed unnecessary checks, simplified code.

 security/integrity/ima/ima.h      |  1 +
 security/integrity/ima/ima_main.c | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index a4f284bd846c..3423b3088de5 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -182,6 +182,7 @@ struct ima_kexec_hdr {
 #define IMA_CHANGE_ATTR		2
 #define IMA_DIGSIG		3
 #define IMA_MUST_MEASURE	4
+#define IMA_EMITTED_OPENWRITERS	5

 /* IMA integrity metadata associated with an inode */
 struct ima_iint_cache {
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 28b8b0db6f9b..95118c1887cb 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -137,8 +137,13 @@ static void ima_rdwr_violation_check(struct file *file,
 	} else {
 		if (must_measure)
 			set_bit(IMA_MUST_MEASURE, &iint->atomic_flags);
-		if (inode_is_open_for_write(inode) && must_measure)
-			send_writers = true;
+
+		/* Limit number of open_writers violations */
+		if (inode_is_open_for_write(inode) && must_measure) {
+			if (!test_and_set_bit(IMA_EMITTED_OPENWRITERS,
+					      &iint->atomic_flags))
+				send_writers = true;
+		}
 	}

 	if (!send_tomtou && !send_writers)
@@ -167,6 +172,8 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
 	if (atomic_read(&inode->i_writecount) == 1) {
 		struct kstat stat;

+		clear_bit(IMA_EMITTED_OPENWRITERS, &iint->atomic_flags);
+
 		update = test_and_clear_bit(IMA_UPDATE_XATTR,
 					    &iint->atomic_flags);
 		if ((iint->flags & IMA_NEW_FILE) ||
--
2.48.1

[PATCH v2 2/2] ima: limit the number of ToMToU integrity violations

[Mimi Zohar]

Each time a file in policy, that is already opened for read, is opened
for write, a Time-of-Measure-Time-of-Use (ToMToU) integrity violation
audit message is emitted and a violation record is added to the IMA
measurement list.  This occurs even if a ToMToU violation has already
been recorded.

Limit the number of ToMToU integrity violations per file open for read.

Note: The IMA_MAY_EMIT_TOMTOU atomic flag must be set from the reader
side based on policy.  This may result in a per file open for read
ToMToU violation.

Since IMA_MUST_MEASURE is only used for violations, rename the atomic
IMA_MUST_MEASURE flag to IMA_MAY_EMIT_TOMTOU.

Tested-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
Changelog v2:
- Based on Roberto's review, renamed the atomic flag name, updated the patch
description.
- Added Petr's and Stefan's tags.

 security/integrity/ima/ima.h      | 2 +-
 security/integrity/ima/ima_main.c | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 3423b3088de5..e0489c6f7f59 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -181,7 +181,7 @@ struct ima_kexec_hdr {
 #define IMA_UPDATE_XATTR	1
 #define IMA_CHANGE_ATTR		2
 #define IMA_DIGSIG		3
-#define IMA_MUST_MEASURE	4
+#define IMA_MAY_EMIT_TOMTOU	4
 #define IMA_EMITTED_OPENWRITERS	5

 /* IMA integrity metadata associated with an inode */
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 95118c1887cb..f3e7ac513db3 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -129,14 +129,15 @@ static void ima_rdwr_violation_check(struct file *file,
 		if (atomic_read(&inode->i_readcount) && IS_IMA(inode)) {
 			if (!iint)
 				iint = ima_iint_find(inode);
+
 			/* IMA_MEASURE is set from reader side */
-			if (iint && test_bit(IMA_MUST_MEASURE,
-						&iint->atomic_flags))
+			if (iint && test_and_clear_bit(IMA_MAY_EMIT_TOMTOU,
+						       &iint->atomic_flags))
 				send_tomtou = true;
 		}
 	} else {
 		if (must_measure)
-			set_bit(IMA_MUST_MEASURE, &iint->atomic_flags);
+			set_bit(IMA_MAY_EMIT_TOMTOU, &iint->atomic_flags);

 		/* Limit number of open_writers violations */
 		if (inode_is_open_for_write(inode) && must_measure) {
--
2.48.1

Re: [PATCH v2 0/2] ima: minimize open-writers and ToMToU violations

[Roberto Sassu]

On Fri, 2025-02-28 at 15:55 -0500, Mimi Zohar wrote:
> Each time a file in policy, that is already opened for write, is opened
> for read, an open-writers integrity violation audit message is emitted
> and a violation record is added to the IMA measurement list.
> 
> Similarly each time a file in policy, that is already opened for read,
> is opened for write, a Time-of-Measure-Time-of-Use (ToMToU) integrity
> violation audit message is emitted and a violation record is added to
> the IMA measurement list.
> 
> As there is no benefit in having multiple open-writers or ToMToU
> violations for the same file open in the audit log and IMA measurement
> list, minimize them.
> 
> Minimizing open-writer violations results in a single open-writers
> violation being emitted until all writers are closed no matter the
> number of subsequent file open readers (or writers).
> 
> Minimizing ToMToU violations results in a single ToMToU violation being
> emitted for all subsequent file open writers, until another in policy
> file open reader.
> 
> Since the IMA_MUST_MEASURE atomic flag is only used for tracking ToMToU
> violations, rename the atomic flag to IMA_MAY_EMIT_TOMTOU.
> 
> Define a new atomic flag named IMA_EMITTED_OPENWRITERS to minimize
> open-writer violations.

Thanks Mimi. For the whole series:

Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>

Roberto

> Mimi Zohar (2):
>   ima: limit the number of open-writers integrity violations
>   ima: limit the number of ToMToU integrity violations
> 
>  security/integrity/ima/ima.h      |  3 ++-
>  security/integrity/ima/ima_main.c | 18 +++++++++++++-----
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> --
> 2.48.1
>