Skip to contentSkip to conversation

ima: limit both open-writers and ToMToU violations

PATCH Superseded Mimi Zohar 4 participants 14 messages Feb 19, 2025 – Feb 27, 2025

Subsystems (4)
Security (LSM) Keys-trusted Integrity measurement architecture (IMA) Extended Verification Module (EVM)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Mimi Zohar

author maintainer
#0 lore raw

cover ima: limit both open-writers and ToMToU violations

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, even if an open-writers violation has already been recorded.

Similalry 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, even if a ToMToU violation has already been recorded.

Minimize the violations in the audit log and the IMA measurement list.

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      |  1 +
 security/integrity/ima/ima_main.c | 16 ++++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

--
2.48.1

author maintainer
#1 lore raw

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

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, 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 occur.

Signed-off-by Mimi Zohar <[email protected]>

---
Change log v1:
- Basesd on Stefan's RFC comments, updated the patch description and code.

 security/integrity/ima/ima.h      |  1 +
 security/integrity/ima/ima_main.c | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)
security/integrity/ima/ima.h +1 -0
index a4f284bd846c..7f21568544dd 100644
@@ -182,6 +182,7 @@ struct ima_kexec_hdr {
 #define IMA_CHANGE_ATTR		2
 #define IMA_DIGSIG		3
 #define IMA_MUST_MEASURE	4
+#define IMA_LIMIT_VIOLATIONS	5
 
 /* IMA integrity metadata associated with an inode */
 struct ima_iint_cache {
security/integrity/ima/ima_main.c +9 -2
index 28b8b0db6f9b..cde3ae55d654 100644
@@ -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_LIMIT_VIOLATIONS,
+					      &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_LIMIT_VIOLATIONS, &iint->atomic_flags);
+
 		update = test_and_clear_bit(IMA_UPDATE_XATTR,
 					    &iint->atomic_flags);
 		if ((iint->flags & IMA_NEW_FILE) ||

--
2.48.1

Tested-by on v1 1/2
#2 lore raw

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

Quoted 10 lines · 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, 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 occur.

Signed-off-by Mimi Zohar <[email protected]>
Tested-by Stefan Berger <[email protected]>
Quoted 52 lines

---
Change log v1:
- Basesd on Stefan's RFC comments, updated the patch description and code.

  security/integrity/ima/ima.h      |  1 +
  security/integrity/ima/ima_main.c | 11 +++++++++--
  2 files changed, 10 insertions(+), 2 deletions(-)
security/integrity/ima/ima.h +1 -0
index a4f284bd846c..7f21568544dd 100644
@@ -182,6 +182,7 @@ struct ima_kexec_hdr {
  #define IMA_CHANGE_ATTR		2
  #define IMA_DIGSIG		3
  #define IMA_MUST_MEASURE	4
+#define IMA_LIMIT_VIOLATIONS	5
 
  /* IMA integrity metadata associated with an inode */
  struct ima_iint_cache {
security/integrity/ima/ima_main.c +9 -3
index 28b8b0db6f9b..cde3ae55d654 100644
@@ -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_LIMIT_VIOLATIONS,
+					      &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_LIMIT_VIOLATIONS, &iint->atomic_flags);
+
  		update = test_and_clear_bit(IMA_UPDATE_XATTR,
  					    &iint->atomic_flags);
  		if ((iint->flags & IMA_NEW_FILE) ||
--

2.48.1

Reviewed-by on v1 1/2
#3 lore raw

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

In reply to Mimi Zohar

Hi Mimi,

Quoted 8 lines

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, 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 occur.

LGTM.

Reviewed-by Petr Vorel <[email protected]>

I also did a regression testing on LTP IMA tests on x86_64, aarch64, ppc64le. (not testing the feature itself, just really a very basic regression testing, therefore I do not dare to add my TBT).

Kind regards,
Petr

#5 lore raw

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

In reply to Mimi Zohar
Quoted 8 lines · 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, 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 occur.

More precisely, may be emitted again.

Quoted 4 lines
Signed-off-by Mimi Zohar <[email protected]>

---
Change log v1:
- Basesd on Stefan's RFC comments, updated the patch description and code.

Based.

Could be also useful to have here what is changed.

security/integrity/ima/ima.h struct ima_kexec_hdr { · line 185

Quoted 13 lines
 security/integrity/ima/ima.h      |  1 +
 security/integrity/ima/ima_main.c | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)
security/integrity/ima/ima.h +1 -0
index a4f284bd846c..7f21568544dd 100644
@@ -182,6 +182,7 @@ struct ima_kexec_hdr {
 #define IMA_CHANGE_ATTR		2
 #define IMA_DIGSIG		3
 #define IMA_MUST_MEASURE	4
+#define IMA_LIMIT_VIOLATIONS	5

I like to name variables in a way that it is clear what the intent is. Thinking about it, maybe:

IMA_OPEN_WRITERS_EMITTED

Thanks

Roberto

Quoted 34 lines
security/integrity/ima/ima.h +0 -0
 /* IMA integrity metadata associated with an inode */
 struct ima_iint_cache {
security/integrity/ima/ima_main.c +9 -3
index 28b8b0db6f9b..cde3ae55d654 100644
@@ -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_LIMIT_VIOLATIONS,
+					      &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_LIMIT_VIOLATIONS, &iint->atomic_flags);
+
 		update = test_and_clear_bit(IMA_UPDATE_XATTR,
 					    &iint->atomic_flags);
 		if ((iint->flags & IMA_NEW_FILE) ||
--

2.48.1

author maintainer
#6 lore raw

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

In reply to 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, even if a ToMToU violation has already been recorded.

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

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

Signed-off-by Mimi Zohar <[email protected]>
---
 security/integrity/ima/ima_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
security/integrity/ima/ima_main.c +3 -2
index cde3ae55d654..f1671799a11b 100644
@@ -129,9 +129,10 @@ 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_MUST_MEASURE,
+						       &iint->atomic_flags))
 				send_tomtou = true;
 		}
 	} else {
Signature · 1 line
2.48.1
Tested-by on v1 2/2
#7 lore raw

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

Quoted 5 lines · 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, even if a ToMToU violation has already been recorded.

Quoted 8 lines

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

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

Signed-off-by Mimi Zohar <[email protected]>
Tested-by Stefan Berger <[email protected]>
Quoted 21 lines
---
  security/integrity/ima/ima_main.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
security/integrity/ima/ima_main.c +3 -2
index cde3ae55d654..f1671799a11b 100644
@@ -129,9 +129,10 @@ 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_MUST_MEASURE,
+						       &iint->atomic_flags))
  				send_tomtou = true;
  		}
  	} else {
#10 lore raw

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

In reply to Mimi Zohar
Quoted 11 lines · 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, even if a ToMToU violation has already been recorded.

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

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

Probably the goal can be summarized as to limit emitting consecutive ToMToU violations.

In the previous patch, we are not emitting a new open_writers violation until all writers close the file. Here, it is a bit different, we are not emitting an additional ToMToU violation until there is another reader matching the policy. Maybe we should highlight this difference.

Quoted 10 lines
Signed-off-by Mimi Zohar <[email protected]>
---
 security/integrity/ima/ima_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
security/integrity/ima/ima_main.c +0 -0
index cde3ae55d654..f1671799a11b 100644

security/integrity/ima/ima_main.c static void ima_rdwr_violation_check(struct file *file, · line 134

Quoted 8 lines
security/integrity/ima/ima_main.c +2 -2
@@ -129,9 +129,10 @@ 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_MUST_MEASURE,

Since IMA_MUST_MEASURE is only used for violations, what if we rename it to:

IMA_TOMTOU_MAY_EMIT

Thanks

Roberto

Quoted 4 lines
security/integrity/ima/ima_main.c +1 -0
+						       &iint->atomic_flags))
 				send_tomtou = true;
 		}
 	} else {
author maintainer
#11 lore raw

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

Hi Roberto,

Quoted 9 lines · Roberto Sassu
Quoted 8 lines · 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, even if a ToMToU violation has already been recorded.

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

Quoted 6 lines
Quoted 3 lines

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

Probably the goal can be summarized as to limit emitting consecutive ToMToU violations.

Other audit messages and measurements could have been emitted, so they may not be consecutive.

Quoted 28 lines

In the previous patch, we are not emitting a new open_writers violation until all writers close the file. Here, it is a bit different, we are not emitting an additional ToMToU violation until there is another reader matching the policy. Maybe we should highlight this difference.

Quoted 10 lines
Signed-off-by Mimi Zohar <[email protected]>
---
 security/integrity/ima/ima_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
security/integrity/ima/ima_main.c +0 -0
index cde3ae55d654..f1671799a11b 100644

security/integrity/ima/ima_main.c static void ima_rdwr_violation_check(struct file *file, · line 134

Quoted 8 lines
security/integrity/ima/ima_main.c +2 -2
@@ -129,9 +129,10 @@ 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_MUST_MEASURE,

Since IMA_MUST_MEASURE is only used for violations, what if we rename it to:

Quoted 1 line

IMA_TOMTOU_MAY_EMIT

How about naming the atomic flags as IMA_MAY_EMIT_TOMTOU and IMA_EMIT_OPENWRITERS?

Mimi

#12 lore raw

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

Quoted 16 lines · Mimi Zohar

Hi Roberto,

Quoted 13 lines · Roberto Sassu
Quoted 11 lines · 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, even if a ToMToU violation has already been recorded.

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

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

Quoted 5 lines
Quoted 2 lines

Probably the goal can be summarized as to limit emitting consecutive ToMToU violations.

Other audit messages and measurements could have been emitted, so they may not be consecutive.

Ah, sorry, not well expressed. I meant if there is a second ToMToU violation after the first (read -> write -> write). Not consecutive means when there is a new measurement (more correct would be when there is a new policy match) on the same file to be invalidated.

Quoted 28 lines
Quoted 28 lines

In the previous patch, we are not emitting a new open_writers violation until all writers close the file. Here, it is a bit different, we are not emitting an additional ToMToU violation until there is another reader matching the policy. Maybe we should highlight this difference.

Quoted 10 lines
Signed-off-by Mimi Zohar <[email protected]>
---
 security/integrity/ima/ima_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
security/integrity/ima/ima_main.c +0 -0
index cde3ae55d654..f1671799a11b 100644

security/integrity/ima/ima_main.c static void ima_rdwr_violation_check(struct file *file, · line 134

Quoted 8 lines
security/integrity/ima/ima_main.c +2 -2
@@ -129,9 +129,10 @@ 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_MUST_MEASURE,

Since IMA_MUST_MEASURE is only used for violations, what if we rename it to:

Quoted 4 lines
Quoted 1 line

IMA_TOMTOU_MAY_EMIT

How about naming the atomic flags as IMA_MAY_EMIT_TOMTOU and IMA_EMIT_OPENWRITERS?

Yes, I like them.

Thanks

Roberto

#13 lore raw
In reply to Mimi Zohar

On Wed, 2025-02-19 at 11:21 -0500, Mimi Zohar wrote:

Hi Mimi

Quoted 2 lines

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

I would put a comma after 'for read' and remove the previous ones.

Quoted 1 line

and a violation record is added to the IMA measurement list, even if an

I would stop the sentence before 'even' and start a new sentence.

IMA does not track previous violations, and emits a new one of the same kind, even if there was one before, resulting in redundant information being produced.

The information might not be redundant though, if process-based credentials are added to the measurement list. In that case, more information about the process causing the violation would be shown.

Quoted 3 lines

open-writers violation has already been recorded.

Similalry each time a file in policy, that is already opened for read,

Typo.

Quoted 6 lines

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, even if a ToMToU violation has already been recorded.

Minimize the violations in the audit log and the IMA measurement list.

I would describe more precisely how you are trying to minimize them.

Thanks

Roberto

Quoted 11 lines

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      |  1 +
 security/integrity/ima/ima_main.c | 16 ++++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

--
2.48.1