* Re: [PATCH v8 03/12] landlock: Replace union access_masks_all with helper functions
From: Mickaël Salaün @ 2026-04-01 17:57 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, kernel test robot, linux-security-module,
Tingmao Wang, Justin Suess, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
Demi Marie Obenour, Alyssa Ross, Jann Horn, Tahera Fahimi,
Sebastian Andrzej Siewior, Kuniyuki Iwashima, Georgia Garcia
In-Reply-To: <20260330.6229e57c9563@gnoack.org>
On Mon, Mar 30, 2026 at 09:00:31PM +0200, Günther Noack wrote:
> On Mon, Mar 30, 2026 at 12:53:21PM +0200, Mickaël Salaün wrote:
> > On Mon, Mar 30, 2026 at 11:56:40AM +0200, Mickaël Salaün wrote:
> > > On Fri, Mar 27, 2026 at 05:48:28PM +0100, Günther Noack wrote:
> > > > * Stop using a union for access_masks_all.
> > > > * Expose helper functions for intersection checks and union operations.
> > > >
> > > > The memory layout of bitfields is only loosely defined by the C
> > > > standard, so our static assertion that expects a fixed size was
> > > > brittle, and it broke on some compilers when we attempted to add a
> > > > 17th file system access right.
> > > >
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > Closes: https://lore.kernel.org/oe-kbuild-all/202603261438.jBx2DGNe-lkp@intel.com/
> > > > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > > > ---
> > > > security/landlock/access.h | 21 ++++++++++++++-------
> > > > security/landlock/cred.h | 10 ++--------
> > > > security/landlock/ruleset.h | 13 ++++---------
> > > > 3 files changed, 20 insertions(+), 24 deletions(-)
> > >
> > > I'd prefer this approach:
> > >
> > > diff --git a/security/landlock/access.h b/security/landlock/access.h
> > > index 89dc8e7b93da..bc9efbb5c900 100644
> > > --- a/security/landlock/access.h
> > > +++ b/security/landlock/access.h
> > > @@ -50,7 +50,7 @@ struct access_masks {
> > > access_mask_t fs : LANDLOCK_NUM_ACCESS_FS;
> > > access_mask_t net : LANDLOCK_NUM_ACCESS_NET;
> > > access_mask_t scope : LANDLOCK_NUM_SCOPE;
> > > -};
> > > +} __packed;
> >
> > Actually, we can just use '__packed __aligned(sizeof(u32))' and avoid
> > the static_assert change. That would have no impact on x86, but pack it
> > on m68k.
>
> Thanks, good catch (and thanks for pushing it to mic-next).
> Fingers crossed that this works on m68k.
So, this works! I did some experiments with m68k and this architecture
is very special: it packs bitfields at byte granularity, not at
storage-unit granularity, except when the size of a bitfield is a
multiple of 8, in which case it aligns on this size.
I also look at the past versions of Landlock (in the stable branches),
and they are good because struct access_masks (and the related assert)
was introduced in v6.11 and fs was exactly 16 bits, which makes m68k
aligns on 2 bytes and then the size of the struct was 4 bytes.
Switching fs to 17 bits removes this optimization (I guess) and pack
(back) to 3 bytes, so recording more bits can take less space!
That's why we don't need a standalone fix to backport...
^ permalink raw reply
* Re: [PATCH v4 09/13] ima: Add support for staging measurements with prompt
From: steven chen @ 2026-04-01 17:52 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, zohar, dmitry.kasatkin,
eric.snowberg, paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, nramas, Roberto Sassu, steven chen
In-Reply-To: <19a1815a1222bd78f6bfde30f60b60ebfacb65aa.camel@huaweicloud.com>
On 3/27/2026 9:45 AM, Roberto Sassu wrote:
> On Thu, 2026-03-26 at 15:44 -0700, steven chen wrote:
>> On 3/26/2026 10:30 AM, Roberto Sassu wrote:
>>> From: Roberto Sassu <roberto.sassu@huawei.com>
>>>
>>> Introduce the ability of staging the IMA measurement list and deleting them
>>> with a prompt.
>>>
>>> Staging means moving the current content of the measurement list to a
>>> separate location, and allowing users to read and delete it. This causes
>>> the measurement list to be atomically truncated before new measurements can
>>> be added. Staging can be done only once at a time. In the event of kexec(),
>>> staging is reverted and staged entries will be carried over to the new
>>> kernel.
>>>
>>> Introduce ascii_runtime_measurements_<algo>_staged and
>>> binary_runtime_measurements_<algo>_staged interfaces to stage and delete
>>> the measurements. Use 'echo A > <IMA interface>' and
>>> 'echo D > <IMA interface>' to respectively stage and delete the entire
>>> measurements list. Locking of these interfaces is also mediated with a call
>>> to _ima_measurements_open() and with ima_measurements_release().
>>>
>>> Implement the staging functionality by introducing the new global
>>> measurements list ima_measurements_staged, and ima_queue_stage() and
>>> ima_queue_delete_staged_all() to respectively move measurements from the
>>> current measurements list to the staged one, and to move staged
>>> measurements to the ima_measurements_trim list for deletion. Introduce
>>> ima_queue_delete() to delete the measurements.
>>>
>>> Finally, introduce the BINARY_STAGED AND BINARY_FULL binary measurements
>>> list types, to maintain the counters and the binary size of staged
>>> measurements and the full measurements list (including entries that were
>>> staged). BINARY still represents the current binary measurements list.
>>>
>>> Use the binary size for the BINARY + BINARY_STAGED types in
>>> ima_add_kexec_buffer(), since both measurements list types are copied to
>>> the secondary kernel during kexec. Use BINARY_FULL in
>>> ima_measure_kexec_event(), to generate a critical data record.
>>>
>>> It should be noted that the BINARY_FULL counter is not passed through
>>> kexec. Thus, the number of entries included in the kexec critical data
>>> records refers to the entries since the previous kexec records.
>>>
>>> Note: This code derives from the Alt-IMA Huawei project, whose license is
>>> GPL-2.0 OR MIT.
>>>
>>> Link: https://github.com/linux-integrity/linux/issues/1
>>> Suggested-by: Gregory Lumen <gregorylumen@linux.microsoft.com> (staging revert)
>>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>>> ---
>>> security/integrity/ima/Kconfig | 13 +++
>>> security/integrity/ima/ima.h | 8 +-
>>> security/integrity/ima/ima_fs.c | 167 ++++++++++++++++++++++++++---
>>> security/integrity/ima/ima_kexec.c | 22 +++-
>>> security/integrity/ima/ima_queue.c | 97 ++++++++++++++++-
>>> 5 files changed, 286 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
>>> index 976e75f9b9ba..e714726f3384 100644
>>> --- a/security/integrity/ima/Kconfig
>>> +++ b/security/integrity/ima/Kconfig
>>> @@ -332,4 +332,17 @@ config IMA_KEXEC_EXTRA_MEMORY_KB
>>> If set to the default value of 0, an extra half page of memory for those
>>> additional measurements will be allocated.
>>>
>>> +config IMA_STAGING
>>> + bool "Support for staging the measurements list"
>>> + default y
>>> + help
>>> + Add support for staging the measurements list.
>>> +
>>> + It allows user space to stage the measurements list for deletion and
>>> + to delete the staged measurements after confirmation.
>>> +
>>> + On kexec, staging is reverted and staged measurements are prepended
>>> + to the current measurements list when measurements are copied to the
>>> + secondary kernel.
>>> +
>>> endif
>>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>>> index 97b7d6024b5d..65db152a0a24 100644
>>> --- a/security/integrity/ima/ima.h
>>> +++ b/security/integrity/ima/ima.h
>>> @@ -30,9 +30,11 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>>>
>>> /*
>>> * BINARY: current binary measurements list
>>> + * BINARY_STAGED: staged binary measurements list
>>> + * BINARY_FULL: binary measurements list since IMA init (lost after kexec)
>>> */
>>> enum binary_lists {
>>> - BINARY, BINARY__LAST
>>> + BINARY, BINARY_STAGED, BINARY_FULL, BINARY__LAST
>>> };
>>>
>>> /* digest size for IMA, fits SHA1 or MD5 */
>>> @@ -125,6 +127,7 @@ struct ima_queue_entry {
>>> struct ima_template_entry *entry;
>>> };
>>> extern struct list_head ima_measurements; /* list of all measurements */
>>> +extern struct list_head ima_measurements_staged; /* list of staged meas. */
>>>
>>> /* Some details preceding the binary serialized measurement list */
>>> struct ima_kexec_hdr {
>>> @@ -314,6 +317,8 @@ struct ima_template_desc *ima_template_desc_current(void);
>>> struct ima_template_desc *ima_template_desc_buf(void);
>>> struct ima_template_desc *lookup_template_desc(const char *name);
>>> bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
>>> +int ima_queue_stage(void);
>>> +int ima_queue_staged_delete_all(void);
>>> int ima_restore_measurement_entry(struct ima_template_entry *entry);
>>> int ima_restore_measurement_list(loff_t bufsize, void *buf);
>>> int ima_measurements_show(struct seq_file *m, void *v);
>>> @@ -334,6 +339,7 @@ extern spinlock_t ima_queue_lock;
>>> extern atomic_long_t ima_num_entries[BINARY__LAST];
>>> extern atomic_long_t ima_num_violations;
>>> extern struct hlist_head __rcu *ima_htable;
>>> +extern struct mutex ima_extend_list_mutex;
>>>
>>> static inline unsigned int ima_hash_key(u8 *digest)
>>> {
>>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
>>> index 7709a4576322..39d9128e9f22 100644
>>> --- a/security/integrity/ima/ima_fs.c
>>> +++ b/security/integrity/ima/ima_fs.c
>>> @@ -24,6 +24,13 @@
>>>
>>> #include "ima.h"
>>>
>>> +/*
>>> + * Requests:
>>> + * 'A\n': stage the entire measurements list
>>> + * 'D\n': delete all staged measurements
>>> + */
>>> +#define STAGED_REQ_LENGTH 21
>>> +
>>> static DEFINE_MUTEX(ima_write_mutex);
>>> static DEFINE_MUTEX(ima_measure_mutex);
>>> static long ima_measure_users;
>>> @@ -97,6 +104,11 @@ static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
>>> return _ima_measurements_start(m, pos, &ima_measurements);
>>> }
>>>
>>> +static void *ima_measurements_staged_start(struct seq_file *m, loff_t *pos)
>>> +{
>>> + return _ima_measurements_start(m, pos, &ima_measurements_staged);
>>> +}
>>> +
>>> static void *_ima_measurements_next(struct seq_file *m, void *v, loff_t *pos,
>>> struct list_head *head)
>>> {
>>> @@ -118,6 +130,12 @@ static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
>>> return _ima_measurements_next(m, v, pos, &ima_measurements);
>>> }
>>>
>>> +static void *ima_measurements_staged_next(struct seq_file *m, void *v,
>>> + loff_t *pos)
>>> +{
>>> + return _ima_measurements_next(m, v, pos, &ima_measurements_staged);
>>> +}
>>> +
>>> static void ima_measurements_stop(struct seq_file *m, void *v)
>>> {
>>> }
>>> @@ -283,6 +301,68 @@ static const struct file_operations ima_measurements_ops = {
>>> .release = ima_measurements_release,
>>> };
>>>
>>> +static const struct seq_operations ima_measurments_staged_seqops = {
>>> + .start = ima_measurements_staged_start,
>>> + .next = ima_measurements_staged_next,
>>> + .stop = ima_measurements_stop,
>>> + .show = ima_measurements_show
>>> +};
>>> +
>>> +static int ima_measurements_staged_open(struct inode *inode, struct file *file)
>>> +{
>>> + return _ima_measurements_open(inode, file,
>>> + &ima_measurments_staged_seqops);
>>> +}
>>> +
>>> +static ssize_t ima_measurements_staged_write(struct file *file,
>>> + const char __user *buf,
>>> + size_t datalen, loff_t *ppos)
>>> +{
>>> + char req[STAGED_REQ_LENGTH];
>>> + int ret;
>>> +
>>> + if (*ppos > 0 || datalen < 2 || datalen > STAGED_REQ_LENGTH)
>>> + return -EINVAL;
>>> +
>>> + if (copy_from_user(req, buf, datalen) != 0)
>>> + return -EFAULT;
>>> +
>>> + if (req[datalen - 1] != '\n')
>>> + return -EINVAL;
>>> +
>>> + req[datalen - 1] = '\0';
>>> +
>>> + switch (req[0]) {
>>> + case 'A':
>>> + if (datalen != 2)
>>> + return -EINVAL;
>>> +
>>> + ret = ima_queue_stage();
>>> + break;
>>> + case 'D':
>>> + if (datalen != 2)
>>> + return -EINVAL;
>>> +
>>> + ret = ima_queue_staged_delete_all();
>>> + break;
>> I think the following two steps may not work because of race condition:
>>
>> step1: ret = ima_queue_stage(); //this will put all logs in active list into staged list;
>> step2: ret = ima_queue_staged_delete_all(); //this will delete all logs in staged list;
>>
>> The following is the step of race condition:
>> 1. current active log list LA1;
>> 2. user agent read the TPM quote QA1 match list LA1;
>> 3. new event NewLog is added into active log list LA1+NewLog
>> 4. user agent call ima_queue_stage() and generated staged list
>> including LA1+NewLog.
>> 5. user agent call ima_queue_staged_delete_all();
>> The new log NewLog in step 3 is also deleted
> Please refer to the documentation patch which explains the intended
> workflow of this approach (Remote Attestation Agent Workflow).
>
> Roberto
So the user agent needs to deeply involve measurement list management:
in user space, user agent need to maintain two lists
user agent is more complicated in this case
Steven
>> Next time the attestation will fail if using the active log list in the
>> kernel.
>>
>> Thanks,
>>
>> Steven
>>
>>> + default:
>>> + ret = -EINVAL;
>>> + }
>>> +
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + return datalen;
>>> +}
>>> +
>>> +static const struct file_operations ima_measurements_staged_ops = {
>>> + .open = ima_measurements_staged_open,
>>> + .read = seq_read,
>>> + .write = ima_measurements_staged_write,
>>> + .llseek = seq_lseek,
>>> + .release = ima_measurements_release,
>>> +};
>>> +
>>> void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
>>> {
>>> u32 i;
>>> @@ -356,6 +436,28 @@ static const struct file_operations ima_ascii_measurements_ops = {
>>> .release = ima_measurements_release,
>>> };
>>>
>>> +static const struct seq_operations ima_ascii_measurements_staged_seqops = {
>>> + .start = ima_measurements_staged_start,
>>> + .next = ima_measurements_staged_next,
>>> + .stop = ima_measurements_stop,
>>> + .show = ima_ascii_measurements_show
>>> +};
>>> +
>>> +static int ima_ascii_measurements_staged_open(struct inode *inode,
>>> + struct file *file)
>>> +{
>>> + return _ima_measurements_open(inode, file,
>>> + &ima_ascii_measurements_staged_seqops);
>>> +}
>>> +
>>> +static const struct file_operations ima_ascii_measurements_staged_ops = {
>>> + .open = ima_ascii_measurements_staged_open,
>>> + .read = seq_read,
>>> + .write = ima_measurements_staged_write,
>>> + .llseek = seq_lseek,
>>> + .release = ima_measurements_release,
>>> +};
>>> +
>>> static ssize_t ima_read_policy(char *path)
>>> {
>>> void *data = NULL;
>>> @@ -459,10 +561,21 @@ static const struct seq_operations ima_policy_seqops = {
>>> };
>>> #endif
>>>
>>> -static int __init create_securityfs_measurement_lists(void)
>>> +static int __init create_securityfs_measurement_lists(bool staging)
>>> {
>>> + const struct file_operations *ascii_ops = &ima_ascii_measurements_ops;
>>> + const struct file_operations *binary_ops = &ima_measurements_ops;
>>> + mode_t permissions = S_IRUSR | S_IRGRP;
>>> + const char *file_suffix = "";
>>> int count = NR_BANKS(ima_tpm_chip);
>>>
>>> + if (staging) {
>>> + ascii_ops = &ima_ascii_measurements_staged_ops;
>>> + binary_ops = &ima_measurements_staged_ops;
>>> + file_suffix = "_staged";
>>> + permissions |= (S_IWUSR | S_IWGRP);
>>> + }
>>> +
>>> if (ima_sha1_idx >= NR_BANKS(ima_tpm_chip))
>>> count++;
>>>
>>> @@ -473,29 +586,32 @@ static int __init create_securityfs_measurement_lists(void)
>>>
>>> if (algo == HASH_ALGO__LAST)
>>> snprintf(file_name, sizeof(file_name),
>>> - "ascii_runtime_measurements_tpm_alg_%x",
>>> - ima_tpm_chip->allocated_banks[i].alg_id);
>>> + "ascii_runtime_measurements_tpm_alg_%x%s",
>>> + ima_tpm_chip->allocated_banks[i].alg_id,
>>> + file_suffix);
>>> else
>>> snprintf(file_name, sizeof(file_name),
>>> - "ascii_runtime_measurements_%s",
>>> - hash_algo_name[algo]);
>>> - dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
>>> + "ascii_runtime_measurements_%s%s",
>>> + hash_algo_name[algo], file_suffix);
>>> + dentry = securityfs_create_file(file_name, permissions,
>>> ima_dir, (void *)(uintptr_t)i,
>>> - &ima_ascii_measurements_ops);
>>> + ascii_ops);
>>> if (IS_ERR(dentry))
>>> return PTR_ERR(dentry);
>>>
>>> if (algo == HASH_ALGO__LAST)
>>> snprintf(file_name, sizeof(file_name),
>>> - "binary_runtime_measurements_tpm_alg_%x",
>>> - ima_tpm_chip->allocated_banks[i].alg_id);
>>> + "binary_runtime_measurements_tpm_alg_%x%s",
>>> + ima_tpm_chip->allocated_banks[i].alg_id,
>>> + file_suffix);
>>> else
>>> snprintf(file_name, sizeof(file_name),
>>> - "binary_runtime_measurements_%s",
>>> - hash_algo_name[algo]);
>>> - dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
>>> + "binary_runtime_measurements_%s%s",
>>> + hash_algo_name[algo], file_suffix);
>>> +
>>> + dentry = securityfs_create_file(file_name, permissions,
>>> ima_dir, (void *)(uintptr_t)i,
>>> - &ima_measurements_ops);
>>> + binary_ops);
>>> if (IS_ERR(dentry))
>>> return PTR_ERR(dentry);
>>> }
>>> @@ -503,6 +619,23 @@ static int __init create_securityfs_measurement_lists(void)
>>> return 0;
>>> }
>>>
>>> +static int __init create_securityfs_staging_links(void)
>>> +{
>>> + struct dentry *dentry;
>>> +
>>> + dentry = securityfs_create_symlink("binary_runtime_measurements_staged",
>>> + ima_dir, "binary_runtime_measurements_sha1_staged", NULL);
>>> + if (IS_ERR(dentry))
>>> + return PTR_ERR(dentry);
>>> +
>>> + dentry = securityfs_create_symlink("ascii_runtime_measurements_staged",
>>> + ima_dir, "ascii_runtime_measurements_sha1_staged", NULL);
>>> + if (IS_ERR(dentry))
>>> + return PTR_ERR(dentry);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> /*
>>> * ima_open_policy: sequentialize access to the policy file
>>> */
>>> @@ -595,7 +728,13 @@ int __init ima_fs_init(void)
>>> goto out;
>>> }
>>>
>>> - ret = create_securityfs_measurement_lists();
>>> + ret = create_securityfs_measurement_lists(false);
>>> + if (ret == 0 && IS_ENABLED(CONFIG_IMA_STAGING)) {
>>> + ret = create_securityfs_measurement_lists(true);
>>> + if (ret == 0)
>>> + ret = create_securityfs_staging_links();
>>> + }
>>> +
>>> if (ret != 0)
>>> goto out;
>>>
>>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
>>> index d7d0fb639d99..d5503dd5cc9b 100644
>>> --- a/security/integrity/ima/ima_kexec.c
>>> +++ b/security/integrity/ima/ima_kexec.c
>>> @@ -42,8 +42,8 @@ void ima_measure_kexec_event(const char *event_name)
>>> long len;
>>> int n;
>>>
>>> - buf_size = ima_get_binary_runtime_size(BINARY);
>>> - len = atomic_long_read(&ima_num_entries[BINARY]);
>>> + buf_size = ima_get_binary_runtime_size(BINARY_FULL);
>>> + len = atomic_long_read(&ima_num_entries[BINARY_FULL]);
>>>
>>> n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
>>> "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
>>> @@ -106,13 +106,26 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
>>>
>>> memset(&khdr, 0, sizeof(khdr));
>>> khdr.version = 1;
>>> - /* This is an append-only list, no need to hold the RCU read lock */
>>> - list_for_each_entry_rcu(qe, &ima_measurements, later, true) {
>>> + /* It can race with ima_queue_stage() and ima_queue_delete_staged(). */
>>> + mutex_lock(&ima_extend_list_mutex);
>>> +
>>> + list_for_each_entry_rcu(qe, &ima_measurements_staged, later,
>>> + lockdep_is_held(&ima_extend_list_mutex)) {
>>> ret = ima_dump_measurement(&khdr, qe);
>>> if (ret < 0)
>>> break;
>>> }
>>>
>>> + list_for_each_entry_rcu(qe, &ima_measurements, later,
>>> + lockdep_is_held(&ima_extend_list_mutex)) {
>>> + if (!ret)
>>> + ret = ima_dump_measurement(&khdr, qe);
>>> + if (ret < 0)
>>> + break;
>>> + }
>>> +
>>> + mutex_unlock(&ima_extend_list_mutex);
>>> +
>>> /*
>>> * fill in reserved space with some buffer details
>>> * (eg. version, buffer size, number of measurements)
>>> @@ -167,6 +180,7 @@ void ima_add_kexec_buffer(struct kimage *image)
>>> extra_memory = CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB * 1024;
>>>
>>> binary_runtime_size = ima_get_binary_runtime_size(BINARY) +
>>> + ima_get_binary_runtime_size(BINARY_STAGED) +
>>> extra_memory;
>>>
>>> if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
>>> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
>>> index b6d10dceb669..50519ed837d4 100644
>>> --- a/security/integrity/ima/ima_queue.c
>>> +++ b/security/integrity/ima/ima_queue.c
>>> @@ -26,6 +26,7 @@
>>> static struct tpm_digest *digests;
>>>
>>> LIST_HEAD(ima_measurements); /* list of all measurements */
>>> +LIST_HEAD(ima_measurements_staged); /* list of staged measurements */
>>> #ifdef CONFIG_IMA_KEXEC
>>> static unsigned long binary_runtime_size[BINARY__LAST];
>>> #else
>>> @@ -45,11 +46,11 @@ atomic_long_t ima_num_violations = ATOMIC_LONG_INIT(0);
>>> /* key: inode (before secure-hashing a file) */
>>> struct hlist_head __rcu *ima_htable;
>>>
>>> -/* mutex protects atomicity of extending measurement list
>>> +/* mutex protects atomicity of extending and staging measurement list
>>> * and extending the TPM PCR aggregate. Since tpm_extend can take
>>> * long (and the tpm driver uses a mutex), we can't use the spinlock.
>>> */
>>> -static DEFINE_MUTEX(ima_extend_list_mutex);
>>> +DEFINE_MUTEX(ima_extend_list_mutex);
>>>
>>> /*
>>> * Used internally by the kernel to suspend measurements.
>>> @@ -174,12 +175,16 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
>>> lockdep_is_held(&ima_extend_list_mutex));
>>>
>>> atomic_long_inc(&ima_num_entries[BINARY]);
>>> + atomic_long_inc(&ima_num_entries[BINARY_FULL]);
>>> +
>>> if (update_htable) {
>>> key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
>>> hlist_add_head_rcu(&qe->hnext, &htable[key]);
>>> }
>>>
>>> ima_update_binary_runtime_size(entry, BINARY);
>>> + ima_update_binary_runtime_size(entry, BINARY_FULL);
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -280,6 +285,94 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>>> return result;
>>> }
>>>
>>> +int ima_queue_stage(void)
>>> +{
>>> + int ret = 0;
>>> +
>>> + mutex_lock(&ima_extend_list_mutex);
>>> + if (!list_empty(&ima_measurements_staged)) {
>>> + ret = -EEXIST;
>>> + goto out_unlock;
>>> + }
>>> +
>>> + if (list_empty(&ima_measurements)) {
>>> + ret = -ENOENT;
>>> + goto out_unlock;
>>> + }
>>> +
>>> + list_replace(&ima_measurements, &ima_measurements_staged);
>>> + INIT_LIST_HEAD(&ima_measurements);
>>> +
>>> + atomic_long_set(&ima_num_entries[BINARY_STAGED],
>>> + atomic_long_read(&ima_num_entries[BINARY]));
>>> + atomic_long_set(&ima_num_entries[BINARY], 0);
>>> +
>>> + if (IS_ENABLED(CONFIG_IMA_KEXEC)) {
>>> + binary_runtime_size[BINARY_STAGED] =
>>> + binary_runtime_size[BINARY];
>>> + binary_runtime_size[BINARY] = 0;
>>> + }
>>> +out_unlock:
>>> + mutex_unlock(&ima_extend_list_mutex);
>>> + return ret;
>>> +}
>>> +
>>> +static void ima_queue_delete(struct list_head *head);
>>> +
>>> +int ima_queue_staged_delete_all(void)
>>> +{
>>> + LIST_HEAD(ima_measurements_trim);
>>> +
>>> + mutex_lock(&ima_extend_list_mutex);
>>> + if (list_empty(&ima_measurements_staged)) {
>>> + mutex_unlock(&ima_extend_list_mutex);
>>> + return -ENOENT;
>>> + }
>>> +
>>> + list_replace(&ima_measurements_staged, &ima_measurements_trim);
>>> + INIT_LIST_HEAD(&ima_measurements_staged);
>>> +
>>> + atomic_long_set(&ima_num_entries[BINARY_STAGED], 0);
>>> +
>>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>>> + binary_runtime_size[BINARY_STAGED] = 0;
>>> +
>>> + mutex_unlock(&ima_extend_list_mutex);
>>> +
>>> + ima_queue_delete(&ima_measurements_trim);
>>> + return 0;
>>> +}
>>> +
>>> +static void ima_queue_delete(struct list_head *head)
>>> +{
>>> + struct ima_queue_entry *qe, *qe_tmp;
>>> + unsigned int i;
>>> +
>>> + list_for_each_entry_safe(qe, qe_tmp, head, later) {
>>> + /*
>>> + * Safe to free template_data here without synchronize_rcu()
>>> + * because the only htable reader, ima_lookup_digest_entry(),
>>> + * accesses only entry->digests, not template_data. If new
>>> + * htable readers are added that access template_data, a
>>> + * synchronize_rcu() is required here.
>>> + */
>>> + for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
>>> + kfree(qe->entry->template_data[i].data);
>>> + qe->entry->template_data[i].data = NULL;
>>> + qe->entry->template_data[i].len = 0;
>>> + }
>>> +
>>> + list_del(&qe->later);
>>> +
>>> + /* No leak if condition is false, referenced by ima_htable. */
>>> + if (IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE)) {
>>> + kfree(qe->entry->digests);
>>> + kfree(qe->entry);
>>> + kfree(qe);
>>> + }
>>> + }
>>> +}
>>> +
>>> int ima_restore_measurement_entry(struct ima_template_entry *entry)
>>> {
>>> int result = 0;
^ permalink raw reply
* Re: [PATCH v4 11/13] ima: Support staging and deleting N measurements entries
From: steven chen @ 2026-04-01 17:48 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, zohar, dmitry.kasatkin,
eric.snowberg, paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, nramas, Roberto Sassu, steven chen
In-Reply-To: <af6aa732b85af36e07e4a82b29170e80b13dc7c4.camel@huaweicloud.com>
On 3/27/2026 10:02 AM, Roberto Sassu wrote:
> On Thu, 2026-03-26 at 16:19 -0700, steven chen wrote:
>> On 3/26/2026 10:30 AM, Roberto Sassu wrote:
>>> From: Roberto Sassu <roberto.sassu@huawei.com>
>>>
>>> Add support for sending a value N between 1 and ULONG_MAX to the staging
>>> interface. This value represents the number of measurements that should be
>>> deleted from the current measurements list.
>>>
>>> This staging method allows the remote attestation agents to easily separate
>>> the measurements that were verified (staged and deleted) from those that
>>> weren't due to the race between taking a TPM quote and reading the
>>> measurements list.
>>>
>>> In order to minimize the locking time of ima_extend_list_mutex, deleting
>>> N entries is realized by staging the entire current measurements list
>>> (with the lock), by determining the N-th staged entry (without the lock),
>>> and by splicing the entries in excess back to the current measurements list
>>> (with the lock). Finally, the N entries are deleted (without the lock).
>>>
>>> Flushing the hash table is not supported for N entries, since it would
>>> require removing the N entries one by one from the hash table under the
>>> ima_extend_list_mutex lock, which would increase the locking time.
>>>
>>> The ima_extend_list_mutex lock is necessary in ima_dump_measurement_list()
>>> because ima_queue_staged_delete_partial() uses __list_cut_position() to
>>> modify ima_measurements_staged, for which no RCU-safe variant exists. For
>>> the staging with prompt flavor alone, list_replace_rcu() could have been
>>> used instead, but since both flavors share the same kexec serialization
>>> path, the mutex is required regardless.
>>>
>>> Link: https://github.com/linux-integrity/linux/issues/1
>>> Suggested-by: Steven Chen <chenste@linux.microsoft.com>
>>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>>> ---
>>> security/integrity/ima/Kconfig | 3 ++
>>> security/integrity/ima/ima.h | 1 +
>>> security/integrity/ima/ima_fs.c | 22 +++++++++-
>>> security/integrity/ima/ima_queue.c | 70 ++++++++++++++++++++++++++++++
>>> 4 files changed, 95 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
>>> index e714726f3384..6ddb4e77bff5 100644
>>> --- a/security/integrity/ima/Kconfig
>>> +++ b/security/integrity/ima/Kconfig
>>> @@ -341,6 +341,9 @@ config IMA_STAGING
>>> It allows user space to stage the measurements list for deletion and
>>> to delete the staged measurements after confirmation.
>>>
>>> + Or, alternatively, it allows user space to specify N measurements
>>> + entries to be deleted.
>>> +
>>> On kexec, staging is reverted and staged measurements are prepended
>>> to the current measurements list when measurements are copied to the
>>> secondary kernel.
>>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>>> index 699b735dec7d..de0693fce53c 100644
>>> --- a/security/integrity/ima/ima.h
>>> +++ b/security/integrity/ima/ima.h
>>> @@ -319,6 +319,7 @@ struct ima_template_desc *lookup_template_desc(const char *name);
>>> bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
>>> int ima_queue_stage(void);
>>> int ima_queue_staged_delete_all(void);
>>> +int ima_queue_staged_delete_partial(unsigned long req_value);
>>> int ima_restore_measurement_entry(struct ima_template_entry *entry);
>>> int ima_restore_measurement_list(loff_t bufsize, void *buf);
>>> int ima_measurements_show(struct seq_file *m, void *v);
>>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
>>> index 39d9128e9f22..eb3f343c1138 100644
>>> --- a/security/integrity/ima/ima_fs.c
>>> +++ b/security/integrity/ima/ima_fs.c
>>> @@ -28,6 +28,7 @@
>>> * Requests:
>>> * 'A\n': stage the entire measurements list
>>> * 'D\n': delete all staged measurements
>>> + * '[1, ULONG_MAX]\n' delete N measurements entries
>>> */
>>> #define STAGED_REQ_LENGTH 21
>>>
>>> @@ -319,6 +320,7 @@ static ssize_t ima_measurements_staged_write(struct file *file,
>>> size_t datalen, loff_t *ppos)
>>> {
>>> char req[STAGED_REQ_LENGTH];
>>> + unsigned long req_value;
>>> int ret;
>>>
>>> if (*ppos > 0 || datalen < 2 || datalen > STAGED_REQ_LENGTH)
>>> @@ -346,7 +348,25 @@ static ssize_t ima_measurements_staged_write(struct file *file,
>>> ret = ima_queue_staged_delete_all();
>>> break;
>>> default:
>>> - ret = -EINVAL;
>>> + if (ima_flush_htable) {
>>> + pr_debug("Deleting staged N measurements not supported when flushing the hash table is requested\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = kstrtoul(req, 10, &req_value);
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + if (req_value == 0) {
>>> + pr_debug("Must delete at least one entry\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = ima_queue_stage();
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + ret = ima_queue_staged_delete_partial(req_value);
>> The default processing is "Trim N" idea plus performance improvement.
>>
>> Here do everything in one time. And this is what I said in v3.
>>
>> [PATCH v3 1/3] ima: Remove ima_h_table structure
>> <https://lore.kernel.org/linux-integrity/c61aeaa79929a98cb3a6d30835972891fac3570f.camel@linux.ibm.com/T/#t>
> In your approach you do:
>
> lock ima_extend_measure_list_mutex
> scan entries until N
> cut list staged -> trim
> unlock ima_extend_measure_list_mutex
>
>
> In my approach I do:
> lock ima_extend_measure_list_mutex
> list replace active -> staged
> unlock ima_extend_measure_list_mutex
>
> scan entries until N
>
> lock ima_extend_measure_list_mutex
> cut list staged -> trim
> splice staged ->active
> unlock ima_extend_measure_list_mutex
>
> So, I guess if you refer to less user space locking time, you mean one
> lock/unlock and one list replace + list splice less in your solution.
>
> In exchange, you propose to hold the lock in the kernel while scanning
> N. I think it is a significant increase of kernel locking time vs a
> negligible increase of user space locking time (in the kernel, all
> processes need to wait for the ima_extend_measure_list_mutex to be
> released, in user space it is just the agent waiting).
>
> Roberto
Please the version 5:
[PATCH v5 0/3] Trim N entries of IMA event logs
<https://lore.kernel.org/linux-integrity/20260401172956.4581-1-chenste@linux.microsoft.com/T/#t>
Scanning N is moved out of the lock period.
"Trim N" proposal has less lock time than "Staging and deleting" proposal.
"Trim N" proposal has much less code than "Staging and deleting" proposal.
"Trim N" proposal user space operation is more simple than "Staging and
deleting".
Steven
>> The important two parts of trimming is "trim N" and performance improvement.
>>
>> The performance improvement include two parts:
>> hash table staging
>> active log list staging
>>
>> And I think "Trim N" plus performance improvement is the right direction
>> to go.
>> Lots of code for two steps "stage and trim" "stage" part can be removed.
>>
>> Also race condition may happen if not holding the list all time in user
>> space
>> during attestation period: from stage, read list, attestation and trimming.
>>
>> So in order to improve the above user space lock time, "Trim T:N" can be
>> used
>> not to hold list long in user space during attestation.
>>
>> For Trim T:N, T represent total log trimmed since system boot up. Please
>> refer to
>> https://lore.kernel.org/linux-integrity/20260205235849.7086-1-chenste@linux.microsoft.com/T/#t
>>
>> Thanks,
>>
>> Steven
>>> }
>>>
>>> if (ret < 0)
>>> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
>>> index f5c18acfbc43..4fb557d61a88 100644
>>> --- a/security/integrity/ima/ima_queue.c
>>> +++ b/security/integrity/ima/ima_queue.c
>>> @@ -371,6 +371,76 @@ int ima_queue_staged_delete_all(void)
>>> return 0;
>>> }
>>>
>>> +int ima_queue_staged_delete_partial(unsigned long req_value)
>>> +{
>>> + unsigned long req_value_copy = req_value;
>>> + unsigned long size_to_remove = 0, num_to_remove = 0;
>>> + struct list_head *cut_pos = NULL;
>>> + LIST_HEAD(ima_measurements_trim);
>>> + struct ima_queue_entry *qe;
>>> + int ret = 0;
>>> +
>>> + /*
>>> + * Safe walk (no concurrent write), not under ima_extend_list_mutex
>>> + * for performance reasons.
>>> + */
>>> + list_for_each_entry(qe, &ima_measurements_staged, later) {
>>> + size_to_remove += get_binary_runtime_size(qe->entry);
>>> + num_to_remove++;
>>> +
>>> + if (--req_value_copy == 0) {
>>> + /* qe->later always points to a valid list entry. */
>>> + cut_pos = &qe->later;
>>> + break;
>>> + }
>>> + }
>>> +
>>> + /* Nothing to remove, undoing staging. */
>>> + if (req_value_copy > 0) {
>>> + size_to_remove = 0;
>>> + num_to_remove = 0;
>>> + ret = -ENOENT;
>>> + }
>>> +
>>> + mutex_lock(&ima_extend_list_mutex);
>>> + if (list_empty(&ima_measurements_staged)) {
>>> + mutex_unlock(&ima_extend_list_mutex);
>>> + return -ENOENT;
>>> + }
>>> +
>>> + if (cut_pos != NULL)
>>> + /*
>>> + * ima_dump_measurement_list() does not modify the list,
>>> + * cut_pos remains the same even if it was computed before
>>> + * the lock.
>>> + */
>>> + __list_cut_position(&ima_measurements_trim,
>>> + &ima_measurements_staged, cut_pos);
>>> +
>>> + atomic_long_sub(num_to_remove, &ima_num_entries[BINARY_STAGED]);
>>> + atomic_long_add(atomic_long_read(&ima_num_entries[BINARY_STAGED]),
>>> + &ima_num_entries[BINARY]);
>>> + atomic_long_set(&ima_num_entries[BINARY_STAGED], 0);
>>> +
>>> + if (IS_ENABLED(CONFIG_IMA_KEXEC)) {
>>> + binary_runtime_size[BINARY_STAGED] -= size_to_remove;
>>> + binary_runtime_size[BINARY] +=
>>> + binary_runtime_size[BINARY_STAGED];
>>> + binary_runtime_size[BINARY_STAGED] = 0;
>>> + }
>>> +
>>> + /*
>>> + * Splice (prepend) any remaining non-deleted staged entries to the
>>> + * active list (RCU not needed, there cannot be concurrent readers).
>>> + */
>>> + list_splice(&ima_measurements_staged, &ima_measurements);
>>> + INIT_LIST_HEAD(&ima_measurements_staged);
>>> + mutex_unlock(&ima_extend_list_mutex);
>>> +
>>> + ima_queue_delete(&ima_measurements_trim, false);
>>> + return ret;
>>> +}
>>> +
>>> static void ima_queue_delete(struct list_head *head, bool flush_htable)
>>> {
>>> struct ima_queue_entry *qe, *qe_tmp;
^ permalink raw reply
* [PATCH v5 3/3] ima: add new critical data record to measure log trim
From: steven chen @ 2026-04-01 17:29 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve, chenste,
gregorylumen, nramas, sushring, linux-doc
In-Reply-To: <20260401172956.4581-1-chenste@linux.microsoft.com>
Add a new critical data record to measure the trimming event when
ima event records are deleted since system boot up.
If all IMA event logs are saved in the userspace, use this log to get total
numbers of records deleted since system boot up at that point.
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
security/integrity/ima/ima_fs.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 8e26e0f34311..38d0a49b587f 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -43,6 +43,7 @@ static int valid_policy = 1;
#define IMA_LOG_TRIM_REQ_NUM_LENGTH 15
#define IMA_LOG_TRIM_REQ_TOTAL_LENGTH 32
+#define IMA_LOG_TRIM_EVENT_LEN 256
atomic_long_t ima_number_entries = ATOMIC_LONG_INIT(0);
static long trimcount;
/* mutex protects atomicity of trimming measurement list
@@ -52,6 +53,22 @@ static long trimcount;
static DEFINE_MUTEX(ima_measure_lock);
static long ima_measure_users;
+static void ima_measure_trim_event(void)
+{
+ char ima_log_trim_event[IMA_LOG_TRIM_EVENT_LEN];
+ struct timespec64 ts;
+ u64 time_ns;
+ int n;
+
+ ktime_get_real_ts64(&ts);
+ time_ns = (u64)ts.tv_sec * 1000000000ULL + ts.tv_nsec;
+ n = scnprintf(ima_log_trim_event, IMA_LOG_TRIM_EVENT_LEN,
+ "time= %llu; number= %lu;", time_ns, trimcount);
+
+ ima_measure_critical_data("ima_log_trim", "trim ima event logs",
+ ima_log_trim_event, n, false, NULL, 0);
+}
+
static ssize_t ima_show_htable_value(char __user *buf, size_t count,
loff_t *ppos, atomic_long_t *val)
{
@@ -436,6 +453,9 @@ static ssize_t ima_log_trim_write(struct file *file,
if (ret < 0)
goto out;
+ if (ret > 0)
+ ima_measure_trim_event();
+
trimcount += ret;
ret = datalen;
--
2.43.0
^ permalink raw reply related
* [PATCH v5 2/3] ima: trim N IMA event log records
From: steven chen @ 2026-04-01 17:29 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve, chenste,
gregorylumen, nramas, sushring, linux-doc
In-Reply-To: <20260401172956.4581-1-chenste@linux.microsoft.com>
Trim N entries of the IMA event logs. Do not clean the hash table.
The values saved in hash table were already used.
Provide a userspace interface ima_trim_log:
When read this interface, it returns total number T of entries trimmed
since system boot up.
When write to this interface need to provide two numbers T:N to let
kernel to trim N entries of IMA event logs.
Kernel measurement list lock time performance improvement by not
clean the hash table.
when kernel get log trim request T:N
- Get the T, compare with the total trimmed number
- if equal, then do trim N and change T to T+N
- else return error
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
.../admin-guide/kernel-parameters.txt | 4 +
security/integrity/ima/ima.h | 4 +-
security/integrity/ima/ima_fs.c | 198 +++++++++++++++++-
security/integrity/ima/ima_kexec.c | 2 +-
security/integrity/ima/ima_queue.c | 96 +++++++++
5 files changed, 296 insertions(+), 8 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index e92c0056e4e0..cd1a1d0bf0e2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2197,6 +2197,10 @@
Use the canonical format for the binary runtime
measurements, instead of host native format.
+ ima_flush_htable [IMA]
+ Flush the measurement list hash table when trim all
+ or a part of it for deletion.
+
ima_hash= [IMA]
Format: { md5 | sha1 | rmd160 | sha256 | sha384
| sha512 | ... }
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e3d71d8d56e3..5cbee3a295a0 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -243,11 +243,13 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
const void *payload, size_t plen,
unsigned long flags, bool create);
#endif
-
+extern atomic_long_t ima_number_entries;
#ifdef CONFIG_IMA_KEXEC
void ima_measure_kexec_event(const char *event_name);
+long ima_delete_event_log(long req_val);
#else
static inline void ima_measure_kexec_event(const char *event_name) {}
+static inline long ima_delete_event_log(long req_val) { return 0; }
#endif
/*
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 87045b09f120..8e26e0f34311 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -21,6 +21,9 @@
#include <linux/rcupdate.h>
#include <linux/parser.h>
#include <linux/vmalloc.h>
+#include <linux/ktime.h>
+#include <linux/timekeeping.h>
+#include <linux/ima.h>
#include "ima.h"
@@ -38,6 +41,17 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
static int valid_policy = 1;
+#define IMA_LOG_TRIM_REQ_NUM_LENGTH 15
+#define IMA_LOG_TRIM_REQ_TOTAL_LENGTH 32
+atomic_long_t ima_number_entries = ATOMIC_LONG_INIT(0);
+static long trimcount;
+/* mutex protects atomicity of trimming measurement list
+ * and also protects atomicity the measurement list read
+ * write operation.
+ */
+static DEFINE_MUTEX(ima_measure_lock);
+static long ima_measure_users;
+
static ssize_t ima_show_htable_value(char __user *buf, size_t count,
loff_t *ppos, atomic_long_t *val)
{
@@ -64,8 +78,7 @@ static ssize_t ima_show_measurements_count(struct file *filp,
char __user *buf,
size_t count, loff_t *ppos)
{
- return ima_show_htable_value(buf, count, ppos, &ima_htable.len);
-
+ return ima_show_htable_value(buf, count, ppos, &ima_number_entries);
}
static const struct file_operations ima_measurements_count_ops = {
@@ -202,16 +215,77 @@ static const struct seq_operations ima_measurments_seqops = {
.show = ima_measurements_show
};
+/*
+ * _ima_measurements_open - open the IMA measurements file
+ * @inode: inode of the file being opened
+ * @file: file being opened
+ * @seq_ops: sequence operations for the file
+ *
+ * Returns 0 on success, or negative error code.
+ * Implements mutual exclusion between readers and writer
+ * of the measurements file. Multiple readers are allowed,
+ * but writer get exclusive access only no other readers/writers.
+ * Readers is not allowed when there is a writer.
+ */
+static int _ima_measurements_open(struct inode *inode, struct file *file,
+ const struct seq_operations *seq_ops)
+{
+ bool write = !!(file->f_mode & FMODE_WRITE);
+ int ret;
+
+ if (write && !capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ mutex_lock(&ima_measure_lock);
+ if ((write && ima_measure_users != 0) ||
+ (!write && ima_measure_users < 0)) {
+ mutex_unlock(&ima_measure_lock);
+ return -EBUSY;
+ }
+
+ ret = seq_open(file, seq_ops);
+ if (ret < 0) {
+ mutex_unlock(&ima_measure_lock);
+ return ret;
+ }
+
+ if (write)
+ ima_measure_users--;
+ else
+ ima_measure_users++;
+
+ mutex_unlock(&ima_measure_lock);
+ return ret;
+}
+
static int ima_measurements_open(struct inode *inode, struct file *file)
{
- return seq_open(file, &ima_measurments_seqops);
+ return _ima_measurements_open(inode, file, &ima_measurments_seqops);
+}
+
+static int ima_measurements_release(struct inode *inode, struct file *file)
+{
+ bool write = !!(file->f_mode & FMODE_WRITE);
+ int ret;
+
+ mutex_lock(&ima_measure_lock);
+ ret = seq_release(inode, file);
+ if (!ret) {
+ if (!write)
+ ima_measure_users--;
+ else
+ ima_measure_users++;
+ }
+
+ mutex_unlock(&ima_measure_lock);
+ return ret;
}
static const struct file_operations ima_measurements_ops = {
.open = ima_measurements_open,
.read = seq_read,
.llseek = seq_lseek,
- .release = seq_release,
+ .release = ima_measurements_release,
};
void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
@@ -279,14 +353,114 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
{
- return seq_open(file, &ima_ascii_measurements_seqops);
+ return _ima_measurements_open(inode, file, &ima_ascii_measurements_seqops);
}
static const struct file_operations ima_ascii_measurements_ops = {
.open = ima_ascii_measurements_open,
.read = seq_read,
.llseek = seq_lseek,
- .release = seq_release,
+ .release = ima_measurements_release,
+};
+
+static int ima_log_trim_open(struct inode *inode, struct file *file)
+{
+ bool write = !!(file->f_mode & FMODE_WRITE);
+
+ if (!write && capable(CAP_SYS_ADMIN))
+ return 0;
+ else if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ return _ima_measurements_open(inode, file, &ima_measurments_seqops);
+}
+
+static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
+{
+ char tmpbuf[IMA_LOG_TRIM_REQ_NUM_LENGTH];
+ ssize_t len;
+
+ len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
+ return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
+}
+
+static ssize_t ima_log_trim_write(struct file *file,
+ const char __user *buf, size_t datalen, loff_t *ppos)
+{
+ char tmpbuf[IMA_LOG_TRIM_REQ_TOTAL_LENGTH];
+ char *p = tmpbuf;
+ long count, ret, val = 0, max = LONG_MAX;
+
+ if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_TOTAL_LENGTH || datalen < 2) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (copy_from_user(tmpbuf, buf, datalen) != 0) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ p = tmpbuf;
+
+ while (*p && *p != ':') {
+ if (!isdigit((unsigned char)*p))
+ return -EINVAL;
+
+ /* digit value */
+ int d = *p - '0';
+
+ /* overflow check: val * 10 + d > max -> (val > (max - d) / 10) */
+ if (val > (max - d) / 10)
+ return -ERANGE;
+
+ val = val * 10 + d;
+ p++;
+ }
+
+ if (*p != ':')
+ return -EINVAL;
+
+ /* verify trim count matches */
+ if (val != trimcount)
+ return -EINVAL;
+
+ p++; /* skip ':' */
+ ret = kstrtoul(p, 0, &count);
+
+ if (ret < 0)
+ goto out;
+
+ ret = ima_delete_event_log(count);
+
+ if (ret < 0)
+ goto out;
+
+ trimcount += ret;
+
+ ret = datalen;
+out:
+ return ret;
+}
+
+static int ima_log_trim_release(struct inode *inode, struct file *file)
+{
+ bool write = !!(file->f_mode & FMODE_WRITE);
+
+ if (!write && capable(CAP_SYS_ADMIN))
+ return 0;
+ else if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ return ima_measurements_release(inode, file);
+}
+
+static const struct file_operations ima_log_trim_ops = {
+ .open = ima_log_trim_open,
+ .read = ima_log_trim_read,
+ .write = ima_log_trim_write,
+ .llseek = generic_file_llseek,
+ .release = ima_log_trim_release
};
static ssize_t ima_read_policy(char *path)
@@ -528,6 +702,18 @@ int __init ima_fs_init(void)
goto out;
}
+ if (IS_ENABLED(CONFIG_IMA_LOG_TRIMMING)) {
+ dentry = securityfs_create_file("ima_trim_log",
+ S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
+ ima_dir, NULL, &ima_log_trim_ops);
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
+ goto out;
+ }
+ }
+
+ trimcount = 0;
+
dentry = securityfs_create_file("runtime_measurements_count",
S_IRUSR | S_IRGRP, ima_dir, NULL,
&ima_measurements_count_ops);
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 7362f68f2d8b..bee997683e03 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -41,7 +41,7 @@ void ima_measure_kexec_event(const char *event_name)
int n;
buf_size = ima_get_binary_runtime_size();
- len = atomic_long_read(&ima_htable.len);
+ len = atomic_long_read(&ima_number_entries);
n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
"kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 590637e81ad1..07225e19b9b5 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -22,6 +22,14 @@
#define AUDIT_CAUSE_LEN_MAX 32
+bool ima_flush_htable;
+static int __init ima_flush_htable_setup(char *str)
+{
+ ima_flush_htable = true;
+ return 1;
+}
+__setup("ima_flush_htable", ima_flush_htable_setup);
+
/* pre-allocated array of tpm_digest structures to extend a PCR */
static struct tpm_digest *digests;
@@ -114,6 +122,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
list_add_tail_rcu(&qe->later, &ima_measurements);
atomic_long_inc(&ima_htable.len);
+ atomic_long_inc(&ima_number_entries);
if (update_htable) {
key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
@@ -220,6 +229,93 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
return result;
}
+/**
+ * ima_delete_event_log - delete IMA event entry
+ * @num_records: number of records to delete
+ *
+ * delete num_records entries off the measurement list.
+ * Returns num_records, or negative error code.
+ */
+long ima_delete_event_log(long num_records)
+{
+ long len, cur = num_records, tmp_len = 0;
+ struct ima_queue_entry *qe, *qe_tmp;
+ LIST_HEAD(ima_measurements_to_delete);
+ struct list_head *list_ptr;
+
+ if (!IS_ENABLED(CONFIG_IMA_LOG_TRIMMING))
+ return -EOPNOTSUPP;
+
+ if (num_records <= 0)
+ return num_records;
+
+ list_ptr = &ima_measurements;
+
+ len = atomic_long_read(&ima_number_entries);
+
+ if (num_records <= len) {
+ list_for_each_entry(qe, list_ptr, later) {
+ if (cur > 0) {
+ tmp_len += get_binary_runtime_size(qe->entry);
+ --cur;
+ }
+ if (cur == 0) {
+ qe_tmp = qe;
+ break;
+ }
+ }
+ }
+ else {
+ return -ENOENT;
+ }
+
+
+ mutex_lock(&ima_extend_list_mutex);
+ len = atomic_long_read(&ima_number_entries);
+
+ if (num_records == len) {
+ list_replace(&ima_measurements, &ima_measurements_to_delete);
+ INIT_LIST_HEAD(&ima_measurements);
+ atomic_long_set(&ima_number_entries, 0);
+ list_ptr = &ima_measurements_to_delete;
+ }
+ else {
+ __list_cut_position(&ima_measurements_to_delete, &ima_measurements,
+ &qe_tmp->later);
+ atomic_long_sub(num_records, &ima_number_entries);
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ binary_runtime_size -= tmp_len;
+ }
+
+ mutex_unlock(&ima_extend_list_mutex);
+
+ if (ima_flush_htable)
+ synchronize_rcu();
+
+ list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_to_delete, later) {
+ /*
+ * Ok because after list delete qe is only accessed by
+ * ima_lookup_digest_entry().
+ */
+ for (int i = 0; i < qe->entry->template_desc->num_fields; i++) {
+ kfree(qe->entry->template_data[i].data);
+ qe->entry->template_data[i].data = NULL;
+ qe->entry->template_data[i].len = 0;
+ }
+
+ list_del(&qe->later);
+
+ /* No leak if !ima_flush_htable, referenced by ima_htable. */
+ if (ima_flush_htable) {
+ kfree(qe->entry->digests);
+ kfree(qe->entry);
+ kfree(qe);
+ }
+ }
+
+ return num_records;
+}
+
int ima_restore_measurement_entry(struct ima_template_entry *entry)
{
int result = 0;
--
2.43.0
^ permalink raw reply related
* [PATCH v5 1/3] ima: make ima event log trimming configurable
From: steven chen @ 2026-04-01 17:29 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve, chenste,
gregorylumen, nramas, sushring, linux-doc
In-Reply-To: <20260401172956.4581-1-chenste@linux.microsoft.com>
Make ima event log trimming function configurable.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
security/integrity/ima/Kconfig | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 976e75f9b9ba..322964ae4772 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -332,4 +332,16 @@ config IMA_KEXEC_EXTRA_MEMORY_KB
If set to the default value of 0, an extra half page of memory for those
additional measurements will be allocated.
+config IMA_LOG_TRIMMING
+ bool "IMA Event Log Trimming"
+ default n
+ help
+ Say Y here if you want support for IMA Event Log Trimming.
+ This creates the file /sys/kernel/security/integrity/ima/ima_trim_log.
+ Userspace
+ - writes to this file to trigger IMA event log trimming
+ - reads this file to get number of entried trimming last time
+
+ If unsure, say N.
+
endif
--
2.43.0
^ permalink raw reply related
* [PATCH v5 0/3] Trim N entries of IMA event logs
From: steven chen @ 2026-04-01 17:29 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve, chenste,
gregorylumen, nramas, sushring, linux-doc
The Integrity Measurement Architecture (IMA) maintains a measurement list
—a record of system events used for integrity verification. The IMA event
logs are the entries within this measurement list, each representing a
specific event or measurement that contributes to the system's integrity
assessment.
This update introduces the ability to trim, or remove, N entries from the
current measurement list. Trimming involves deleting N entries from the
list. This action atomically truncates the measurement list, ensuring that
no new measurements can be added until the operation is complete.
Importantly, only one writer can initiate this trimming process at a time,
maintaining consistency and preventing race conditions.
A userspace interface, ima_trim_log, has been provided for this purpose.
When this interface is read, it returns the total number T of entries
trimmed since system boot up. This value T need to be preserved across
kexec soft reboots. By writing two number T:N to this interface, userspace
can request the kernel to trim N entries from the IMA event logs.
To maintain a complete record, userspace is responsible for concatenating
and storing the logs before initiating trimming. Userspace can then send
the collected data to remote verifiers for validation. After receiving
confirmation from the remote verifiers, userspace may instruct the kernel
to proceed with trimming the IMA event logs accordingly.
The primary benefit of this solution is the ability to free valuable
kernel memory by delegating the task of reconstructing the full
measurement list from log chunks to userspace. Trust is not required in
userspace for the integrity of the measurement list, as its integrity is
cryptographically protected by the Trusted Platform Module (TPM).
Multiple readers are allowed to access the ima_trim_log interface
concurrently, while only one writer can trigger log trimming at any time.
During trimming, readers do not see the list and cannot access it while
deletion is in progress, ensuring atomicity.
Introduce the new kernel option ima_flush_htable to decide whether or not
the digests of measurement entries are flushed from the hash table (from
reference [2]).
The ima_measure_users counter (protected by the ima_measure_lock mutex) has
been introduced to protect access to the measurement list part. The open
method of all the measurement interfaces has been extended to allow only
one writer at a time or, in alternative, multiple readers. The write
permission is used to stage/delete the measurements, the read permission
to read them. Write requires also the CAP_SYS_ADMIN capability (from
reference [2]). This ima_measure_users needs to be preserved across kexec
soft reboots
The total trimmed number T and the ima_measure_users both need to be
preserved across kexec soft reboot and new patch will be added for this
purpose in next version.
New IMA log trim event is added when trimming finish.
The time required for trimming is minimal, and IMA event logs are briefly
on hold during this process, preventing read or add operations. This short
interruption has no impact on the overall functionality of IMA.
A new critical data record "ima_log_trim" is added in this solution. This
record logs the trim event with number of entries deleted total T since
system start and time when this happened. User space can get the total
number T of entries trimmed by checking "ima_log_trim" event in the
measurement list.
The following are how user space to use the measurement list and
ima_log_trim interface
1. get the total numer trimmed T through "ima_log_trim" interface
2. get the PCR quote
3. read the measurement list file, close the file, send for verification
4. wait for response from verifier, until get the good response from
verifier with number N that matched the PCR quote got in step 2
5. get the number N from the above message
6. write the T:N to the ima_log_trim interface when no conflict
when kernel get log trim request T:N
Get the T, compare with the total trimmed number
if equal, then do trim N and change T to T+N
else return error
Using above way to trim the log, the time for user space to hold the list
will be trimming T:N operation itself at the step 6. User space agent
race condition is solved too in this way.
References:
-----------
[1] [PATCH 0/1] Trim N entries of IMA event logs
https://lore.kernel.org/linux-integrity/20251202232857.8211-1-chenste@linux.microsoft.com/T/#t
[2] [RFC][PATCH] ima: Add support for staging measurements for deletion
https://lore.kernel.org/linux-integrity/207fd6d7-53c-57bb-36d8-13a0902052d1@linux.microsoft.com/T/#t
[3] [PATCH v2 0/1] Trim N entries of IMA event logs
https://lore.kernel.org/linux-integrity/20251210235314.3341-1-chenste@linux.microsoft.com/T/#t
[4] [PATCH v3 0/3] Trim N entries of IMA event logs
https://lore.kernel.org/linux-integrity/20260106020713.3994-1-chenste@linux.microsoft.com/T/#t
[5] [PATCH v4 0/3] Trim N entries of IMA event logs
https://lore.kernel.org/linux-integrity/20260205235849.7086-1-chenste@linux.microsoft.com/T/#t
Change Log v5:
- lock time performance improvement
- Keep hash table unchanged because log already use the hash value
- Updated patch descriptions as necessary.
Change Log v4:
- Incorporated feedback from Roberto on v3 series.
- Update "ima_log_trim" interface definition
When read this interface, return total number of records trimmed T
need to write T:N to this interface to trim N records
- Update user space use case on how to trim IMA event logs
- Updated patch descriptions as necessary.
Change Log v3:
- Incorporated feedback from Mimi on v2 series.
- split patch into multiple patches
- lock time performance improvement
- Updated patch descriptions as necessary.
Change Log v2:
- Incorporated feedback from the Roberto on v1 series.
- Adapted code from Roberto's RFC [Reference 2]
- Add IMA log trim event log to record trim event
- Updated patch descriptions as necessary
steven chen (3):
ima: make ima event log trimming configurable
ima: trim N IMA event log records
ima: add new critical data record to measure log trim
.../admin-guide/kernel-parameters.txt | 4 +
security/integrity/ima/Kconfig | 12 +
security/integrity/ima/ima.h | 4 +-
security/integrity/ima/ima_fs.c | 218 +++++++++++++++++-
security/integrity/ima/ima_kexec.c | 2 +-
security/integrity/ima/ima_queue.c | 96 ++++++++
6 files changed, 328 insertions(+), 8 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH] landlock: Document fallocate(2) as another truncation corner case
From: Günther Noack @ 2026-04-01 17:13 UTC (permalink / raw)
To: Mickaël Salaün; +Cc: linux-security-module
In-Reply-To: <20260401.oor1chahp2oF@digikod.net>
On Wed, Apr 01, 2026 at 06:30:28PM +0200, Mickaël Salaün wrote:
> On Wed, Apr 01, 2026 at 05:09:10PM +0200, Günther Noack wrote:
> > Reinforce the already stated policy that LANDLOCK_ACCESS_FS_TRUNCATE should
> > always go hand in hand with LANDLOCK_ACCESS_FS_WRITE_FILE, as their
> > meanings and enforcement overlap in counterintuitive ways.
> >
> > On many common file systems, fallocate(2) offers a way to shorten files as
> > long as the file is opened for writing, side-stepping the
> > LANDLOCK_ACCESS_FS_TRUNCATE right.
> >
> > Assisted-by: Gemini-CLI:gemini-3.1
> > Signed-off-by: Günther Noack <gnoack@google.com>
> > ---
> > Documentation/userspace-api/landlock.rst | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> > index 7f86d7a37dc2..d5691ec136cc 100644
> > --- a/Documentation/userspace-api/landlock.rst
> > +++ b/Documentation/userspace-api/landlock.rst
> > @@ -378,8 +378,8 @@ Truncating files
> >
> > The operations covered by ``LANDLOCK_ACCESS_FS_WRITE_FILE`` and
> > ``LANDLOCK_ACCESS_FS_TRUNCATE`` both change the contents of a file and sometimes
> > -overlap in non-intuitive ways. It is recommended to always specify both of
> > -these together.
> > +overlap in non-intuitive ways. It is strongly recommended to always specify
> > +both of these together (either granting both, or granting none).
> >
> > A particularly surprising example is :manpage:`creat(2)`. The name suggests
> > that this system call requires the rights to create and write files. However,
> > @@ -391,6 +391,10 @@ It should also be noted that truncating files does not require the
> > system call, this can also be done through :manpage:`open(2)` with the flags
> > ``O_RDONLY | O_TRUNC``.
> >
> > +At the same time, on some filesystems, :manpage:`fallocate(2)` offers a way to
> > +shorten file contents with ``FALLOC_FL_COLLAPSE_RANGE`` when the file is opened
> > +for writing, sidestepping the ``LANDLOCK_ACCESS_FS_TRUNCATE`` right.
>
> Interesting, which filesystems? Shouldn't it be fixed in the code
> instead?
It works on ext4, and I also see mentions of FALLOC_FL_COLLAPSE_RANGE
in XFS, F2FS, SMB and NTFS3.
I should mention, it is not *exactly* the same as a truncation, but
you can remove a chunk of the file from the middle, which also leads
to a shorter file. For example, assuming a block size of 1024:
1. Make a file with 2*1024 bytes: 1024*'A', then 1024*'B'
2. fallocate(collapse range, 0, 1024)
Resulting file is 1024*'B', and the file is shortened to 1024 bytes.
So this is not *exactly* a truncation. (The man page says that an
attempt to remove the end of a file results in EINVAL, so you have to
take it from the middle, and it needs to align with block boundaries.)
But it's quite similar, also shortens the file, and it does not
require the Landlock truncation access right.
I agree, another way would potentially be to call the LSM ftruncate
hook. I suspect this would stay compatible with other LSMs, because
the LSM ftruncate hook is a relatively recent addition (but have not
checked in detail).
The implementation of fallocate is vfs_fallocate() in fs/open.c - I
only had a tentative look now; it checks that the file->f_mode is open
for writing and calls security_file_permission() with MAY_WRITE.
I always saw LANDLOCK_ACCESS_FS_WRITE_FILE and
LANDLOCK_ACCESS_FS_TRUNCATE as rights that should always go together,
so I suspect that it does not make a big difference in practice, and
that is why I am suggesting to just document it more clearly for now.
—Günther
^ permalink raw reply
* Re: [RFC PATCH v1 02/11] security: Add LSM_AUDIT_DATA_NS for namespace audit records
From: Mickaël Salaün @ 2026-04-01 16:38 UTC (permalink / raw)
To: Christian Brauner
Cc: Günther Noack, Paul Moore, Serge E . Hallyn, Justin Suess,
Lennart Poettering, Mikhail Ivanov, Nicolas Bouchinet,
Shervin Oloumi, Tingmao Wang, kernel-team, linux-fsdevel,
linux-kernel, linux-security-module
In-Reply-To: <20260325-fachgebiet-parzelle-8ae15e6f305f@brauner>
On Wed, Mar 25, 2026 at 01:32:42PM +0100, Christian Brauner wrote:
> On Thu, Mar 12, 2026 at 11:04:35AM +0100, Mickaël Salaün wrote:
> > Add a new LSM audit data type LSM_AUDIT_DATA_NS that logs namespace
> > information in audit records. Two fields are provided, matching the
> > field names of struct ns_common:
> >
> > - ns_type: the CLONE_NEW* flag identifying the namespace type, logged in
> > hexadecimal.
> >
> > - inum: the proc inode number identifying a specific namespace instance.
> > Namespace inode numbers are allocated by proc_alloc_inum() via
> > ida_alloc_max() bounded to UINT_MAX, so the value always fits in 32
> > bits.
> >
> > A new audit data type is needed because no existing LSM_AUDIT_DATA_*
> > type carries namespace information. The closest alternatives (e.g.
> > LSM_AUDIT_DATA_TASK or LSM_AUDIT_DATA_NONE with custom strings) would
> > either lose the namespace type or require ad-hoc formatting that
> > bypasses the structured audit data union.
> >
> > Cc: Christian Brauner <brauner@kernel.org>
> > Cc: Günther Noack <gnoack@google.com>
> > Cc: Paul Moore <paul@paul-moore.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> > include/linux/lsm_audit.h | 5 +++++
> > security/lsm_audit.c | 4 ++++
> > 2 files changed, 9 insertions(+)
> >
> > diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
> > index 382c56a97bba..6e20a56b8c22 100644
> > --- a/include/linux/lsm_audit.h
> > +++ b/include/linux/lsm_audit.h
> > @@ -78,6 +78,7 @@ struct common_audit_data {
> > #define LSM_AUDIT_DATA_NOTIFICATION 16
> > #define LSM_AUDIT_DATA_ANONINODE 17
> > #define LSM_AUDIT_DATA_NLMSGTYPE 18
> > +#define LSM_AUDIT_DATA_NS 19
> > union {
> > struct path path;
> > struct dentry *dentry;
> > @@ -100,6 +101,10 @@ struct common_audit_data {
> > int reason;
> > const char *anonclass;
> > u16 nlmsg_type;
> > + struct {
> > + u32 ns_type;
> > + unsigned int inum;
>
> fwiw, you might want to start the 64-bit namespace id as well.
> But either way:
Right now these numbers are generated by ida_alloc_max(), which return
an int. Is there an ongoing patch series for this change?
>
> Reviewed-by: Christian Brauner <brauner@kernel.org>
>
^ permalink raw reply
* Re: [PATCH] landlock: Document fallocate(2) as another truncation corner case
From: Mickaël Salaün @ 2026-04-01 16:30 UTC (permalink / raw)
To: Günther Noack; +Cc: linux-security-module
In-Reply-To: <20260401150911.1038072-1-gnoack@google.com>
On Wed, Apr 01, 2026 at 05:09:10PM +0200, Günther Noack wrote:
> Reinforce the already stated policy that LANDLOCK_ACCESS_FS_TRUNCATE should
> always go hand in hand with LANDLOCK_ACCESS_FS_WRITE_FILE, as their
> meanings and enforcement overlap in counterintuitive ways.
>
> On many common file systems, fallocate(2) offers a way to shorten files as
> long as the file is opened for writing, side-stepping the
> LANDLOCK_ACCESS_FS_TRUNCATE right.
>
> Assisted-by: Gemini-CLI:gemini-3.1
> Signed-off-by: Günther Noack <gnoack@google.com>
> ---
> Documentation/userspace-api/landlock.rst | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 7f86d7a37dc2..d5691ec136cc 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -378,8 +378,8 @@ Truncating files
>
> The operations covered by ``LANDLOCK_ACCESS_FS_WRITE_FILE`` and
> ``LANDLOCK_ACCESS_FS_TRUNCATE`` both change the contents of a file and sometimes
> -overlap in non-intuitive ways. It is recommended to always specify both of
> -these together.
> +overlap in non-intuitive ways. It is strongly recommended to always specify
> +both of these together (either granting both, or granting none).
>
> A particularly surprising example is :manpage:`creat(2)`. The name suggests
> that this system call requires the rights to create and write files. However,
> @@ -391,6 +391,10 @@ It should also be noted that truncating files does not require the
> system call, this can also be done through :manpage:`open(2)` with the flags
> ``O_RDONLY | O_TRUNC``.
>
> +At the same time, on some filesystems, :manpage:`fallocate(2)` offers a way to
> +shorten file contents with ``FALLOC_FL_COLLAPSE_RANGE`` when the file is opened
> +for writing, sidestepping the ``LANDLOCK_ACCESS_FS_TRUNCATE`` right.
Interesting, which filesystems? Shouldn't it be fixed in the code
instead?
> +
> The truncate right is associated with the opened file (see below).
>
> Rights associated with file descriptors
> --
> 2.53.0.1185.g05d4b7b318-goog
>
>
^ permalink raw reply
* [PATCH v2 1/4] selftests/landlock: Fix snprintf truncation checks in audit helpers
From: Mickaël Salaün @ 2026-04-01 16:14 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, linux-security-module, Justin Suess,
Tingmao Wang
In-Reply-To: <20260401161503.1136946-1-mic@digikod.net>
snprintf() returns the number of characters that would have been
written, excluding the terminating NUL byte. When the output is
truncated, this return value equals or exceeds the buffer size. Fix
matches_log_domain_allocated() and matches_log_domain_deallocated() to
detect truncation with ">=" instead of ">".
Cc: Günther Noack <gnoack@google.com>
Fixes: 6a500b22971c ("selftests/landlock: Add tests for audit flags and domain IDs")
Reviewed-by: Günther Noack <gnoack@google.com>
Link: https://lore.kernel.org/r/20260312100444.2609563-8-mic@digikod.net
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
Changes since v1:
https://lore.kernel.org/r/20260312100444.2609563-8-mic@digikod.net
- New patch (split from the drain fix).
---
tools/testing/selftests/landlock/audit.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/landlock/audit.h b/tools/testing/selftests/landlock/audit.h
index 44eb433e9666..1049a0582af5 100644
--- a/tools/testing/selftests/landlock/audit.h
+++ b/tools/testing/selftests/landlock/audit.h
@@ -309,7 +309,7 @@ static int __maybe_unused matches_log_domain_allocated(int audit_fd, pid_t pid,
log_match_len =
snprintf(log_match, sizeof(log_match), log_template, pid);
- if (log_match_len > sizeof(log_match))
+ if (log_match_len >= sizeof(log_match))
return -E2BIG;
return audit_match_record(audit_fd, AUDIT_LANDLOCK_DOMAIN, log_match,
@@ -326,7 +326,7 @@ static int __maybe_unused matches_log_domain_deallocated(
log_match_len = snprintf(log_match, sizeof(log_match), log_template,
num_denials);
- if (log_match_len > sizeof(log_match))
+ if (log_match_len >= sizeof(log_match))
return -E2BIG;
return audit_match_record(audit_fd, AUDIT_LANDLOCK_DOMAIN, log_match,
--
2.53.0
^ permalink raw reply related
* [PATCH v2 4/4] selftests/landlock: Skip stale records in audit_match_record()
From: Mickaël Salaün @ 2026-04-01 16:14 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, linux-security-module, Justin Suess,
Tingmao Wang
In-Reply-To: <20260401161503.1136946-1-mic@digikod.net>
Domain deallocation records are emitted asynchronously from kworker
threads (via free_ruleset_work()). Stale deallocation records from a
previous test can arrive during the current test's deallocation read
loop and be picked up by audit_match_record() instead of the expected
record, causing a domain ID mismatch. The audit.layers test (which
creates 16 nested domains) is particularly vulnerable because it reads
16 deallocation records in sequence, providing a large window for stale
records to interleave.
The same issue affects audit_flags.signal, where deallocation records
from a previous test (audit.layers) can leak into the next test and be
picked up by audit_match_record() instead of the expected record.
Fix this by continuing to read records when the type matches but the
content pattern does not. Stale records are silently consumed, and the
loop only stops when both type and pattern match (or the socket times
out with -EAGAIN).
Additionally, extend matches_log_domain_deallocated() with an
expected_domain_id parameter. When set, the regex pattern includes the
specific domain ID as a literal hex value, so that deallocation records
for a different domain do not match the pattern at all. This handles
the case where the stale record has the same denial count as the
expected one (e.g. both have denials=1), which the type+pattern loop
alone cannot distinguish. Callers that already know the expected domain
ID (from a prior denial or allocation record) now pass it to filter
precisely.
When expected_domain_id is set, matches_log_domain_deallocated() also
temporarily increases the socket timeout to audit_tv_dom_drop (1 second)
to wait for the asynchronous kworker deallocation, and restores
audit_tv_default afterward. This removes the need for callers to manage
the timeout switch manually.
Cc: Günther Noack <gnoack@google.com>
Fixes: 6a500b22971c ("selftests/landlock: Add tests for audit flags and domain IDs")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
Changes since v1:
- New patch.
---
tools/testing/selftests/landlock/audit.h | 81 ++++++++++++++-----
tools/testing/selftests/landlock/audit_test.c | 32 ++++----
2 files changed, 75 insertions(+), 38 deletions(-)
diff --git a/tools/testing/selftests/landlock/audit.h b/tools/testing/selftests/landlock/audit.h
index 74e1c3d763be..b439a9b00f0e 100644
--- a/tools/testing/selftests/landlock/audit.h
+++ b/tools/testing/selftests/landlock/audit.h
@@ -249,9 +249,9 @@ static __maybe_unused char *regex_escape(const char *const src, char *dst,
static int audit_match_record(int audit_fd, const __u16 type,
const char *const pattern, __u64 *domain_id)
{
- struct audit_message msg;
+ struct audit_message msg, last_mismatch = {};
int ret, err = 0;
- bool matches_record = !type;
+ int num_type_match = 0;
regmatch_t matches[2];
regex_t regex;
@@ -259,21 +259,35 @@ static int audit_match_record(int audit_fd, const __u16 type,
if (ret)
return -EINVAL;
- do {
+ /*
+ * Reads records until one matches both the expected type and the
+ * pattern. Type-matching records with non-matching content are
+ * silently consumed, which handles stale domain deallocation records
+ * from a previous test emitted asynchronously by kworker threads.
+ */
+ while (true) {
memset(&msg, 0, sizeof(msg));
err = audit_recv(audit_fd, &msg);
- if (err)
+ if (err) {
+ if (num_type_match) {
+ printf("DATA: %s\n", last_mismatch.data);
+ printf("ERROR: %d record(s) matched type %u"
+ " but not pattern: %s\n",
+ num_type_match, type, pattern);
+ }
goto out;
+ }
- if (msg.header.nlmsg_type == type)
- matches_record = true;
- } while (!matches_record);
+ if (type && msg.header.nlmsg_type != type)
+ continue;
- ret = regexec(®ex, msg.data, ARRAY_SIZE(matches), matches, 0);
- if (ret) {
- printf("DATA: %s\n", msg.data);
- printf("ERROR: no match for pattern: %s\n", pattern);
- err = -ENOENT;
+ ret = regexec(®ex, msg.data, ARRAY_SIZE(matches), matches,
+ 0);
+ if (!ret)
+ break;
+
+ num_type_match++;
+ last_mismatch = msg;
}
if (domain_id) {
@@ -316,21 +330,48 @@ static int __maybe_unused matches_log_domain_allocated(int audit_fd, pid_t pid,
domain_id);
}
-static int __maybe_unused matches_log_domain_deallocated(
- int audit_fd, unsigned int num_denials, __u64 *domain_id)
+/*
+ * Matches a domain deallocation record. When expected_domain_id is non-zero,
+ * the pattern includes the specific domain ID so that stale deallocation
+ * records from a previous test (with a different domain ID) are skipped by
+ * audit_match_record(), and the socket timeout is temporarily increased to
+ * audit_tv_dom_drop to wait for the asynchronous kworker deallocation.
+ */
+static int __maybe_unused
+matches_log_domain_deallocated(int audit_fd, unsigned int num_denials,
+ __u64 expected_domain_id, __u64 *domain_id)
{
static const char log_template[] = REGEX_LANDLOCK_PREFIX
" status=deallocated denials=%u$";
- char log_match[sizeof(log_template) + 10];
- int log_match_len;
+ static const char log_template_with_id[] =
+ "^audit([0-9.:]\\+): domain=\\(%llx\\)"
+ " status=deallocated denials=%u$";
+ char log_match[sizeof(log_template_with_id) + 32];
+ int log_match_len, err;
+
+ if (expected_domain_id)
+ log_match_len = snprintf(log_match, sizeof(log_match),
+ log_template_with_id,
+ expected_domain_id, num_denials);
+ else
+ log_match_len = snprintf(log_match, sizeof(log_match),
+ log_template, num_denials);
- log_match_len = snprintf(log_match, sizeof(log_match), log_template,
- num_denials);
if (log_match_len >= sizeof(log_match))
return -E2BIG;
- return audit_match_record(audit_fd, AUDIT_LANDLOCK_DOMAIN, log_match,
- domain_id);
+ if (expected_domain_id)
+ setsockopt(audit_fd, SOL_SOCKET, SO_RCVTIMEO,
+ &audit_tv_dom_drop, sizeof(audit_tv_dom_drop));
+
+ err = audit_match_record(audit_fd, AUDIT_LANDLOCK_DOMAIN, log_match,
+ domain_id);
+
+ if (expected_domain_id)
+ setsockopt(audit_fd, SOL_SOCKET, SO_RCVTIMEO, &audit_tv_default,
+ sizeof(audit_tv_default));
+
+ return err;
}
struct audit_records {
diff --git a/tools/testing/selftests/landlock/audit_test.c b/tools/testing/selftests/landlock/audit_test.c
index f92ba6774faa..a76c3686a0fe 100644
--- a/tools/testing/selftests/landlock/audit_test.c
+++ b/tools/testing/selftests/landlock/audit_test.c
@@ -139,13 +139,16 @@ TEST_F(audit, layers)
WEXITSTATUS(status) != EXIT_SUCCESS)
_metadata->exit_code = KSFT_FAIL;
- /* Purges log from deallocated domains. */
- EXPECT_EQ(0, setsockopt(self->audit_fd, SOL_SOCKET, SO_RCVTIMEO,
- &audit_tv_dom_drop, sizeof(audit_tv_dom_drop)));
+ /*
+ * Purges log from deallocated domains. Records arrive in LIFO order
+ * (innermost domain first) because landlock_put_hierarchy() walks the
+ * chain sequentially in a single kworker context.
+ */
for (i = ARRAY_SIZE(*domain_stack) - 1; i >= 0; i--) {
__u64 deallocated_dom = 2;
EXPECT_EQ(0, matches_log_domain_deallocated(self->audit_fd, 1,
+ (*domain_stack)[i],
&deallocated_dom));
EXPECT_EQ((*domain_stack)[i], deallocated_dom)
{
@@ -154,8 +157,6 @@ TEST_F(audit, layers)
}
}
EXPECT_EQ(0, munmap(domain_stack, sizeof(*domain_stack)));
- EXPECT_EQ(0, setsockopt(self->audit_fd, SOL_SOCKET, SO_RCVTIMEO,
- &audit_tv_default, sizeof(audit_tv_default)));
EXPECT_EQ(0, close(ruleset_fd));
}
@@ -270,13 +271,9 @@ TEST_F(audit, thread)
EXPECT_EQ(0, close(pipe_parent[1]));
ASSERT_EQ(0, pthread_join(thread, NULL));
- EXPECT_EQ(0, setsockopt(self->audit_fd, SOL_SOCKET, SO_RCVTIMEO,
- &audit_tv_dom_drop, sizeof(audit_tv_dom_drop)));
- EXPECT_EQ(0, matches_log_domain_deallocated(self->audit_fd, 1,
- &deallocated_dom));
+ EXPECT_EQ(0, matches_log_domain_deallocated(
+ self->audit_fd, 1, denial_dom, &deallocated_dom));
EXPECT_EQ(denial_dom, deallocated_dom);
- EXPECT_EQ(0, setsockopt(self->audit_fd, SOL_SOCKET, SO_RCVTIMEO,
- &audit_tv_default, sizeof(audit_tv_default)));
}
FIXTURE(audit_flags)
@@ -432,22 +429,21 @@ TEST_F(audit_flags, signal)
if (variant->restrict_flags &
LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF) {
+ /*
+ * No deallocation record: denials=0 never matches a real
+ * record.
+ */
EXPECT_EQ(-EAGAIN,
- matches_log_domain_deallocated(self->audit_fd, 0,
+ matches_log_domain_deallocated(self->audit_fd, 0, 0,
&deallocated_dom));
EXPECT_EQ(deallocated_dom, 2);
} else {
- EXPECT_EQ(0, setsockopt(self->audit_fd, SOL_SOCKET, SO_RCVTIMEO,
- &audit_tv_dom_drop,
- sizeof(audit_tv_dom_drop)));
EXPECT_EQ(0, matches_log_domain_deallocated(self->audit_fd, 2,
+ *self->domain_id,
&deallocated_dom));
EXPECT_NE(deallocated_dom, 2);
EXPECT_NE(deallocated_dom, 0);
EXPECT_EQ(deallocated_dom, *self->domain_id);
- EXPECT_EQ(0, setsockopt(self->audit_fd, SOL_SOCKET, SO_RCVTIMEO,
- &audit_tv_default,
- sizeof(audit_tv_default)));
}
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/4] selftests/landlock: Fix socket file descriptor leaks in audit helpers
From: Mickaël Salaün @ 2026-04-01 16:14 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, linux-security-module, Justin Suess,
Tingmao Wang
In-Reply-To: <20260401161503.1136946-1-mic@digikod.net>
audit_init() opens a netlink socket and configures it, but leaks the
file descriptor if audit_set_status() or setsockopt() fails. Fix this
by jumping to an error path that closes the socket before returning.
Apply the same fix to audit_init_with_exe_filter(), which leaks the file
descriptor from audit_init() if audit_init_filter_exe() or
audit_filter_exe() fails, and to audit_cleanup(), which leaks it if
audit_init_filter_exe() fails in FIXTURE_TEARDOWN_PARENT().
Cc: Günther Noack <gnoack@google.com>
Fixes: 6a500b22971c ("selftests/landlock: Add tests for audit flags and domain IDs")
Link: https://lore.kernel.org/r/20260312100444.2609563-8-mic@digikod.net
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
Changes since v1:
https://lore.kernel.org/r/20260312100444.2609563-8-mic@digikod.net
- New patch (split from the drain fix, extended to
audit_init_with_exe_filter() and audit_cleanup()).
---
tools/testing/selftests/landlock/audit.h | 26 +++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/landlock/audit.h b/tools/testing/selftests/landlock/audit.h
index 1049a0582af5..6422943fc69e 100644
--- a/tools/testing/selftests/landlock/audit.h
+++ b/tools/testing/selftests/landlock/audit.h
@@ -379,19 +379,25 @@ static int audit_init(void)
err = audit_set_status(fd, AUDIT_STATUS_ENABLED, 1);
if (err)
- return err;
+ goto err_close;
err = audit_set_status(fd, AUDIT_STATUS_PID, getpid());
if (err)
- return err;
+ goto err_close;
/* Sets a timeout for negative tests. */
err = setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &audit_tv_default,
sizeof(audit_tv_default));
- if (err)
- return -errno;
+ if (err) {
+ err = -errno;
+ goto err_close;
+ }
return fd;
+
+err_close:
+ close(fd);
+ return err;
}
static int audit_init_filter_exe(struct audit_filter *filter, const char *path)
@@ -441,8 +447,10 @@ static int audit_cleanup(int audit_fd, struct audit_filter *filter)
filter = &new_filter;
err = audit_init_filter_exe(filter, NULL);
- if (err)
+ if (err) {
+ close(audit_fd);
return err;
+ }
}
/* Filters might not be in place. */
@@ -468,11 +476,15 @@ static int audit_init_with_exe_filter(struct audit_filter *filter)
err = audit_init_filter_exe(filter, NULL);
if (err)
- return err;
+ goto err_close;
err = audit_filter_exe(fd, filter, AUDIT_ADD_RULE);
if (err)
- return err;
+ goto err_close;
return fd;
+
+err_close:
+ close(fd);
+ return err;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 3/4] selftests/landlock: Drain stale audit records on init
From: Mickaël Salaün @ 2026-04-01 16:14 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, linux-security-module, Justin Suess,
Tingmao Wang
In-Reply-To: <20260401161503.1136946-1-mic@digikod.net>
Non-audit Landlock tests generate audit records as side effects when
audit_enabled is non-zero (e.g. from boot configuration). These records
accumulate in the kernel audit backlog while no audit daemon socket is
open. When the next test opens a new netlink socket and registers as
the audit daemon, the stale backlog is delivered, causing baseline
record count checks to fail spuriously.
Fix this by draining all pending records in audit_init() right after
setting the receive timeout. The 1-usec SO_RCVTIMEO causes audit_recv()
to return -EAGAIN once the backlog is empty, naturally terminating the
drain loop.
Domain deallocation records are emitted asynchronously from a work
queue, so they may still arrive after the drain. Remove records.domain
== 0 checks that are not preceded by audit_match_record() calls, which
would otherwise consume stale records before the count. Document this
constraint above audit_count_records().
Increasing the drain timeout to catch in-flight deallocation records was
considered but rejected: a longer timeout adds latency to every
audit_init() call even when no stale record is pending, and any fixed
timeout is still not guaranteed to catch all records under load.
Removing the unprotected checks is simpler and avoids the spurious
failures.
Cc: Günther Noack <gnoack@google.com>
Fixes: 6a500b22971c ("selftests/landlock: Add tests for audit flags and domain IDs")
Link: https://lore.kernel.org/r/20260312100444.2609563-8-mic@digikod.net
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
Changes since v1:
https://lore.kernel.org/r/20260312100444.2609563-8-mic@digikod.net
- Also remove domain checks from audit.trace and
scoped_audit.connect_to_child.
- Document records.domain == 0 constraint above
audit_count_records().
- Explain why a longer drain timeout was rejected.
- Drop Reviewed-by (new code comment not in v1).
- Split snprintf and fd leak fixes into separate patches.
---
tools/testing/selftests/landlock/audit.h | 19 +++++++++++++++++++
tools/testing/selftests/landlock/audit_test.c | 2 --
.../testing/selftests/landlock/ptrace_test.c | 1 -
.../landlock/scoped_abstract_unix_test.c | 1 -
4 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/landlock/audit.h b/tools/testing/selftests/landlock/audit.h
index 6422943fc69e..74e1c3d763be 100644
--- a/tools/testing/selftests/landlock/audit.h
+++ b/tools/testing/selftests/landlock/audit.h
@@ -338,6 +338,15 @@ struct audit_records {
size_t domain;
};
+/*
+ * WARNING: Do not assert records.domain == 0 without a preceding
+ * audit_match_record() call. Domain deallocation records are emitted
+ * asynchronously from kworker threads and can arrive after the drain in
+ * audit_init(), corrupting the domain count. A preceding audit_match_record()
+ * call consumes stale records while scanning, making the assertion safe in
+ * practice because stale deallocation records arrive before the expected access
+ * records.
+ */
static int audit_count_records(int audit_fd, struct audit_records *records)
{
struct audit_message msg;
@@ -393,6 +402,16 @@ static int audit_init(void)
goto err_close;
}
+ /*
+ * Drains stale audit records that accumulated in the kernel backlog
+ * while no audit daemon socket was open. This happens when non-audit
+ * Landlock tests generate records while audit_enabled is non-zero (e.g.
+ * from boot configuration), or when domain deallocation records arrive
+ * asynchronously after a previous test's socket was closed.
+ */
+ while (audit_recv(fd, NULL) == 0)
+ ;
+
return fd;
err_close:
diff --git a/tools/testing/selftests/landlock/audit_test.c b/tools/testing/selftests/landlock/audit_test.c
index 46d02d49835a..f92ba6774faa 100644
--- a/tools/testing/selftests/landlock/audit_test.c
+++ b/tools/testing/selftests/landlock/audit_test.c
@@ -412,7 +412,6 @@ TEST_F(audit_flags, signal)
} else {
EXPECT_EQ(1, records.access);
}
- EXPECT_EQ(0, records.domain);
/* Updates filter rules to match the drop record. */
set_cap(_metadata, CAP_AUDIT_CONTROL);
@@ -601,7 +600,6 @@ TEST_F(audit_exec, signal_and_open)
/* Tests that there was no denial until now. */
EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
EXPECT_EQ(0, records.access);
- EXPECT_EQ(0, records.domain);
/*
* Wait for the child to do a first denied action by layer1 and
diff --git a/tools/testing/selftests/landlock/ptrace_test.c b/tools/testing/selftests/landlock/ptrace_test.c
index 4f64c90583cd..1b6c8b53bf33 100644
--- a/tools/testing/selftests/landlock/ptrace_test.c
+++ b/tools/testing/selftests/landlock/ptrace_test.c
@@ -342,7 +342,6 @@ TEST_F(audit, trace)
/* Makes sure there is no superfluous logged records. */
EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
EXPECT_EQ(0, records.access);
- EXPECT_EQ(0, records.domain);
yama_ptrace_scope = get_yama_ptrace_scope();
ASSERT_LE(0, yama_ptrace_scope);
diff --git a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c b/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
index 72f97648d4a7..c47491d2d1c1 100644
--- a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
+++ b/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
@@ -312,7 +312,6 @@ TEST_F(scoped_audit, connect_to_child)
/* Makes sure there is no superfluous logged records. */
EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
EXPECT_EQ(0, records.access);
- EXPECT_EQ(0, records.domain);
ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
--
2.53.0
^ permalink raw reply related
* [PATCH v2 0/4] Fix Landlock audit test flakiness
From: Mickaël Salaün @ 2026-04-01 16:14 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, linux-security-module, Justin Suess,
Tingmao Wang
This series fixes two classes of audit selftest failures plus two minor
bugs in the audit test helpers.
The main issue is that domain deallocation audit records are emitted
asynchronously from kworker threads and can arrive after a previous
test's socket has been closed. This causes two distinct failure modes:
- audit_match_record() picks up a stale deallocation record from a
previous test instead of the expected one, causing a domain ID
mismatch. The audit.layers test (which reads 16 deallocation records
in sequence) is particularly vulnerable because the large read window
allows stale records to interleave. Patch 4 fixes this by filtering
deallocation records by domain ID and skipping type-matching records
with wrong content patterns.
- audit_count_records() counts stale deallocation records from a
previous test, incrementing records.domain from the expected 0 to 1.
Patch 3 fixes this by draining stale records at audit_init() time and
removing records.domain == 0 checks that are not preceded by
audit_match_record() calls (which would consume stale records).
These races are more likely to manifest when additional instrumentation
changes kworker timing in the deallocation path (e.g. with the upcoming
Landlock tracepoints work).
The two minor fixes (patches 1-2) correct a snprintf truncation check
off-by-one and socket file descriptor leaks on error paths in
audit_init(), audit_init_with_exe_filter(), and audit_cleanup().
Patch 1 is an exact subset of the v1 combined patch, which is why it
carries the Reviewed-by tag. Patches 2 and 3 extend beyond what was in
v1, so the Reviewed-by is not carried. Patch 4 is new.
Changes since v1:
https://lore.kernel.org/r/20260312100444.2609563-8-mic@digikod.net
- Split the combined drain fix into four separate patches.
- Patch 2: extend fd leak fix to audit_init_with_exe_filter() and
audit_cleanup().
- Patch 3: also remove domain checks from audit.trace and
scoped_audit.connect_to_child, document constraint, explain why a
longer drain timeout was rejected.
- Patch 4: new, add domain ID filtering and timeout management to
matches_log_domain_deallocated(), skip stale records in
audit_match_record().
Mickaël Salaün (4):
selftests/landlock: Fix snprintf truncation checks in audit helpers
selftests/landlock: Fix socket file descriptor leaks in audit helpers
selftests/landlock: Drain stale audit records on init
selftests/landlock: Skip stale records in audit_match_record()
tools/testing/selftests/landlock/audit.h | 132 ++++++++++++++----
tools/testing/selftests/landlock/audit_test.c | 34 ++---
.../testing/selftests/landlock/ptrace_test.c | 1 -
.../landlock/scoped_abstract_unix_test.c | 1 -
4 files changed, 116 insertions(+), 52 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH] landlock: Document fallocate(2) as another truncation corner case
From: Günther Noack @ 2026-04-01 15:09 UTC (permalink / raw)
To: Mickaël Salaün; +Cc: linux-security-module, Günther Noack
Reinforce the already stated policy that LANDLOCK_ACCESS_FS_TRUNCATE should
always go hand in hand with LANDLOCK_ACCESS_FS_WRITE_FILE, as their
meanings and enforcement overlap in counterintuitive ways.
On many common file systems, fallocate(2) offers a way to shorten files as
long as the file is opened for writing, side-stepping the
LANDLOCK_ACCESS_FS_TRUNCATE right.
Assisted-by: Gemini-CLI:gemini-3.1
Signed-off-by: Günther Noack <gnoack@google.com>
---
Documentation/userspace-api/landlock.rst | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 7f86d7a37dc2..d5691ec136cc 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -378,8 +378,8 @@ Truncating files
The operations covered by ``LANDLOCK_ACCESS_FS_WRITE_FILE`` and
``LANDLOCK_ACCESS_FS_TRUNCATE`` both change the contents of a file and sometimes
-overlap in non-intuitive ways. It is recommended to always specify both of
-these together.
+overlap in non-intuitive ways. It is strongly recommended to always specify
+both of these together (either granting both, or granting none).
A particularly surprising example is :manpage:`creat(2)`. The name suggests
that this system call requires the rights to create and write files. However,
@@ -391,6 +391,10 @@ It should also be noted that truncating files does not require the
system call, this can also be done through :manpage:`open(2)` with the flags
``O_RDONLY | O_TRUNC``.
+At the same time, on some filesystems, :manpage:`fallocate(2)` offers a way to
+shorten file contents with ``FALLOC_FL_COLLAPSE_RANGE`` when the file is opened
+for writing, sidestepping the ``LANDLOCK_ACCESS_FS_TRUNCATE`` right.
+
The truncate right is associated with the opened file (see below).
Rights associated with file descriptors
--
2.53.0.1185.g05d4b7b318-goog
^ permalink raw reply related
* Re: [PATCH v3 6/9] security: Hornet LSM
From: Paul Moore @ 2026-03-31 23:49 UTC (permalink / raw)
To: Blaise Boscaccy, Blaise Boscaccy, Jonathan Corbet, James Morris,
Serge E. Hallyn, Mickaël Salaün, Günther Noack,
Dr. David Alan Gilbert, Andrew Morton, James.Bottomley, dhowells,
Fan Wu, Ryan Foster, Randy Dunlap, linux-security-module,
linux-doc, linux-kernel, bpf
In-Reply-To: <20260326060655.2550595-7-bboscaccy@linux.microsoft.com>
On Mar 26, 2026 Blaise Boscaccy <bboscaccy@linux.microsoft.com> wrote:
>
> This adds the Hornet Linux Security Module which provides enhanced
> signature verification and data validation for eBPF programs. This
> allows users to continue to maintain an invariant that all code
> running inside of the kernel has actually been signed and verified, by
> the kernel.
>
> This effort builds upon the currently excepted upstream solution. It
> further hardens it by providing deterministic, in-kernel checking of
> map hashes to solidify auditing along with preventing TOCTOU attacks
> against lskel map hashes.
>
> Target map hashes are passed in via PKCS#7 signed attributes. Hornet
> determines the extent which the eBFP program is signed and defers to
> other LSMs for policy decisions.
>
> Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
> Nacked-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> ---
> Documentation/admin-guide/LSM/Hornet.rst | 321 ++++++++++++++++++++++
> Documentation/admin-guide/LSM/index.rst | 1 +
> MAINTAINERS | 9 +
> include/linux/oid_registry.h | 3 +
> include/uapi/linux/lsm.h | 1 +
> security/Kconfig | 3 +-
> security/Makefile | 1 +
> security/hornet/Kconfig | 11 +
> security/hornet/Makefile | 7 +
> security/hornet/hornet.asn1 | 13 +
> security/hornet/hornet_lsm.c | 333 +++++++++++++++++++++++
> 11 files changed, 702 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/admin-guide/LSM/Hornet.rst
> create mode 100644 security/hornet/Kconfig
> create mode 100644 security/hornet/Makefile
> create mode 100644 security/hornet/hornet.asn1
> create mode 100644 security/hornet/hornet_lsm.c
...
> +static int hornet_check_program(struct bpf_prog *prog, union bpf_attr *attr,
> + struct bpf_token *token, bool is_kernel,
> + enum lsm_integrity_verdict *verdict)
> +{
> + struct hornet_maps maps = {0};
> + bpfptr_t usig = make_bpfptr(attr->signature, is_kernel);
> + struct pkcs7_message *msg;
> + struct hornet_parse_context *ctx;
> + void *sig;
> + int err;
> + const void *authattrs;
> + size_t authattrs_len;
> +
> + if (!attr->signature) {
> + *verdict = LSM_INT_VERDICT_UNSIGNED;
> + return 0;
> + }
> +
> + ctx = kzalloc(sizeof(struct hornet_parse_context), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + maps.fd_array = make_bpfptr(attr->fd_array, is_kernel);
> + sig = kzalloc(attr->signature_size, GFP_KERNEL);
> + if (!sig) {
> + err = -ENOMEM;
> + goto out;
> + }
> + err = copy_from_bpfptr(sig, usig, attr->signature_size);
> + if (err != 0)
> + goto cleanup_sig;
> +
> + msg = pkcs7_parse_message(sig, attr->signature_size);
> + if (IS_ERR(msg)) {
> + err = LSM_INT_VERDICT_BADSIG;
> + goto cleanup_sig;
> + }
> +
> + if (verify_pkcs7_message_sig(prog->insnsi, prog->len * sizeof(struct bpf_insn), msg,
> + VERIFY_USE_SECONDARY_KEYRING,
> + VERIFYING_BPF_SIGNATURE,
> + NULL, NULL)) {
> + err = LSM_INT_VERDICT_UNKNOWNKEY;
> + goto cleanup_msg;
> + }
Given that kernel module signatures are verified with
VERIFY_USE_SECONDARY_KEYRING it's reasonable to do the same here in
Hornet. I suspect most users concerned about code integrity, especially
code running in the kernel's context, will likely want to verify BPF
programs with the secondary keyring.
However, as we've seen from prior discussions, there is a desire among
some users to support arbitrary keyrings, and we should find a way to
support that in some configuration.
If we take a similar approach to bpf_verify_pkcs7_signature() and take
the keyring from attr->keyring_id, LSMs that provide enforcement via the
bpf_prog_load_post_integrity callback should be able to check the
keyring_id as part of their decision making and respond accordingly. Do
we need to worry about a malicious userspace modifying attr at this
point? I think the answer is "no", but I didn't chase it through the
code to be sure.
I suppose there might be a need for a yama-esque LSM which only provides
a bpf_prog_load_post_integrity callback and ensures a valid signature
verified against the VERIFY_USE_SECONDARY_KEYRING without the need for
any other policy or tunables, but let's see what the v4 revision looks
like first. We can always add this later if needed, and it could live
within the Hornet dir (similar to how the integrity directory hosts
both the IMA and EVM LSMs).
> + if (pkcs7_get_authattr(msg, OID_hornet_data,
> + &authattrs, &authattrs_len) == -ENODATA) {
> + err = LSM_INT_VERDICT_PARTIALSIG;
> + goto cleanup_msg;
> + }
> +
> + err = asn1_ber_decoder(&hornet_decoder, ctx, authattrs, authattrs_len);
> + if (err < 0 || authattrs == NULL) {
> + err = LSM_INT_VERDICT_BADSIG;
> + goto cleanup_msg;
> + }
> +
> + err = hornet_verify_hashes(&maps, ctx, prog);
> +
> +cleanup_msg:
> + pkcs7_free_message(msg);
> +cleanup_sig:
> + kfree(sig);
> +out:
> + kfree(ctx);
> + return err;
> +}
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 4/9] lsm: framework for BPF integrity verification
From: Paul Moore @ 2026-03-31 22:04 UTC (permalink / raw)
To: Song Liu
Cc: Blaise Boscaccy, Jonathan Corbet, James Morris, Serge E. Hallyn,
Mickaël Salaün, Günther Noack,
Dr. David Alan Gilbert, Andrew Morton, James.Bottomley, dhowells,
Fan Wu, Ryan Foster, Randy Dunlap, linux-security-module,
linux-doc, linux-kernel, bpf
In-Reply-To: <CAPhsuW6fWG8674+LOAACqb1LDAz17w-CxvwBaTf7JXQL6ip5Pg@mail.gmail.com>
On Fri, Mar 27, 2026 at 2:25 PM Song Liu <song@kernel.org> wrote:
> On Fri, Mar 27, 2026 at 10:54 AM Blaise Boscaccy
> <bboscaccy@linux.microsoft.com> wrote:
> >
> > Song Liu <song@kernel.org> writes:
> >
> > > On Wed, Mar 25, 2026 at 11:07 PM Blaise Boscaccy
> > > <bboscaccy@linux.microsoft.com> wrote:
> > > [...]
> > >> The first new callback, bpf_prog_load_integrity(), located within the
> > >> security_bpf_prog_load() hook, is necessary to ensure that the integrity
> > >> verification callbacks are executed before any of the existing LSMs
> > >> are executed via the bpf_prog_load() callback. Reusing the existing
> > >> bpf_prog_load() callback for integrity verification could result in LSMs
> > >> not having access to the integrity verification results when asked to
> > >> authorize the BPF program load in the bpf_prog_load() callback.
> > >>
> > >> The new LSM hook, security_bpf_prog_load_post_integrity(), is intended
> > >> to be called from within LSMs performing BPF program integrity
> > >> verification. It is used to report the verdict of the integrity
> > >> verification to other LSMs enforcing access control policy on BPF
> > >> program loads. LSMs enforcing such access controls should register a
> > >> bpf_prog_load_post_integrity() callback to receive integrity verdicts.
> > >
> > > bpf_prog_load_post_integrity() is weird. Some questions about it:
> > >
> > > 1. Is it possible to call it from other LSMs (not hornet)? Specifically, is it
> > > possible to call it from BPF LSM?
> >
> > There is nothing hornet exclusive about that security hook. If the BPF
> > LSM folks wanted to use it they would probably need to implement a
> > kfunc to invoke it.
>
> Please also include the kfunc in v4.
Blaise is welcome to provide a kfunc for
bpf_prog_load_post_integrity(), but I don't see that as a requirement
for Hornet's acceptance. If a developer wanted to write a service
LSM, like Hornet, in BPF to verify a BPF program's integrity, that
developer would be responsible for implementing the kfunc. If you are
interested in doing that, I suggest you talk with KP as he is the BPF
LSM maintainer and I suspect he may have some concerns around
supporting that (see prior discussions around BPF signature
verification and his own implementation of BPF signature
verification).
As a reminder, if you want to apply security policy to a BPF program
load operation without considering the program's integrity or
provenance, you can do that today with the
security_bpf_prog_load()/bpf_prog_load hook/callback combination.
However, as you pointed out, we ultimately need to see at least one
LSM providing a callback for the bpf_prog_load_post_integrity
callback. I wrote a toy SELinux implementation when I was playing
with Hornet a couple of revisions ago, which could serve as the basis
for a proper patch if needed, but my understanding is that Blaise has
been working with Fan on an IPE implementation.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 1/2] lsm: add backing_file LSM hooks
From: Paul Moore @ 2026-03-31 2:13 UTC (permalink / raw)
To: Amir Goldstein
Cc: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs, Gao Xiang, Christian Brauner
In-Reply-To: <CAOQ4uxgcOCP8cf8KvCsC=5OiuRvULKOf52mc2n9qEBAhPKoUGg@mail.gmail.com>
On Mon, Mar 30, 2026 at 4:35 AM Amir Goldstein <amir73il@gmail.com> wrote:
> On Fri, Mar 27, 2026 at 11:05 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > Stacked filesystems such as overlayfs do not currently provide the
> > necessary mechanisms for LSMs to properly enforce access controls on the
> > mmap() and mprotect() operations. In order to resolve this gap, a LSM
> > security blob is being added to the backing_file struct and the following
> > new LSM hooks are being created ...
...
> > diff --git a/fs/file_table.c b/fs/file_table.c
> > index aaa5faaace1e..0bdc26cae138 100644
> > --- a/fs/file_table.c
> > +++ b/fs/file_table.c
> > @@ -50,6 +50,7 @@ struct backing_file {
> > struct path user_path;
> > freeptr_t bf_freeptr;
> > };
> > + void *security;
>
> This needs ifdef SECURITY
Yep. That will require some other changes, but I should be able to
keep those fairly limited.
> and the name should be user_security
I'd strongly prefer to keep it as "security" both because "void
*security" is a very common pattern in the kernel when adding LSM
blobs to structs, and we don't know what each and every LSM may need
to store in the backing_file LSM blob.
> > +void *backing_file_security(const struct file *f)
> > +{
> > + return backing_file(f)->security;
> > +}
>
> I prefer the name backing_file_user_security()
>
> Terminology here is very confusing but when saying
> "backing file" it is more natural that one is referring to the
> backing xfs file with overlayfs has opened.
>
> The "backing file" already has an LSM blob f->f_security
> which is fair the call it the "backing file's LSM blob" ...
From a LSM dev's perspective, I would call file->f_security blob a
file's LSM blob regardless of if the file is a user or backing file;
the backing_file->security blob would be a backing file LSM blob. If
you look at what the SELinux code does, specifically in the
__file_has_perm() and __file_map_prot_check() functions (newly added
in this patchset), you'll see this supported by the code: the
file->f_security field is basically handled the same regardless of if
it is a user or backing file whereas the backing_file->security field
is handled very differently because it is a backing file.
While I think it makes sense to match the accessor function's name to
the struct field, ultimately I care more about the struct field's
name. If you really feel strongly about changing
backing_file_security() to backing_file_user_security() I can live
with that so long as we keep backing_file->security intact.
> > @@ -73,8 +79,11 @@ static inline void file_free(struct file *f)
> > percpu_counter_dec(&nr_files);
> > put_cred(f->f_cred);
> > if (unlikely(f->f_mode & FMODE_BACKING)) {
> > - path_put(backing_file_user_path(f));
> > - kmem_cache_free(bfilp_cachep, backing_file(f));
> > + struct backing_file *ff = backing_file(f);
> > +
> > + security_backing_file_free(&ff->security);
> > + path_put(&ff->user_path);
> > + kmem_cache_free(bfilp_cachep, ff);
>
> Not directly related to your patch, but as this is growing, IMO
> this would look cleaner with backing_file_free() inline helper
> (see attached path).
Sure, I'll include your patch in the patchset. I'll also fix the
comment style in the patch to match C style in the rest of the file
(I'll note the change in the metadata).
> > } else {
> > kmem_cache_free(filp_cachep, f);
> > }
> > @@ -290,7 +299,8 @@ struct file *alloc_empty_file_noaccount(int flags, const struct cred *cred)
> > * This is only for kernel internal use, and the allocate file must not be
> > * installed into file tables or such.
> > */
> > -struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
> > +struct file *alloc_empty_backing_file(int flags, const struct cred *cred,
> > + const struct file *user_file)
> > {
> > struct backing_file *ff;
> > int error;
> > @@ -306,6 +316,11 @@ struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
> > }
> >
> > ff->file.f_mode |= FMODE_BACKING | FMODE_NOACCOUNT;
> > + error = security_backing_file_alloc(&ff->security, user_file);
> > + if (unlikely(error)) {
> > + fput(&ff->file);
> > + return ERR_PTR(error);
> > + }
> > return &ff->file;
> > }
>
> There is an API issue here.
> in order to call fput() we must ensure that user_security was initialized to
> NULL (or allocated).
>
> I don't think that we want security_backing_file_alloc() to provide this
> semantic and the current patch does not implement it.
>
> Furthermore, user_path is also not initialized in the error case.
>
> Attached UNTESTED fixup patch to suggest a cleanup with
> init_backing_file() helper.
>
> It also changes the variable and helper name to user_security
> and plays some trick to avoid many ifdef SECURITY.
> Feel free to take whichever bits you like with/without attribution.
I think I've got a cleaner approach than what you've proposed, let me
code it up, test it all, and I'll post a new revision of the patchset
soon.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 6/7] tomoyo: Convert from sb_mount to granular mount hooks
From: Song Liu @ 2026-03-30 20:04 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Song Liu, Christian Brauner, viro@zeniv.linux.org.uk,
paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com,
jack@suse.cz, john.johansen@canonical.com,
stephen.smalley.work@gmail.com, omosnace@redhat.com,
mic@digikod.net, gnoack@google.com, takedakn@nttdata.co.jp,
herton@canonical.com, Kernel Team, selinux@vger.kernel.org,
apparmor@lists.ubuntu.com, linux-fsdevel@vger.kernel.org,
linux-security-module@vger.kernel.org
In-Reply-To: <4f5d1b1f-ecb2-421a-8a46-36c7a12d48de@I-love.SAKURA.ne.jp>
Hi Tetsuo,
On Tue, Mar 24, 2026 at 6:02 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
[...]
> >
> > If I understand Christian correctly, the main challenge here is that
> > FS_REQUIRES_DEV doesn't imply fc->source is the path of a device.
>
> Correct. FS_REQUIRES_DEV no longer implies that fc->source is a pathname.
>
> > Changing this assumption is a major change between VFS and many
> > filesystems.
>
> Wrong. I'm not trying to change this assumption. I'm trying to move LSM hook
> to a location after fc->source was interpreted by individual filesystem.
After spending some more time on this, I think we should not add
fc->source_path. This is because in many cases, dev_name for a new mount
is not a dev, or a path. For example, I can create a btrfs raid0 volume on two
devices, and use either of them as dev_name to mount the volume. It is not
accurate to put the path of either device in fc->source_path.
OTOH, if a LSM really wants to monitor the devices used by this mount,
security_sb_kern_mount() is a better hook to use. This will require the
LSM understands s_fs_info for specific file systems, which is not easy but
necessary.
For now, I think it makes sense to keep security_mount_new() in this set
as-is, and let LSMs like tomoyo and apparmor call kern_path when
necessary.
Thanks,
Song
^ permalink raw reply
* Re: [PATCH v8 01/12] lsm: Add LSM hook security_unix_find
From: Günther Noack @ 2026-03-30 19:02 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Paul Moore, John Johansen, Georgia Garcia, James Morris,
Serge E . Hallyn, Tingmao Wang, Justin Suess,
linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Jann Horn, Tahera Fahimi, Sebastian Andrzej Siewior,
Kuniyuki Iwashima, Simon Horman, netdev, Alexander Viro,
Christian Brauner
In-Reply-To: <20260330.ie1eth0ex9Pa@digikod.net>
On Mon, Mar 30, 2026 at 06:02:05PM +0200, Mickaël Salaün wrote:
> On Fri, Mar 27, 2026 at 01:55:58PM -0400, Paul Moore wrote:
> > On Fri, Mar 27, 2026 at 12:49 PM Günther Noack <gnoack3000@gmail.com> wrote:
> > > Cc: Günther Noack <gnoack3000@gmail.com>
> > > Cc: Tingmao Wang <m@maowtm.org>
> > > Cc: Mickaël Salaün <mic@digikod.net>
> > > Cc: Paul Moore <paul@paul-moore.com>
> > > Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> > > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > > ---
> > > include/linux/lsm_hook_defs.h | 5 +++++
> > > include/linux/security.h | 11 +++++++++++
> > > net/unix/af_unix.c | 10 +++++++---
> > > security/security.c | 20 ++++++++++++++++++++
> > > 4 files changed, 43 insertions(+), 3 deletions(-)
> >
> > This patch doesn't look like it changed significantly in this
> > revision, is there a reason you dropped the tags from Georgia and I?
>
> You'r right, the patch didn't change at all. I added Georgia's tag in my
> -next branch for the previous version, I guess Günther forgot to add it
> for this version, but I updated my branch with the same tag, so it's
> still there. Thank you both BTW!
I only missed to add them by accident, thanks for adding it back,
Mickaël, and Paul for spotting it!
> I just included a one-line fix because of the m68k warning, we'll see if
> it works as expected, and we should be good to go. It would be nice to
> have John's feedback though.
Agreed. John, I would also appreciate if you could find the time to
have a look.
–Günther
^ permalink raw reply
* Re: [PATCH v8 03/12] landlock: Replace union access_masks_all with helper functions
From: Günther Noack @ 2026-03-30 19:00 UTC (permalink / raw)
To: Mickaël Salaün
Cc: John Johansen, kernel test robot, linux-security-module,
Tingmao Wang, Justin Suess, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
Demi Marie Obenour, Alyssa Ross, Jann Horn, Tahera Fahimi,
Sebastian Andrzej Siewior, Kuniyuki Iwashima, Georgia Garcia
In-Reply-To: <20260330.laew6AthooD6@digikod.net>
On Mon, Mar 30, 2026 at 12:53:21PM +0200, Mickaël Salaün wrote:
> On Mon, Mar 30, 2026 at 11:56:40AM +0200, Mickaël Salaün wrote:
> > On Fri, Mar 27, 2026 at 05:48:28PM +0100, Günther Noack wrote:
> > > * Stop using a union for access_masks_all.
> > > * Expose helper functions for intersection checks and union operations.
> > >
> > > The memory layout of bitfields is only loosely defined by the C
> > > standard, so our static assertion that expects a fixed size was
> > > brittle, and it broke on some compilers when we attempted to add a
> > > 17th file system access right.
> > >
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Closes: https://lore.kernel.org/oe-kbuild-all/202603261438.jBx2DGNe-lkp@intel.com/
> > > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > > ---
> > > security/landlock/access.h | 21 ++++++++++++++-------
> > > security/landlock/cred.h | 10 ++--------
> > > security/landlock/ruleset.h | 13 ++++---------
> > > 3 files changed, 20 insertions(+), 24 deletions(-)
> >
> > I'd prefer this approach:
> >
> > diff --git a/security/landlock/access.h b/security/landlock/access.h
> > index 89dc8e7b93da..bc9efbb5c900 100644
> > --- a/security/landlock/access.h
> > +++ b/security/landlock/access.h
> > @@ -50,7 +50,7 @@ struct access_masks {
> > access_mask_t fs : LANDLOCK_NUM_ACCESS_FS;
> > access_mask_t net : LANDLOCK_NUM_ACCESS_NET;
> > access_mask_t scope : LANDLOCK_NUM_SCOPE;
> > -};
> > +} __packed;
>
> Actually, we can just use '__packed __aligned(sizeof(u32))' and avoid
> the static_assert change. That would have no impact on x86, but pack it
> on m68k.
Thanks, good catch (and thanks for pushing it to mic-next).
Fingers crossed that this works on m68k.
–Günther
^ permalink raw reply
* Re: [PATCH v8 01/12] lsm: Add LSM hook security_unix_find
From: Mickaël Salaün @ 2026-03-30 16:02 UTC (permalink / raw)
To: Paul Moore, John Johansen, Georgia Garcia
Cc: Günther Noack, James Morris, Serge E . Hallyn, Tingmao Wang,
Justin Suess, linux-security-module, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
Demi Marie Obenour, Alyssa Ross, Jann Horn, Tahera Fahimi,
Sebastian Andrzej Siewior, Kuniyuki Iwashima, Simon Horman,
netdev, Alexander Viro, Christian Brauner
In-Reply-To: <CAHC9VhRktViRp_U3ZAvo2e7vbjUd6RZb-y8+YPXhtLpt7hsXYg@mail.gmail.com>
On Fri, Mar 27, 2026 at 01:55:58PM -0400, Paul Moore wrote:
> On Fri, Mar 27, 2026 at 12:49 PM Günther Noack <gnoack3000@gmail.com> wrote:
> >
> > From: Justin Suess <utilityemal77@gmail.com>
> >
> > Add an LSM hook security_unix_find.
> >
> > This hook is called to check the path of a named UNIX socket before a
> > connection is initiated. The peer socket may be inspected as well.
> >
> > Why existing hooks are unsuitable:
> >
> > Existing socket hooks, security_unix_stream_connect(),
> > security_unix_may_send(), and security_socket_connect() don't provide
> > TOCTOU-free / namespace independent access to the paths of sockets.
> >
> > (1) We cannot resolve the path from the struct sockaddr in existing hooks.
> > This requires another path lookup. A change in the path between the
> > two lookups will cause a TOCTOU bug.
> >
> > (2) We cannot use the struct path from the listening socket, because it
> > may be bound to a path in a different namespace than the caller,
> > resulting in a path that cannot be referenced at policy creation time.
> >
> > Consumers of the hook wishing to reference @other are responsible
> > for acquiring the unix_state_lock and checking for the SOCK_DEAD flag
> > therein, ensuring the socket hasn't died since lookup.
> >
> > Cc: Günther Noack <gnoack3000@gmail.com>
> > Cc: Tingmao Wang <m@maowtm.org>
> > Cc: Mickaël Salaün <mic@digikod.net>
> > Cc: Paul Moore <paul@paul-moore.com>
> > Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > ---
> > include/linux/lsm_hook_defs.h | 5 +++++
> > include/linux/security.h | 11 +++++++++++
> > net/unix/af_unix.c | 10 +++++++---
> > security/security.c | 20 ++++++++++++++++++++
> > 4 files changed, 43 insertions(+), 3 deletions(-)
>
> This patch doesn't look like it changed significantly in this
> revision, is there a reason you dropped the tags from Georgia and I?
You'r right, the patch didn't change at all. I added Georgia's tag in my
-next branch for the previous version, I guess Günther forgot to add it
for this version, but I updated my branch with the same tag, so it's
still there. Thank you both BTW!
I just included a one-line fix because of the m68k warning, we'll see if
it works as expected, and we should be good to go. It would be nice to
have John's feedback though.
^ permalink raw reply
* Re: [PATCH v8 03/12] landlock: Replace union access_masks_all with helper functions
From: Mickaël Salaün @ 2026-03-30 10:53 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, kernel test robot, linux-security-module,
Tingmao Wang, Justin Suess, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
Demi Marie Obenour, Alyssa Ross, Jann Horn, Tahera Fahimi,
Sebastian Andrzej Siewior, Kuniyuki Iwashima, Georgia Garcia
In-Reply-To: <20260330.ohsoe7vu2Nob@digikod.net>
On Mon, Mar 30, 2026 at 11:56:40AM +0200, Mickaël Salaün wrote:
> On Fri, Mar 27, 2026 at 05:48:28PM +0100, Günther Noack wrote:
> > * Stop using a union for access_masks_all.
> > * Expose helper functions for intersection checks and union operations.
> >
> > The memory layout of bitfields is only loosely defined by the C
> > standard, so our static assertion that expects a fixed size was
> > brittle, and it broke on some compilers when we attempted to add a
> > 17th file system access right.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202603261438.jBx2DGNe-lkp@intel.com/
> > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > ---
> > security/landlock/access.h | 21 ++++++++++++++-------
> > security/landlock/cred.h | 10 ++--------
> > security/landlock/ruleset.h | 13 ++++---------
> > 3 files changed, 20 insertions(+), 24 deletions(-)
>
> I'd prefer this approach:
>
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 89dc8e7b93da..bc9efbb5c900 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -50,7 +50,7 @@ struct access_masks {
> access_mask_t fs : LANDLOCK_NUM_ACCESS_FS;
> access_mask_t net : LANDLOCK_NUM_ACCESS_NET;
> access_mask_t scope : LANDLOCK_NUM_SCOPE;
> -};
> +} __packed;
Actually, we can just use '__packed __aligned(sizeof(u32))' and avoid
the static_assert change. That would have no impact on x86, but pack it
on m68k.
>
> union access_masks_all {
> struct access_masks masks;
> @@ -58,7 +58,7 @@ union access_masks_all {
> };
>
> /* Makes sure all fields are covered. */
> -static_assert(sizeof(typeof_member(union access_masks_all, masks)) ==
> +static_assert(sizeof(typeof_member(union access_masks_all, masks)) <=
> sizeof(typeof_member(union access_masks_all, all)));
>
> /**
>
>
> This keep the check and make sure new field will not introduce issues, but more
> importantly it save memory, which was one of the goal of struct access_masks.
>
> If that's good with you I'll replace your patch and squash this packed
> annotation with the following patch.
>
>
> >
> > diff --git a/security/landlock/access.h b/security/landlock/access.h
> > index 42c95747d7bd..277b6ed7f7bb 100644
> > --- a/security/landlock/access.h
> > +++ b/security/landlock/access.h
> > @@ -52,14 +52,21 @@ struct access_masks {
> > access_mask_t scope : LANDLOCK_NUM_SCOPE;
> > };
> >
> > -union access_masks_all {
> > - struct access_masks masks;
> > - u32 all;
> > -};
> > +/* Checks whether two access masks have any common bit set. */
> > +static inline bool access_masks_intersect(const struct access_masks a,
> > + const struct access_masks b)
> > +{
> > + return (a.fs & b.fs) || (a.net & b.net) || (a.scope & b.scope);
> > +}
> >
> > -/* Makes sure all fields are covered. */
> > -static_assert(sizeof(typeof_member(union access_masks_all, masks)) ==
> > - sizeof(typeof_member(union access_masks_all, all)));
> > +/* ORs the bits of @src into @dst. */
> > +static inline void access_masks_merge(struct access_masks *dst,
> > + const struct access_masks src)
> > +{
> > + dst->fs |= src.fs;
> > + dst->net |= src.net;
> > + dst->scope |= src.scope;
> > +}
> >
> > /**
> > * struct layer_access_masks - A boolean matrix of layers and access rights
> > diff --git a/security/landlock/cred.h b/security/landlock/cred.h
> > index f287c56b5fd4..207a6db1c086 100644
> > --- a/security/landlock/cred.h
> > +++ b/security/landlock/cred.h
> > @@ -123,9 +123,6 @@ landlock_get_applicable_subject(const struct cred *const cred,
> > const struct access_masks masks,
> > size_t *const handle_layer)
> > {
> > - const union access_masks_all masks_all = {
> > - .masks = masks,
> > - };
> > const struct landlock_ruleset *domain;
> > ssize_t layer_level;
> >
> > @@ -138,11 +135,8 @@ landlock_get_applicable_subject(const struct cred *const cred,
> >
> > for (layer_level = domain->num_layers - 1; layer_level >= 0;
> > layer_level--) {
> > - union access_masks_all layer = {
> > - .masks = domain->access_masks[layer_level],
> > - };
> > -
> > - if (layer.all & masks_all.all) {
> > + if (access_masks_intersect(domain->access_masks[layer_level],
> > + masks)) {
> > if (handle_layer)
> > *handle_layer = layer_level;
> >
> > diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> > index 889f4b30301a..9f8b33815c2c 100644
> > --- a/security/landlock/ruleset.h
> > +++ b/security/landlock/ruleset.h
> > @@ -229,18 +229,13 @@ static inline void landlock_get_ruleset(struct landlock_ruleset *const ruleset)
> > static inline struct access_masks
> > landlock_union_access_masks(const struct landlock_ruleset *const domain)
> > {
> > - union access_masks_all matches = {};
> > + struct access_masks matches = {};
> > size_t layer_level;
> >
> > - for (layer_level = 0; layer_level < domain->num_layers; layer_level++) {
> > - union access_masks_all layer = {
> > - .masks = domain->access_masks[layer_level],
> > - };
> > + for (layer_level = 0; layer_level < domain->num_layers; layer_level++)
> > + access_masks_merge(&matches, domain->access_masks[layer_level]);
> >
> > - matches.all |= layer.all;
> > - }
> > -
> > - return matches.masks;
> > + return matches;
> > }
> >
> > static inline void
> > --
> > 2.53.0
> >
> >
^ permalink raw reply
* Re: [PATCH v8 03/12] landlock: Replace union access_masks_all with helper functions
From: Mickaël Salaün @ 2026-03-30 9:56 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, kernel test robot, linux-security-module,
Tingmao Wang, Justin Suess, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
Demi Marie Obenour, Alyssa Ross, Jann Horn, Tahera Fahimi,
Sebastian Andrzej Siewior, Kuniyuki Iwashima, Georgia Garcia
In-Reply-To: <20260327164838.38231-4-gnoack3000@gmail.com>
On Fri, Mar 27, 2026 at 05:48:28PM +0100, Günther Noack wrote:
> * Stop using a union for access_masks_all.
> * Expose helper functions for intersection checks and union operations.
>
> The memory layout of bitfields is only loosely defined by the C
> standard, so our static assertion that expects a fixed size was
> brittle, and it broke on some compilers when we attempted to add a
> 17th file system access right.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202603261438.jBx2DGNe-lkp@intel.com/
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> security/landlock/access.h | 21 ++++++++++++++-------
> security/landlock/cred.h | 10 ++--------
> security/landlock/ruleset.h | 13 ++++---------
> 3 files changed, 20 insertions(+), 24 deletions(-)
I'd prefer this approach:
diff --git a/security/landlock/access.h b/security/landlock/access.h
index 89dc8e7b93da..bc9efbb5c900 100644
--- a/security/landlock/access.h
+++ b/security/landlock/access.h
@@ -50,7 +50,7 @@ struct access_masks {
access_mask_t fs : LANDLOCK_NUM_ACCESS_FS;
access_mask_t net : LANDLOCK_NUM_ACCESS_NET;
access_mask_t scope : LANDLOCK_NUM_SCOPE;
-};
+} __packed;
union access_masks_all {
struct access_masks masks;
@@ -58,7 +58,7 @@ union access_masks_all {
};
/* Makes sure all fields are covered. */
-static_assert(sizeof(typeof_member(union access_masks_all, masks)) ==
+static_assert(sizeof(typeof_member(union access_masks_all, masks)) <=
sizeof(typeof_member(union access_masks_all, all)));
/**
This keep the check and make sure new field will not introduce issues, but more
importantly it save memory, which was one of the goal of struct access_masks.
If that's good with you I'll replace your patch and squash this packed
annotation with the following patch.
>
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 42c95747d7bd..277b6ed7f7bb 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -52,14 +52,21 @@ struct access_masks {
> access_mask_t scope : LANDLOCK_NUM_SCOPE;
> };
>
> -union access_masks_all {
> - struct access_masks masks;
> - u32 all;
> -};
> +/* Checks whether two access masks have any common bit set. */
> +static inline bool access_masks_intersect(const struct access_masks a,
> + const struct access_masks b)
> +{
> + return (a.fs & b.fs) || (a.net & b.net) || (a.scope & b.scope);
> +}
>
> -/* Makes sure all fields are covered. */
> -static_assert(sizeof(typeof_member(union access_masks_all, masks)) ==
> - sizeof(typeof_member(union access_masks_all, all)));
> +/* ORs the bits of @src into @dst. */
> +static inline void access_masks_merge(struct access_masks *dst,
> + const struct access_masks src)
> +{
> + dst->fs |= src.fs;
> + dst->net |= src.net;
> + dst->scope |= src.scope;
> +}
>
> /**
> * struct layer_access_masks - A boolean matrix of layers and access rights
> diff --git a/security/landlock/cred.h b/security/landlock/cred.h
> index f287c56b5fd4..207a6db1c086 100644
> --- a/security/landlock/cred.h
> +++ b/security/landlock/cred.h
> @@ -123,9 +123,6 @@ landlock_get_applicable_subject(const struct cred *const cred,
> const struct access_masks masks,
> size_t *const handle_layer)
> {
> - const union access_masks_all masks_all = {
> - .masks = masks,
> - };
> const struct landlock_ruleset *domain;
> ssize_t layer_level;
>
> @@ -138,11 +135,8 @@ landlock_get_applicable_subject(const struct cred *const cred,
>
> for (layer_level = domain->num_layers - 1; layer_level >= 0;
> layer_level--) {
> - union access_masks_all layer = {
> - .masks = domain->access_masks[layer_level],
> - };
> -
> - if (layer.all & masks_all.all) {
> + if (access_masks_intersect(domain->access_masks[layer_level],
> + masks)) {
> if (handle_layer)
> *handle_layer = layer_level;
>
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index 889f4b30301a..9f8b33815c2c 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -229,18 +229,13 @@ static inline void landlock_get_ruleset(struct landlock_ruleset *const ruleset)
> static inline struct access_masks
> landlock_union_access_masks(const struct landlock_ruleset *const domain)
> {
> - union access_masks_all matches = {};
> + struct access_masks matches = {};
> size_t layer_level;
>
> - for (layer_level = 0; layer_level < domain->num_layers; layer_level++) {
> - union access_masks_all layer = {
> - .masks = domain->access_masks[layer_level],
> - };
> + for (layer_level = 0; layer_level < domain->num_layers; layer_level++)
> + access_masks_merge(&matches, domain->access_masks[layer_level]);
>
> - matches.all |= layer.all;
> - }
> -
> - return matches.masks;
> + return matches;
> }
>
> static inline void
> --
> 2.53.0
>
>
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox