On Wed, 2025-02-26 at 14:19 -0500, Mimi Zohar wrote:
> Hi Roberto,
>
> On Fri, 2025-02-21 at 18:36 +0100, Roberto Sassu wrote:
> > On Wed, 2025-02-19 at 11:21 -0500, Mimi Zohar wrote:
> > > 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.
>
> 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.
> >
> > 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.
> >
> > > Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
> > > ---
> > > security/integrity/ima/ima_main.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > > index cde3ae55d654..f1671799a11b 100644
> > > --- a/security/integrity/ima/ima_main.c
> > > +++ b/security/integrity/ima/ima_main.c
> > > @@ -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
>
> How about naming the atomic flags as IMA_MAY_EMIT_TOMTOU and
> IMA_EMIT_OPENWRITERS?
Yes, I like them.
Thanks
Roberto
