On Wed, 2026-05-20 at 22:07 -0400, Mimi Zohar wrote:
> On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> > From: Roberto Sassu <[email protected]>
> >
> > Introduce the ima_measure_users counter, to implement a semaphore-like
> > locking scheme where the binary and ASCII measurements list interfaces can
> > be concurrently open by multiple readers, or alternatively by a single
> > writer.
> >
> > A semaphore cannot be used because the kernel cannot return to user space
> > with a lock held.
> >
> > Introduce the ima_measure_lock() and ima_measure_unlock() primitives, to
> > respectively lock/unlock the interfaces (safely with the ima_measure_users
> > counter, without holding a lock).
> >
> > Finally, introduce _ima_measurements_open() to lock the interface before
> > seq_open(), and call it from ima_measurements_open() and
> > ima_ascii_measurements_open(). And, introduce ima_measurements_release(),
> > to unlock the interface.
> >
> > Require CAP_SYS_ADMIN if the interface is opened for write (not possible
> > for the current measurements interfaces, since they only have read
> > permission).
> >
> > No functional changes: multiple readers are allowed as before.
> >
> > Link: https://github.com/linux-integrity/linux/issues/1
> > Signed-off-by: Roberto Sassu <[email protected]>
> > ---
> > security/integrity/ima/ima_fs.c | 71 +++++++++++++++++++++++++++++++--
> > 1 file changed, 67 insertions(+), 4 deletions(-)
> >
> > diff --git a/security/integrity/ima/ima_fs.c
> > b/security/integrity/ima/ima_fs.c
> > index 9a8dba14d82a..68edea7139d5 100644
> > --- a/security/integrity/ima/ima_fs.c
> > +++ b/security/integrity/ima/ima_fs.c
> > @@ -25,6 +25,8 @@
> > #include "ima.h"
> >
> > static DEFINE_MUTEX(ima_write_mutex);
> > +static DEFINE_MUTEX(ima_measure_mutex);
> > +static long ima_measure_users;
>
> long?
The limit pre process can be up to INT_MAX. Two processes could
overflow the counter if it was int.
Since privileged users can bypass the system wide max-file check (see
alloc_empty_file()), I will add an overflow check to be sure.
> >
> > bool ima_canonical_fmt;
> > static int __init default_canonical_fmt_setup(char *str)
> > @@ -209,16 +211,76 @@ static const struct seq_operations
> > ima_measurments_seqops = {
> > .show = ima_measurements_show
> > };
> >
> > +static int ima_measure_lock(bool write)
> > +{
> > + mutex_lock(&ima_measure_mutex);
> > + if ((write && ima_measure_users != 0) ||
> > + (!write && ima_measure_users < 0)) {
> > + mutex_unlock(&ima_measure_mutex);
> > + return -EBUSY;
> > + }
>
> Thanks, Roberto. The code is really clear and well written. However, it could
> use a comment indicating the different ima_measure_users values as a reminder.
>
> ima_measure_users: > 0 open readers
> ima_meaasure_users: == -1 open writer
Ok.
> > +
> > + if (write)
> > + ima_measure_users--;
> > + else
> > + ima_measure_users++;
> > + mutex_unlock(&ima_measure_mutex);
> > + return 0;
> > +}
> > +
> > +static void ima_measure_unlock(bool write)
> > +{
> > + mutex_lock(&ima_measure_mutex);
> > + if (write)
> > + ima_measure_users++;
>
> There should only be one writer at a time. ima_measure_users could be set to
> zero.
Sure, but I find the code more clear this way.
Roberto
> > + else
> > + ima_measure_users--;
> > + mutex_unlock(&ima_measure_mutex);
> > +}
> > +
>
> thanks,
>
> Mimi
