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?
>
> 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
> +
> + 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.
> + else
> + ima_measure_users--;
> + mutex_unlock(&ima_measure_mutex);
> +}
> +
thanks,
Mimi