* Re: [PATCH v5 06/13] ima: Mediate open/release method of the measurements list
From: Roberto Sassu @ 2026-05-21 8:30 UTC (permalink / raw)
To: Mimi Zohar, corbet, skhan, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <db872f810f22bf25ff0ae7fe15b44f316b078079.camel@linux.ibm.com>
On Wed, 2026-05-20 at 22:07 -0400, Mimi Zohar wrote:
> On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> >
> > Introduce the ima_measure_users counter, to implement a semaphore-like
> > locking scheme where the binary and ASCII measurements list interfaces can
> > be concurrently open by multiple readers, or alternatively by a single
> > writer.
> >
> > A semaphore cannot be used because the kernel cannot return to user space
> > with a lock held.
> >
> > Introduce the ima_measure_lock() and ima_measure_unlock() primitives, to
> > respectively lock/unlock the interfaces (safely with the ima_measure_users
> > counter, without holding a lock).
> >
> > Finally, introduce _ima_measurements_open() to lock the interface before
> > seq_open(), and call it from ima_measurements_open() and
> > ima_ascii_measurements_open(). And, introduce ima_measurements_release(),
> > to unlock the interface.
> >
> > Require CAP_SYS_ADMIN if the interface is opened for write (not possible
> > for the current measurements interfaces, since they only have read
> > permission).
> >
> > No functional changes: multiple readers are allowed as before.
> >
> > Link: https://github.com/linux-integrity/linux/issues/1
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > ---
> > security/integrity/ima/ima_fs.c | 71 +++++++++++++++++++++++++++++++--
> > 1 file changed, 67 insertions(+), 4 deletions(-)
> >
> > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> > index 9a8dba14d82a..68edea7139d5 100644
> > --- a/security/integrity/ima/ima_fs.c
> > +++ b/security/integrity/ima/ima_fs.c
> > @@ -25,6 +25,8 @@
> > #include "ima.h"
> >
> > static DEFINE_MUTEX(ima_write_mutex);
> > +static DEFINE_MUTEX(ima_measure_mutex);
> > +static long ima_measure_users;
>
> long?
The limit pre process can be up to INT_MAX. Two processes could
overflow the counter if it was int.
Since privileged users can bypass the system wide max-file check (see
alloc_empty_file()), I will add an overflow check to be sure.
> >
> > bool ima_canonical_fmt;
> > static int __init default_canonical_fmt_setup(char *str)
> > @@ -209,16 +211,76 @@ static const struct seq_operations ima_measurments_seqops = {
> > .show = ima_measurements_show
> > };
> >
> > +static int ima_measure_lock(bool write)
> > +{
> > + mutex_lock(&ima_measure_mutex);
> > + if ((write && ima_measure_users != 0) ||
> > + (!write && ima_measure_users < 0)) {
> > + mutex_unlock(&ima_measure_mutex);
> > + return -EBUSY;
> > + }
>
> Thanks, Roberto. The code is really clear and well written. However, it could
> use a comment indicating the different ima_measure_users values as a reminder.
>
> ima_measure_users: > 0 open readers
> ima_meaasure_users: == -1 open writer
Ok.
> > +
> > + if (write)
> > + ima_measure_users--;
> > + else
> > + ima_measure_users++;
> > + mutex_unlock(&ima_measure_mutex);
> > + return 0;
> > +}
> > +
> > +static void ima_measure_unlock(bool write)
> > +{
> > + mutex_lock(&ima_measure_mutex);
> > + if (write)
> > + ima_measure_users++;
>
> There should only be one writer at a time. ima_measure_users could be set to
> zero.
Sure, but I find the code more clear this way.
Roberto
> > + else
> > + ima_measure_users--;
> > + mutex_unlock(&ima_measure_mutex);
> > +}
> > +
>
> thanks,
>
> Mimi
^ permalink raw reply
* Re: [PATCH v5 04/13] ima: Introduce per binary measurements list type binary_runtime_size value
From: Roberto Sassu @ 2026-05-21 7:58 UTC (permalink / raw)
To: Mimi Zohar, corbet, skhan, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <b7f97a0a3b79b72a014d12514febc338d1ecd038.camel@linux.ibm.com>
On Wed, 2026-05-20 at 22:06 -0400, Mimi Zohar wrote:
> On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> >
> > Make binary_runtime_size as an array, to have separate counters per binary
> > measurements list type. Currently, define the BINARY type for the existing
> > binary measurements list.
> >
> > Introduce ima_update_binary_runtime_size() to facilitate updating a
> > binary_runtime_size value with a given binary measurement list type.
> >
> > Also add the binary measurements list type parameter to
> > ima_get_binary_runtime_size(), to retrieve the desired value. Retrieving
> > the value is now done under the ima_extend_list_mutex, since there can be
> > concurrent updates.
> >
> > No functional change (except for the mutex usage, that fixes the
> > concurrency issue): the BINARY array element is equivalent to the old
> > binary_runtime_size.
>
> The patch is really clear and well written, but I don't see a concurrency issue
> requiring taking the ima_extend_list_mutex at least in this patch.
binary_runtime_size is not an atomic variable. It is updated under the
ima_extend_list_mutex lock in ima_add_digest_entry(). The same lock
must be taken on the reader side, ima_get_binary_runtime_size().
Roberto
^ permalink raw reply
* [PATCH] apparmor: Fix inverted comparison in cache_hold_inc()
From: Eduardo Vasconcelos @ 2026-05-21 6:57 UTC (permalink / raw)
To: john.johansen, paul, jmorris, serge
Cc: Eduardo Vasconcelos, apparmor, linux-security-module,
linux-kernel
cache_hold_inc() prevents the per-CPU cache hold counter from
rising above MAX_HOLD_COUNT, but the comparison is inverted
(> MAX_HOLD_COUNT instead of <), so the counter never rises
above 0.
This breaks the cache mechanism because since the hold counter
is always 0, the global pool is always attempted first before
falling back to the local cache. The decrement also never occurs,
thus the hold counter is effectively dead.
Fix by changing > to < in cache_hold_inc().
Signed-off-by: Eduardo Vasconcelos <eduardo@eduardovasconcelos.com>
---
security/apparmor/lsm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 3491e9f60194..b7c19805a216 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -2129,7 +2129,7 @@ static int param_set_mode(const char *val, const struct kernel_param *kp)
*/
static void cache_hold_inc(unsigned int *hold)
{
- if (*hold > MAX_HOLD_COUNT)
+ if (*hold < MAX_HOLD_COUNT)
(*hold)++;
}
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v5 08/13] ima: Introduce ima_dump_measurement()
From: Mimi Zohar @ 2026-05-21 2:07 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260429160319.4162918-9-roberto.sassu@huaweicloud.com>
On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduce ima_dump_measurement() to simplify the code of
> ima_dump_measurement_list() and to avoid repeating the
> ima_dump_measurement() code block if iteration occurs on multiple lists.
>
> No functional change: only code moved to a separate function.
>
> Link: https://github.com/linux-integrity/linux/issues/1
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v5 07/13] ima: Use snprintf() in create_securityfs_measurement_lists
From: Mimi Zohar @ 2026-05-21 2:07 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260429160319.4162918-8-roberto.sassu@huaweicloud.com>
On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Use the more secure snprintf() function (accepting the buffer size) in
> create_securityfs_measurement_lists().
>
> No functional change: sprintf() and snprintf() have the same behavior.
>
> Link: https://github.com/linux-integrity/linux/issues/1
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v5 06/13] ima: Mediate open/release method of the measurements list
From: Mimi Zohar @ 2026-05-21 2:07 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260429160319.4162918-7-roberto.sassu@huaweicloud.com>
On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduce the ima_measure_users counter, to implement a semaphore-like
> locking scheme where the binary and ASCII measurements list interfaces can
> be concurrently open by multiple readers, or alternatively by a single
> writer.
>
> A semaphore cannot be used because the kernel cannot return to user space
> with a lock held.
>
> Introduce the ima_measure_lock() and ima_measure_unlock() primitives, to
> respectively lock/unlock the interfaces (safely with the ima_measure_users
> counter, without holding a lock).
>
> Finally, introduce _ima_measurements_open() to lock the interface before
> seq_open(), and call it from ima_measurements_open() and
> ima_ascii_measurements_open(). And, introduce ima_measurements_release(),
> to unlock the interface.
>
> Require CAP_SYS_ADMIN if the interface is opened for write (not possible
> for the current measurements interfaces, since they only have read
> permission).
>
> No functional changes: multiple readers are allowed as before.
>
> Link: https://github.com/linux-integrity/linux/issues/1
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
> security/integrity/ima/ima_fs.c | 71 +++++++++++++++++++++++++++++++--
> 1 file changed, 67 insertions(+), 4 deletions(-)
>
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 9a8dba14d82a..68edea7139d5 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -25,6 +25,8 @@
> #include "ima.h"
>
> static DEFINE_MUTEX(ima_write_mutex);
> +static DEFINE_MUTEX(ima_measure_mutex);
> +static long ima_measure_users;
long?
>
> bool ima_canonical_fmt;
> static int __init default_canonical_fmt_setup(char *str)
> @@ -209,16 +211,76 @@ static const struct seq_operations ima_measurments_seqops = {
> .show = ima_measurements_show
> };
>
> +static int ima_measure_lock(bool write)
> +{
> + mutex_lock(&ima_measure_mutex);
> + if ((write && ima_measure_users != 0) ||
> + (!write && ima_measure_users < 0)) {
> + mutex_unlock(&ima_measure_mutex);
> + return -EBUSY;
> + }
Thanks, Roberto. The code is really clear and well written. However, it could
use a comment indicating the different ima_measure_users values as a reminder.
ima_measure_users: > 0 open readers
ima_meaasure_users: == -1 open writer
> +
> + if (write)
> + ima_measure_users--;
> + else
> + ima_measure_users++;
> + mutex_unlock(&ima_measure_mutex);
> + return 0;
> +}
> +
> +static void ima_measure_unlock(bool write)
> +{
> + mutex_lock(&ima_measure_mutex);
> + if (write)
> + ima_measure_users++;
There should only be one writer at a time. ima_measure_users could be set to
zero.
> + else
> + ima_measure_users--;
> + mutex_unlock(&ima_measure_mutex);
> +}
> +
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v5 05/13] ima: Introduce _ima_measurements_start() and _ima_measurements_next()
From: Mimi Zohar @ 2026-05-21 2:06 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260429160319.4162918-6-roberto.sassu@huaweicloud.com>
On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduce _ima_measurements_start() and _ima_measurements_next(), renamed
> from ima_measurements_start() and ima_measurements_next(), to include the
> list head as an additional parameter, so that iteration on different lists
> can be implemented by calling those functions.
>
> No functional change: ima_measurements_start() and ima_measurements_next()
> pass the ima_measurements list head, used before.
ima_measurements_start() and ima_measurments_next() become "wrappers" for the
new functions.
>
> Link: https://github.com/linux-integrity/linux/issues/1
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v5 04/13] ima: Introduce per binary measurements list type binary_runtime_size value
From: Mimi Zohar @ 2026-05-21 2:06 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260429160319.4162918-5-roberto.sassu@huaweicloud.com>
On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Make binary_runtime_size as an array, to have separate counters per binary
> measurements list type. Currently, define the BINARY type for the existing
> binary measurements list.
>
> Introduce ima_update_binary_runtime_size() to facilitate updating a
> binary_runtime_size value with a given binary measurement list type.
>
> Also add the binary measurements list type parameter to
> ima_get_binary_runtime_size(), to retrieve the desired value. Retrieving
> the value is now done under the ima_extend_list_mutex, since there can be
> concurrent updates.
>
> No functional change (except for the mutex usage, that fixes the
> concurrency issue): the BINARY array element is equivalent to the old
> binary_runtime_size.
The patch is really clear and well written, but I don't see a concurrency issue
requiring taking the ima_extend_list_mutex at least in this patch.
Mimi
>
> Link: https://github.com/linux-integrity/linux/issues/1
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
^ permalink raw reply
* Re: [PATCH v5 03/13] ima: Introduce per binary measurements list type ima_num_entries counter
From: Mimi Zohar @ 2026-05-21 2:05 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260429160319.4162918-4-roberto.sassu@huaweicloud.com>
On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Make ima_num_entries as an array, to have separate counters per binary
> measurements list type. Currently, define the BINARY type for the existing
> binary measurements list.
>
> No functional change: the BINARY type is equivalent to the value without
> the array.
>
> Link: https://github.com/linux-integrity/linux/issues/1
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Thanks, Roberto.
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v5 02/13] ima: Replace static htable queue with dynamically allocated array
From: Mimi Zohar @ 2026-05-21 2:05 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260429160319.4162918-3-roberto.sassu@huaweicloud.com>
On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> The IMA hash table is a fixed-size array of hlist_head buckets:
>
> struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE];
>
> IMA_MEASURE_HTABLE_SIZE is (1 << IMA_HASH_BITS) = 1024 buckets, each a
> struct hlist_head (one pointer, 8 bytes on 64-bit). That is 8 KiB allocated
> in BSS for every kernel, regardless of whether IMA is ever used, and
> regardless of how many measurements are actually made.
>
> Replace the fixed-size array with a RCU-protected pointer to a dynamically
> allocated array that is initialized in ima_init_htable(), which is called
> from ima_init() during early boot. ima_init_htable() calls the static
> function ima_alloc_replace_htable() which, other than initializing the hash
> table the first time, can also hot-swap the existing hash table with a
> blank one.
>
> The allocation in ima_alloc_replace_htable() uses kcalloc() so the buckets
> are zero-initialised (equivalent to HLIST_HEAD_INIT { .first = NULL }).
> Callers of ima_alloc_replace_htable() must call synchronize_rcu() and free
> the returned hash table.
>
> Finally, access the hash table with rcu_dereference() in
> ima_lookup_digest_entry() (reader side) and with
> rcu_dereference_protected() in ima_add_digest_entry() (writer side).
>
> No functional change: bucket count, hash function, and all locking remain
> identical.
>
> Link: https://github.com/linux-integrity/linux/issues/1
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v5 01/13] ima: Remove ima_h_table structure
From: Mimi Zohar @ 2026-05-21 2:05 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260429160319.4162918-2-roberto.sassu@huaweicloud.com>
On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
Instead of jumping straight to "With the upcoming change ...", some context is
needed. Perhaps something like:
The ima_h_table structure is a collection of IMA measurement list metadata -
number of records in the IMA measurement list, number of integrity violations,
and a hash table containing the IMA template data hash, needed to prevent
measurement list record duplication.
Removing records from the measurement list needs to be reflected in the hash
table. As a pre-req to removing records from the measurement list, separate ...
> With the upcoming change of dynamically allocating and replacing the hash
> table, the ima_h_table structure would have been replaced with a new one.
>
> However, since the ima_h_table structure contains also the counters for
> number of measurements entries and violations, we would have needed to
> preserve their values in the new ima_h_table structure.
>
> Instead, separate those counters from the hash table, remove the
> ima_h_table structure, and just replace the hash table pointer.
>
> Finally, rename ima_show_htable_value(), ima_show_htable_violations()
> and ima_htable_violations_ops respectively to ima_show_counter(),
> ima_show_num_violations() and ima_num_violations_ops.
>
> Link: https://github.com/linux-integrity/linux/issues/1
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Other than referring to "entries" in the measurement list, the patch looks good.
I prefer referring to them as "records".
> ---
> security/integrity/ima/ima.h | 9 +++------
> security/integrity/ima/ima_api.c | 2 +-
> security/integrity/ima/ima_fs.c | 19 +++++++++----------
> security/integrity/ima/ima_kexec.c | 2 +-
> security/integrity/ima/ima_queue.c | 17 ++++++++++-------
> 5 files changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 69e9bf0b82c6..51a8a582df56 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -324,12 +324,9 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
> */
> extern spinlock_t ima_queue_lock;
>
> -struct ima_h_table {
> - atomic_long_t len; /* number of stored measurements in the list */
> - atomic_long_t violations;
> - struct hlist_head queue[IMA_MEASURE_HTABLE_SIZE];
> -};
> -extern struct ima_h_table ima_htable;
> +extern atomic_long_t ima_num_entries;
-> ima_num_records /* Total number of measurement list records */
Will this be the current or total number of measurement list records since a
hard boot?
> +extern atomic_long_t ima_num_violations;
Similarly, will this be the current number or total number of violations since a
hard boot? Please add a comment.
> +extern struct hlist_head ima_htable[IMA_MEASURE_HTABLE_SIZE];
>
> static inline unsigned int ima_hash_key(u8 *digest)
> {
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v5 00/13] ima: Introduce staging mechanism
From: Mimi Zohar @ 2026-05-21 2:02 UTC (permalink / raw)
To: Roberto Sassu, corbet, skhan, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, chenste, nramas, Roberto Sassu
In-Reply-To: <20260429160319.4162918-1-roberto.sassu@huaweicloud.com>
On Wed, 2026-04-29 at 18:03 +0200, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduction
> ============
>
> The IMA measurements list is currently stored in the kernel memory.
> Memory occupation grows linearly with the number of entries, and can
> become a problem especially in environments with reduced resources.
>
> While there is an advantage in keeping the IMA measurements list in
> kernel memory, so that it is always available for reading from the
> securityfs interfaces, storing it elsewhere would make it possible to
> free precious memory for other kernel components.
-> for other kernel usage.
Prefix the following paragraph with:
The IMA measurement list needs to be retained and safely stored for new
attestation servers to validate the entire measurement list. Assuming the IMA
measurement list is properly saved, storing ...
> Storing the IMA measurements list outside the kernel does not introduce
> security issues, since its integrity is anyway protected by the TPM.
>
> Hence, the new IMA staging mechanism is introduced to allow user space
> to remove the desired portion of the measurements list from the kernel.
"desired portion" could be misconstrued as any subset of the measurement list.
-> to remove the entire or a portion of the measurement list ...
>
> Usage
> =====
> The IMA staging mechanism can be enabled from the kernel configuration
> with the CONFIG_IMA_STAGING option.
Continue with:
This option prevents inadvertently removing the IMA measurement list on systems
which do not properly save it.
>
> If it is enabled, IMA duplicates the current measurements interfaces
-> duplicates the current securityfs measurement list interfaces
> (both binary and ASCII), by adding the _staged file suffix. Both the
> original and the staging interfaces gain the write permission for the
> root user and group, but require the process to have CAP_SYS_ADMIN set.
>
> The staging mechanism supports two flavors.
>
> Staging with prompt
> ~~~~~~~~~~~~~~~~~~~
>
> The current measurements list is moved to a temporary staging area, and
> staged measurements are deleted upon confirmation.
-> The current measurement list is moved to a temporary staging area, allowing
it to be saved to external storage, before being deleted upon confirmation.
>
> This staging process is achieved with the following steps.
>
> 1. echo A > <original interface>: the user requests IMA to stage the
> entire measurements list;
> 2. cat <_staged interface>: the user reads the staged measurements;
> 3. echo D > <_staged interface>: the user requests IMA to delete
> staged measurements.
>
> Staging and deleting
> ~~~~~~~~~~~~~~~~~~~~
>
> N measurements are staged to a temporary staging area, and immediately
> deleted without further confirmation.
>
> This staging process is achieved with the following steps.
>
> 1. cat <original interface>: the user reads the current measurements
> list and determines what the value N for staging should be;
> 2. echo N > <original interface>: the user requests IMA to delete N
> measurements from the current measurements list.
>
>
> Management of Staged Measurements
> =================================
>
> Since with the staging mechanism measurement entries are removed from
> the kernel, the user needs to save the staged ones in a storage and
> concatenate them together, so that it can present them to remote
> attestation agents as if staging was never done.
"the user needs to save the staged ones" -> the staged measurements need to be
saved ....
Please mention this could be a system service.
thanks,
Mimi
^ permalink raw reply
* Re: [QUESTION] move load_uefi_certs() and keyring initcall to earlier initcall
From: Yeoreum Yun @ 2026-05-20 16:02 UTC (permalink / raw)
To: linux-kernel, linux-integrity, keyrings, linux-security-module
Cc: zohar, roberto.sassu, ardb, jarkko, dhowells, dwmw2, serge,
jmorris, paul, sami.mujawar, pierre.gondois
In-Reply-To: <agXP3ZyE18pAiy77@e129823.arm.com>
> Hi all,
>
> Recently, I've found possible module load failure via
> request_module() in device_initcall() for absent of certificate loaded by
> load_uefi_certs() in below exemplary case:
>
> - MokListTrustedRT is created.
> - one module is signed with trust-chain with one cert of MokListRT and
> contained in initramfs.
> - loading the above module in device_initcall() get failure since
> relavent certs didn't loaded yet -- load_uefi_certs() is called at
> late_inicall()
>
> I don't think calling request_module() in the initcall is not a problem
> if it's after root_initcall (from device_initcall) where initrd
> population is requested when I see the commit e7cb072eb988
> ("init/initramfs.c: do unpacking asynchronously").
>
> IOW, to address this -- signature verification failure for late loading
> of certificates, It seems to movce load_uefi_certs() or other relevant
> init functions requires to move to "rootfs_initcall()".
>
> Unfortunately, Moving the load_uefi_certs() doesn't seems easy because
> keyring infrastructure is initailised at device_initcall() via
> relevant init functions.
>
> So, I would like to introduce two initcalls macro which using pre-exist
> initcall macro to address above situation --
>
> - keyring_initcall() (which is wrapper of subsys_initcall()).
> - cert_initcall() (which is wrapper of rootfs_initcall()).
>
> so that for init functions which initalise keyring infrastructure,
> for example, the function where calls keyring_alloc() like:
> - system_trusted_keyring_init()
> - blacklist_init()
> - machine_keyring_init()
> - platform_keyring_init()
> - ima_mok_init()
>
> to be replaced from device_initcall() to subsys_initcall() with keyring_initcall()
>
> and for functions which load certificates or late init relevant keyring
> like:
> - load_system_certificate_list()
> - load_uefi_certs()
> - load_powerpc_certs()
> - load_ipl_certs()
> - big_key_init()
> - init_root_keyring()
> - init_trusted()
> - init_encrypted()
>
> to be called from late_initcall() to rootfs_initcall() with cert_initcall().
>
> Am I missing something, or is there perhaps a better idea?
>
I'd appreciate it if someone could let me know whether I might be
missing something.
Thanks!
--
Sincerely,
Yeoreum Yun
^ permalink raw reply
* Re: [GIT PULL] lsm/lsm-pr-20260519
From: pr-tracker-bot @ 2026-05-20 15:24 UTC (permalink / raw)
To: Paul Moore; +Cc: Linus Torvalds, linux-security-module, linux-kernel
In-Reply-To: <96f3b6072567c23ebabe1804a0622af5@paul-moore.com>
The pull request you sent on Tue, 19 May 2026 16:05:25 -0400:
> https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git tags/lsm-pr-20260519
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/e312f536f647156ac55e2f12d021cf887af274aa
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH] landlock: avoid memcpy static check warning
From: Arnd Bergmann @ 2026-05-20 11:45 UTC (permalink / raw)
To: Mickaël Salaün, Arnd Bergmann, Kees Cook,
Gustavo A. R. Silva
Cc: Paul Moore, James Morris, Serge E. Hallyn, Günther Noack,
Tingmao Wang, linux-security-module, linux-kernel
In-Reply-To: <20260520.iez2sheoc8Ae@digikod.net>
[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]
On Wed, May 20, 2026, at 11:10, Mickaël Salaün wrote:
> On Tue, May 19, 2026 at 10:30:05PM +0200, Arnd Bergmann wrote:
>>
>> Out of these individually helpful checks (-Wrestrict, fortified
>> string helpers, flex_array_size), one of them has to go to avoid
>> the warning.
>>
>> Seeing that the length of the array is already checked earlier
>> in this function, through both an explicit LANDLOCK_MAX_NUM_LAYERS
>> comparison and the implicit kzalloc_flex() having succeeded,
>> replace the flex_array_size() call with a direct multiplication.
>
> Can flex_array_size() be fixed instead?
I couldn't figure it out myself, but feel free to give it a try.
I've attached the two randconfig files that showed the problem
for me, as this only shows up very rarely.
Actually thinking about it again, I suspect that this is not
really a false positive but that gcc got things right by detecting
that flex_array_size() returns SIZE_MAX in case of an overflow,
and this would in fact cause data corruption when used as
the length in mempcy().
Arnd
[-- Attachment #2: 0xCD5395EB-config.gz --]
[-- Type: application/gzip, Size: 29766 bytes --]
[-- Attachment #3: 0xF0418B18-config.gz --]
[-- Type: application/gzip, Size: 32113 bytes --]
^ permalink raw reply
* Re: [PATCH] landlock: avoid memcpy static check warning
From: Mickaël Salaün @ 2026-05-20 9:10 UTC (permalink / raw)
To: Arnd Bergmann, Kees Cook, Gustavo A. R. Silva
Cc: Paul Moore, James Morris, Serge E. Hallyn, Arnd Bergmann,
Günther Noack, Tingmao Wang, linux-security-module,
linux-kernel
In-Reply-To: <20260519203012.1340274-1-arnd@kernel.org>
Thanks for the report.
On Tue, May 19, 2026 at 10:30:05PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The fortified string helpers trigger a -Wrestrict warning when
> gcc deducts that the size of the landlock_layer array can
> overflow as a result of the flex_array_size() calculation:
>
> In file included from arch/x86/include/asm/string.h:6,
> from security/landlock/ruleset.c:16:
> security/landlock/ruleset.c: In function 'create_rule':
> arch/x86/include/asm/string_32.h:150:25: error: '__builtin_memcpy' accessing 4294967295 bytes at offsets 0 and 0 overlaps 6442450943 bytes at offset -2147483648 [-Werror=restrict]
> 150 | #define memcpy(t, f, n) __builtin_memcpy(t, f, n)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~
> security/landlock/ruleset.c:139:9: note: in expansion of macro 'memcpy'
> 139 | memcpy(new_rule->layers, layers,
> | ^~~~~~
> 'create_rule': event 1
> include/linux/compiler.h:69:46:
> 68 | (cond) ? \
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 69 | (__if_trace.miss_hit[1]++,1) : \
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
> | |
> | (1) when the condition is evaluated to true
> 70 | (__if_trace.miss_hit[0]++,0); \
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/linux/compiler.h:57:69: note: in expansion of macro '__trace_if_value'
> 57 | #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
> | ^~~~~~~~~~~~~~~~
> include/linux/compiler.h:55:28: note: in expansion of macro '__trace_if_var'
> 55 | #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
> | ^~~~~~~~~~~~~~
> include/linux/overflow.h:334:9: note: in expansion of macro 'if'
> 334 | if (check_mul_overflow(factor1, factor2, &bytes))
> | ^~
> 'create_rule': event 2
>
> Out of these individually helpful checks (-Wrestrict, fortified
> string helpers, flex_array_size), one of them has to go to avoid
> the warning.
>
> Seeing that the length of the array is already checked earlier
> in this function, through both an explicit LANDLOCK_MAX_NUM_LAYERS
> comparison and the implicit kzalloc_flex() having succeeded,
> replace the flex_array_size() call with a direct multiplication.
Can flex_array_size() be fixed instead?
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> security/landlock/ruleset.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index 181df7736bb9..26e0b7193a7b 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -137,7 +137,7 @@ create_rule(const struct landlock_id id,
> new_rule->num_layers = new_num_layers;
> /* Copies the original layer stack. */
> memcpy(new_rule->layers, layers,
> - flex_array_size(new_rule, layers, num_layers));
> + sizeof(struct landlock_layer) * num_layers);
> if (new_layer)
> /* Adds a copy of @new_layer on the layer stack. */
> new_rule->layers[new_rule->num_layers - 1] = *new_layer;
> --
> 2.39.5
>
>
^ permalink raw reply
* [PATCH 3/3] apparmor: replace get_zeroed_page() with kzalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-20 8:18 UTC (permalink / raw)
To: James Morris, John Johansen, Ondrej Mosnacek, Paul Moore,
Serge E. Hallyn, Stephen Smalley
Cc: Mike Rapoport, apparmor, selinux, linux-kernel, linux-mm,
linux-security-module
In-Reply-To: <20260520-security-v1-0-831bd8e21dd0@kernel.org>
multi_transaction_new() allocates memory with get_zeroed_page() and uses
it as struct multi_transaction.
The usage of that structure does not require struct page access and it is
better to allocate multi_transaction objects with kzalloc() that provides
better scalability and more debugging possibilities.
Replace use of get_zeroed_page() with kzalloc().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
security/apparmor/apparmorfs.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index ededaf46f3ca..e5c99c71e7ca 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -9,6 +9,7 @@
*/
#include <linux/ctype.h>
+#include <linux/slab.h>
#include <linux/security.h>
#include <linux/vmalloc.h>
#include <linux/init.h>
@@ -904,7 +905,7 @@ static void multi_transaction_kref(struct kref *kref)
struct multi_transaction *t;
t = container_of(kref, struct multi_transaction, count);
- free_page((unsigned long) t);
+ kfree(t);
}
static struct multi_transaction *
@@ -947,7 +948,7 @@ static struct multi_transaction *multi_transaction_new(struct file *file,
if (size > MULTI_TRANSACTION_LIMIT - 1)
return ERR_PTR(-EFBIG);
- t = (struct multi_transaction *)get_zeroed_page(GFP_KERNEL);
+ t = kzalloc(PAGE_SIZE, GFP_KERNEL);
if (!t)
return ERR_PTR(-ENOMEM);
kref_init(&t->count);
--
2.53.0
^ permalink raw reply related
* [PATCH 2/3] selinux: hooks: use __getname() to allocate path buffer
From: Mike Rapoport (Microsoft) @ 2026-05-20 8:18 UTC (permalink / raw)
To: James Morris, John Johansen, Ondrej Mosnacek, Paul Moore,
Serge E. Hallyn, Stephen Smalley
Cc: Mike Rapoport, apparmor, selinux, linux-kernel, linux-mm,
linux-security-module
In-Reply-To: <20260520-security-v1-0-831bd8e21dd0@kernel.org>
selinux_genfs_get_sid() allocates memory for a path with __get_free_page()
although there is a dedicated helper for allocation of file paths:
__getname().
Replace __get_free_page() for allocation of a path buffer with __getname().
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
security/selinux/hooks.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 0f704380a8c8..05b84b3781e0 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1336,7 +1336,7 @@ static int selinux_genfs_get_sid(struct dentry *dentry,
struct super_block *sb = dentry->d_sb;
char *buffer, *path;
- buffer = (char *)__get_free_page(GFP_KERNEL);
+ buffer = __getname();
if (!buffer)
return -ENOMEM;
@@ -1361,7 +1361,7 @@ static int selinux_genfs_get_sid(struct dentry *dentry,
rc = 0;
}
}
- free_page((unsigned long)buffer);
+ __putname(buffer);
return rc;
}
--
2.53.0
^ permalink raw reply related
* [PATCH 1/3] selinux: use k[mz]alloc() to allocate temporary buffers
From: Mike Rapoport (Microsoft) @ 2026-05-20 8:18 UTC (permalink / raw)
To: James Morris, John Johansen, Ondrej Mosnacek, Paul Moore,
Serge E. Hallyn, Stephen Smalley
Cc: Mike Rapoport, apparmor, selinux, linux-kernel, linux-mm,
linux-security-module
In-Reply-To: <20260520-security-v1-0-831bd8e21dd0@kernel.org>
Several functions in selinuxfs.c allocate temporary buffers using
__get_free_page() or get_zeroed_page().
These buffers are used either to store a string generated by snprintf() (in
sel_make_bools()) or to copy data from user (sel_read_avc_hash_stats() and
sel_read_sidtab_hash_stats()).
Such usage does not require struct page access and it is better to allocate
these buffers with kzalloc()/kmalloc() that provide better scalability and
more debugging possibilities.
Replace use of get_zeroed_page() with kzalloc() and usage of
__get_free_page() with kmalloc().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
security/selinux/selinuxfs.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index 25ca7d714014..e7b884eedf80 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -1244,7 +1244,7 @@ static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_
char **names, *page;
u32 i, num;
- page = (char *)get_zeroed_page(GFP_KERNEL);
+ page = kzalloc(PAGE_SIZE, GFP_KERNEL);
if (!page)
return -ENOMEM;
@@ -1290,7 +1290,7 @@ static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_
ret = sel_attach_file(bool_dir, names[i], inode);
}
out:
- free_page((unsigned long)page);
+ kfree(page);
return ret;
}
@@ -1349,14 +1349,14 @@ static ssize_t sel_read_avc_hash_stats(struct file *filp, char __user *buf,
char *page;
ssize_t length;
- page = (char *)__get_free_page(GFP_KERNEL);
+ page = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!page)
return -ENOMEM;
length = avc_get_hash_stats(page);
if (length >= 0)
length = simple_read_from_buffer(buf, count, ppos, page, length);
- free_page((unsigned long)page);
+ kfree(page);
return length;
}
@@ -1367,7 +1367,7 @@ static ssize_t sel_read_sidtab_hash_stats(struct file *filp, char __user *buf,
char *page;
ssize_t length;
- page = (char *)__get_free_page(GFP_KERNEL);
+ page = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!page)
return -ENOMEM;
@@ -1375,7 +1375,7 @@ static ssize_t sel_read_sidtab_hash_stats(struct file *filp, char __user *buf,
if (length >= 0)
length = simple_read_from_buffer(buf, count, ppos, page,
length);
- free_page((unsigned long)page);
+ kfree(page);
return length;
}
--
2.53.0
^ permalink raw reply related
* [PATCH 0/3] security: replace __get_free_pages() call with kmalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-20 8:18 UTC (permalink / raw)
To: James Morris, John Johansen, Ondrej Mosnacek, Paul Moore,
Serge E. Hallyn, Stephen Smalley
Cc: Mike Rapoport, apparmor, selinux, linux-kernel, linux-mm,
linux-security-module
This is a (tiny) part of larger work of replacing page allocator calls
with kmalloc:
Also in git:
https://git.kernel.org/pub/scm/linux/kernel/git/rppt/linux.git gfp-to-kmalloc/security
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
Mike Rapoport (Microsoft) (3):
selinux: use k[mz]alloc() to allocate temporary buffers
selinux: hooks: use __getname() to allocate path buffer
apparmor: replace get_zeroed_page() with kzalloc()
security/apparmor/apparmorfs.c | 5 +++--
security/selinux/hooks.c | 4 ++--
security/selinux/selinuxfs.c | 12 ++++++------
3 files changed, 11 insertions(+), 10 deletions(-)
---
base-commit: 5d6919055dec134de3c40167a490f33c74c12581
change-id: 20260520-security-6cdd60da7129
Best regards,
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH v2] bpf: reject NULL data/sig in bpf_verify_pkcs7_signature
From: patchwork-bot+netdevbpf @ 2026-05-20 3:30 UTC (permalink / raw)
To: KP Singh; +Cc: bpf, linux-security-module, ast, daniel, dongxianrui1
In-Reply-To: <20260520024059.313468-1-kpsingh@kernel.org>
Hello:
This patch was applied to bpf/bpf.git (master)
by Kumar Kartikeya Dwivedi <memxor@gmail.com>:
On Wed, 20 May 2026 04:40:59 +0200 you wrote:
> __bpf_dynptr_data() can return NULL (FILE dynptrs, any non-contiguous
> backing). bpf_verify_pkcs7_signature() forwards the pointer to
> verify_pkcs7_signature() unchecked, causing a NULL deref in
> asn1_ber_decoder() reachable from a sleepable BPF LSM at lsm.s/bpf.
>
> NULL-check both pointers and reject with -EINVAL. Mirrors the guards
> already in kernel/bpf/crypto.c.
>
> [...]
Here is the summary with links:
- [v2] bpf: reject NULL data/sig in bpf_verify_pkcs7_signature
https://git.kernel.org/bpf/bpf/c/49b18315be4e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v2] bpf: reject NULL data/sig in bpf_verify_pkcs7_signature
From: Kumar Kartikeya Dwivedi @ 2026-05-20 3:23 UTC (permalink / raw)
To: KP Singh, bpf, linux-security-module; +Cc: ast, daniel, Xianrui Dong
In-Reply-To: <20260520024059.313468-1-kpsingh@kernel.org>
On Wed May 20, 2026 at 4:40 AM CEST, KP Singh wrote:
> __bpf_dynptr_data() can return NULL (FILE dynptrs, any non-contiguous
> backing). bpf_verify_pkcs7_signature() forwards the pointer to
> verify_pkcs7_signature() unchecked, causing a NULL deref in
> asn1_ber_decoder() reachable from a sleepable BPF LSM at lsm.s/bpf.
>
> NULL-check both pointers and reject with -EINVAL. Mirrors the guards
> already in kernel/bpf/crypto.c.
>
> Fixes: 865b0566d8f1 ("bpf: Add bpf_verify_pkcs7_signature() kfunc")
> Reported-by: Xianrui Dong <dongxianrui1@gmail.com>
> Signed-off-by: KP Singh <kpsingh@kernel.org>
> ---
Added missing acks before pushing.
> [...]
^ permalink raw reply
* [PATCH v2] bpf: reject NULL data/sig in bpf_verify_pkcs7_signature
From: KP Singh @ 2026-05-20 2:40 UTC (permalink / raw)
To: bpf, linux-security-module; +Cc: ast, daniel, Xianrui Dong, KP Singh
__bpf_dynptr_data() can return NULL (FILE dynptrs, any non-contiguous
backing). bpf_verify_pkcs7_signature() forwards the pointer to
verify_pkcs7_signature() unchecked, causing a NULL deref in
asn1_ber_decoder() reachable from a sleepable BPF LSM at lsm.s/bpf.
NULL-check both pointers and reject with -EINVAL. Mirrors the guards
already in kernel/bpf/crypto.c.
Fixes: 865b0566d8f1 ("bpf: Add bpf_verify_pkcs7_signature() kfunc")
Reported-by: Xianrui Dong <dongxianrui1@gmail.com>
Signed-off-by: KP Singh <kpsingh@kernel.org>
---
kernel/bpf/helpers.c | 5 +++++
tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 2bb60200c266..b5314c9fed3c 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -4241,8 +4241,13 @@ __bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p,
data_len = __bpf_dynptr_size(data_ptr);
data = __bpf_dynptr_data(data_ptr, data_len);
+ if (!data)
+ return -EINVAL;
+
sig_len = __bpf_dynptr_size(sig_ptr);
sig = __bpf_dynptr_data(sig_ptr, sig_len);
+ if (!sig)
+ return -EINVAL;
return verify_pkcs7_signature(data, data_len, sig, sig_len,
trusted_keyring->key,
diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c b/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c
index 8cd298b78e44..04aaf4c9cf5e 100644
--- a/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c
+++ b/tools/testing/selftests/bpf/prog_tests/kfunc_dynptr_param.c
@@ -14,7 +14,7 @@ static struct {
const char *prog_name;
int expected_runtime_err;
} kfunc_dynptr_tests[] = {
- {"dynptr_data_null", -EBADMSG},
+ {"dynptr_data_null", -EINVAL},
};
static bool kfunc_not_supported;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] bpf, libbpf: reject non-exclusive metadata maps in the signed loader
From: bot+bpf-ci @ 2026-05-19 22:50 UTC (permalink / raw)
To: kpsingh, bpf, linux-security-module
Cc: ast, daniel, kpsingh, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260519215358.126364-1-kpsingh@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2192 bytes --]
> Signed-off-by: KP Singh <kpsingh@kernel.org>
This looks like a bug fix for the signed loader. Should this include:
Fixes: 66a1025f7f0b ("Merge tag 'soc-newsoc-6.19' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc")
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
[ ... ]
> diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
> index cd5c2543f54d..becfda5b9025 100644
> --- a/tools/lib/bpf/gen_loader.c
> +++ b/tools/lib/bpf/gen_loader.c
> @@ -601,6 +601,23 @@ static void emit_signature_match(struct bpf_gen *gen)
> emit(gen, BPF_JMP_IMM(BPF_JA, 0, 0, -1));
> }
> }
> +
> + /* Reject if the metadata map is not exclusive. Without exclusivity
> + * the cached map->sha[] verified above can be stale: another BPF
> + * program with map access could have mutated the contents between
> + * BPF_OBJ_GET_INFO_BY_FD and loader execution.
> + */
This isn't a bug, but the BPF subsystem coding-style rule requires
multi-line comments to have the opening /* on its own line, with the
comment text starting on the next line. This applies to tools/lib/bpf/
even when surrounding code in the same file uses the old style:
/*
* Reject if the metadata map is not exclusive. Without exclusivity
* the cached map->sha[] verified above can be stale: another BPF
* program with map access could have mutated the contents between
* BPF_OBJ_GET_INFO_BY_FD and loader execution.
*/
> + emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_1, BPF_PSEUDO_MAP_IDX,
> + 0, 0, 0, 0));
> + emit(gen, BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_1, SHA256_DWORD_SIZE * sizeof(__u64)));
> + off = -(gen->insn_cur - gen->insn_start - gen->cleanup_label) / 8 - 1;
> + if (is_simm16(off)) {
> + emit(gen, BPF_MOV64_IMM(BPF_REG_7, -EINVAL));
> + emit(gen, BPF_JMP_IMM(BPF_JEQ, BPF_REG_2, 0, off));
> + } else {
> + gen->error = -ERANGE;
> + emit(gen, BPF_JMP_IMM(BPF_JA, 0, 0, -1));
> + }
> }
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26127913079
^ permalink raw reply
* Re: [PATCH] bpf: reject NULL data/sig in bpf_verify_pkcs7_signature
From: Daniel Borkmann @ 2026-05-19 22:44 UTC (permalink / raw)
To: KP Singh, bpf, linux-security-module; +Cc: ast, Xianrui Dong
In-Reply-To: <20260519200933.30752-1-kpsingh@kernel.org>
On 5/19/26 10:09 PM, KP Singh wrote:
> __bpf_dynptr_data() can return NULL (FILE dynptrs, any non-contiguous
> backing). bpf_verify_pkcs7_signature() forwards the pointer to
> verify_pkcs7_signature() unchecked, causing a NULL deref in
> asn1_ber_decoder() reachable from a sleepable BPF LSM at lsm.s/bpf.
>
> NULL-check both pointers and reject with -EINVAL. Mirrors the guards
> already in kernel/bpf/crypto.c.
>
> Fixes: 865b0566d8f1 ("bpf: Add bpf_verify_pkcs7_signature() kfunc")
> Reported-by: Xianrui Dong <dongxianrui1@gmail.com>
> Signed-off-by: KP Singh <kpsingh@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
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