| Message ID | 20250219162131.416719-2-zohar@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ima: limit both open-writers and ToMToU violations | expand |
On 2/19/25 11:21 AM, 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, 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 <zohar@linux.ibm.com> Tested-by: Stefan Berger <stefanb@linux.ibm.com> > --- > 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(-) > > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > index a4f284bd846c..7f21568544dd 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_LIMIT_VIOLATIONS 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..cde3ae55d654 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_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 >
Hi Mimi, > 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 <pvorel@suse.cz> 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
Hi Mimi,
Tested-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index a4f284bd846c..7f21568544dd 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_LIMIT_VIOLATIONS 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..cde3ae55d654 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_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) ||
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 <zohar@linux.ibm.com> --- 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(-) -- 2.48.1