* 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
* [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
* [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 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 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
* 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
* 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: [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: [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: [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: [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 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 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 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 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 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 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 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
* [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 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
* 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
* [PATCH 01/11] params: bound array element output to the caller's page buffer
From: Kees Cook @ 2026-05-21 13:33 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Kees Cook, Pengpeng Hou, stable, Petr Pavlu, Richard Weinberger,
Anton Ivanov, Johannes Berg, Rafael J. Wysocki, Len Brown,
Corey Minyard, Gabriel Somlo, Michael S. Tsirkin, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Simona Vetter, Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Jim Cromie, Tiwei Bie,
Benjamin Berg, Ilpo Järvinen, David E. Box,
Maciej W. Rozycki, Srinivas Pandruvada, Peter Zijlstra,
Heiko Carstens, Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
In-Reply-To: <20260521133315.work.845-kees@kernel.org>
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
param_array_get() appends each element's string representation into the
shared sysfs page buffer by passing buffer + off to the element getter.
That works for getters that only write a small bounded string, but
param_get_charp() and similar helpers format against PAGE_SIZE from the
pointer they receive. Once off is non-zero, an element getter can
therefore write past the end of the original sysfs page buffer.
Collect each element into a temporary PAGE_SIZE buffer first and then
copy only the remaining space into the caller's page buffer.
Cc: stable@vger.kernel.org
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Signed-off-by: Kees Cook <kees@kernel.org>
---
kernel/params.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/kernel/params.c b/kernel/params.c
index 74d620bc2521..752721922a15 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -475,22 +475,36 @@ static int param_array_set(const char *val, const struct kernel_param *kp)
static int param_array_get(char *buffer, const struct kernel_param *kp)
{
int i, off, ret;
+ char *elem_buf;
const struct kparam_array *arr = kp->arr;
struct kernel_param p = *kp;
+ elem_buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!elem_buf)
+ return -ENOMEM;
+
for (i = off = 0; i < (arr->num ? *arr->num : arr->max); i++) {
- /* Replace \n with comma */
- if (i)
- buffer[off - 1] = ',';
p.arg = arr->elem + arr->elemsize * i;
check_kparam_locked(p.mod);
- ret = arr->ops->get(buffer + off, &p);
+ ret = arr->ops->get(elem_buf, &p);
if (ret < 0)
- return ret;
+ goto out;
+ ret = min(ret, (int)(PAGE_SIZE - 1 - off));
+ if (!ret)
+ break;
+ /* Replace the previous element's trailing newline with a comma. */
+ if (i)
+ buffer[off - 1] = ',';
+ memcpy(buffer + off, elem_buf, ret);
off += ret;
+ if (off == PAGE_SIZE - 1)
+ break;
}
buffer[off] = '\0';
- return off;
+ ret = off;
+out:
+ kfree(elem_buf);
+ return ret;
}
static void param_array_free(void *arg)
--
2.34.1
^ permalink raw reply related
* [PATCH 00/11] Convert moduleparams to seq_buf
From: Kees Cook @ 2026-05-21 13:33 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Kees Cook, Pengpeng Hou, Petr Pavlu, Richard Weinberger,
Anton Ivanov, Johannes Berg, Rafael J. Wysocki, Len Brown,
Corey Minyard, Gabriel Somlo, Michael S. Tsirkin, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Simona Vetter, Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Jim Cromie, Tiwei Bie,
Benjamin Berg, Ilpo Järvinen, David E. Box,
Maciej W. Rozycki, Srinivas Pandruvada, Peter Zijlstra,
Heiko Carstens, Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
Hi,
I tried to trim the CC list here, but it's still pretty huge...
We've had a long-standing issue with "write to a string pointer" callbacks
that don't bounds check the destination (and for which the bounds is
also not part of the callback prototype, even if it is "known" to be
PAGE_SIZE, which sysfs_emit() depends on). Both moduleparams and sysfs
use this pattern. As a first step, and to test the migration method,
migrate moduleparams first.
There are 2 "mechanical" treewide patches that are handled by Coccinelle:
- treewide: Convert struct kernel_param_ops initializers to DEFINE_KERNEL_PARAM_OPS
- treewide: Convert custom kernel_param_ops .get callbacks to seq_buf via cocci
The last treewide patch is manual, and may need to be broken up into
per-subsystem patches, though I'd prefer to avoid this, as it would
extend the migration from 1 relase to at least 2 releases. (1 to
release the migration infrastructure, then 1 release to collect all the
subsystem changes, and possibly 1 more release to remove the migration
infrastructure.)
Thoughts, questions?
-Kees
Kees Cook (10):
panic: Replace panic_print_get() with generic helper
moduleparam: Add DEFINE_KERNEL_PARAM_OPS macro family
treewide: Convert struct kernel_param_ops initializers to
DEFINE_KERNEL_PARAM_OPS
moduleparam: Rename .get field to .get_str
moduleparam: Add seq_buf-based .get callback alongside .get_str
moduleparam: Route DEFINE_KERNEL_PARAM_OPS get pointer via _Generic
params: Convert generic kernel_param_ops .get helpers to seq_buf
treewide: Convert custom kernel_param_ops .get callbacks to seq_buf
via cocci
treewide: Manually convert custom kernel_param_ops .get callbacks
moduleparam: Drop legacy kernel_param_ops .get_str field and dispatch
logic
Pengpeng Hou (1):
params: bound array element output to the caller's page buffer
include/linux/dynamic_debug.h | 8 +-
include/linux/moduleparam.h | 65 +++++++---
security/apparmor/include/lib.h | 3 +-
mm/kfence/core.c | 15 ++-
arch/powerpc/kvm/book3s_hv.c | 5 +-
arch/s390/kernel/perf_cpum_sf.c | 12 +-
arch/um/drivers/vfio_kern.c | 9 +-
arch/um/drivers/virtio_uml.c | 18 +--
arch/x86/kernel/msr.c | 11 +-
arch/x86/kvm/mmu/mmu.c | 28 ++--
arch/x86/kvm/svm/avic.c | 14 +-
arch/x86/kvm/vmx/vmx.c | 24 ++--
arch/x86/platform/uv/uv_nmi.c | 24 ++--
block/disk-events.c | 6 +-
drivers/acpi/button.c | 19 ++-
drivers/acpi/ec.c | 14 +-
drivers/acpi/sysfs.c | 114 ++++++++--------
drivers/block/loop.c | 12 +-
drivers/block/null_blk/main.c | 12 +-
drivers/block/rnbd/rnbd-srv.c | 6 +-
drivers/block/ublk_drv.c | 12 +-
drivers/char/ipmi/ipmi_msghandler.c | 12 +-
drivers/char/ipmi/ipmi_watchdog.c | 50 +++----
drivers/crypto/hisilicon/hpre/hpre_main.c | 16 +--
drivers/crypto/hisilicon/sec2/sec_main.c | 23 +---
drivers/crypto/hisilicon/zip/zip_crypto.c | 5 +-
drivers/crypto/hisilicon/zip/zip_main.c | 21 +--
drivers/dma/dmatest.c | 34 ++---
drivers/edac/i10nm_base.c | 6 +-
drivers/firmware/efi/efi-pstore.c | 6 +-
drivers/firmware/qcom/qcom_scm.c | 18 +--
drivers/firmware/qemu_fw_cfg.c | 40 +++---
drivers/gpu/drm/drm_panic.c | 13 +-
drivers/gpu/drm/i915/i915_mitigations.c | 31 ++---
drivers/gpu/drm/imagination/pvr_fw_trace.c | 6 +-
drivers/hid/hid-cougar.c | 6 +-
drivers/hid/hid-steam.c | 6 +-
drivers/infiniband/hw/hfi1/driver.c | 12 +-
drivers/infiniband/ulp/iser/iscsi_iser.c | 6 +-
drivers/infiniband/ulp/isert/ib_isert.c | 6 +-
drivers/infiniband/ulp/srp/ib_srp.c | 12 +-
drivers/infiniband/ulp/srpt/ib_srpt.c | 5 +-
drivers/input/misc/ati_remote2.c | 23 ++--
drivers/input/mouse/psmouse-base.c | 15 ++-
drivers/md/md.c | 5 +-
drivers/media/pci/tw686x/tw686x-core.c | 6 +-
drivers/media/usb/uvc/uvc_driver.c | 14 +-
drivers/misc/lis3lv02d/lis3lv02d.c | 5 +-
drivers/net/wireless/ath/wil6210/main.c | 10 +-
drivers/nvme/host/multipath.c | 17 +--
drivers/nvme/host/pci.c | 18 +--
drivers/nvme/target/rdma.c | 5 +-
drivers/nvme/target/tcp.c | 5 +-
drivers/pci/pcie/aspm.c | 17 ++-
drivers/platform/x86/acerhdf.c | 5 +-
drivers/power/supply/bq27xxx_battery.c | 6 +-
drivers/power/supply/test_power.c | 122 +++++++++---------
drivers/scsi/fcoe/fcoe_transport.c | 22 ++--
drivers/scsi/sg.c | 6 +-
drivers/target/target_core_user.c | 25 ++--
.../processor_thermal_soc_slider.c | 24 ++--
drivers/thermal/intel/intel_powerclamp.c | 34 ++---
drivers/tty/hvc/hvc_iucv.c | 24 ++--
drivers/tty/sysrq.c | 6 +-
drivers/ufs/core/ufs-fault-injection.c | 12 +-
drivers/ufs/core/ufs-mcq.c | 18 +--
drivers/ufs/core/ufs-txeq.c | 5 +-
drivers/ufs/core/ufshcd.c | 12 +-
drivers/usb/core/quirks.c | 6 +-
drivers/usb/gadget/legacy/serial.c | 5 +-
drivers/usb/storage/usb.c | 25 ++--
drivers/vhost/scsi.c | 12 +-
drivers/virt/nitro_enclaves/ne_misc_dev.c | 6 +-
drivers/virtio/virtio_mmio.c | 27 ++--
fs/ceph/super.c | 10 +-
fs/fuse/dir.c | 5 +-
fs/nfs/namespace.c | 12 +-
fs/nfs/super.c | 6 +-
fs/ocfs2/dlmfs/dlmfs.c | 5 +-
fs/overlayfs/copy_up.c | 5 +-
fs/ubifs/super.c | 6 +-
kernel/locking/locktorture.c | 12 +-
kernel/panic.c | 11 +-
kernel/params.c | 122 +++++++++---------
kernel/power/hibernate.c | 6 +-
kernel/rcu/tree.c | 24 ++--
kernel/sched/ext.c | 11 +-
kernel/workqueue.c | 18 ++-
lib/dynamic_debug.c | 16 ++-
lib/test_dynamic_debug.c | 12 +-
mm/damon/lru_sort.c | 33 +++--
mm/damon/reclaim.c | 33 +++--
mm/damon/stat.c | 16 +--
mm/memory_hotplug.c | 30 +++--
mm/page_reporting.c | 11 +-
mm/shuffle.c | 6 +-
mm/zswap.c | 14 +-
net/batman-adv/bat_algo.c | 6 +-
net/ceph/ceph_common.c | 10 +-
net/ipv4/tcp_dctcp.c | 6 +-
net/sunrpc/auth.c | 12 +-
net/sunrpc/svc.c | 5 +-
net/sunrpc/xprtsock.c | 18 +--
samples/damon/mtier.c | 6 +-
samples/damon/prcl.c | 6 +-
samples/damon/wsse.c | 6 +-
security/apparmor/lib.c | 27 ++--
security/apparmor/lsm.c | 75 +++++------
sound/hda/controllers/intel.c | 5 +-
sound/usb/card.c | 7 +-
110 files changed, 854 insertions(+), 1066 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH 02/11] panic: Replace panic_print_get() with generic helper
From: Kees Cook @ 2026-05-21 13:33 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Kees Cook, Pengpeng Hou, Petr Pavlu, Richard Weinberger,
Anton Ivanov, Johannes Berg, Rafael J. Wysocki, Len Brown,
Corey Minyard, Gabriel Somlo, Michael S. Tsirkin, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Simona Vetter, Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Jim Cromie, Tiwei Bie,
Benjamin Berg, Ilpo Järvinen, David E. Box,
Maciej W. Rozycki, Srinivas Pandruvada, Peter Zijlstra,
Heiko Carstens, Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
In-Reply-To: <20260521133315.work.845-kees@kernel.org>
Since panic_print_get() just calls the ulong helper directly, there is
no need for it to exist as a wrapper.
Signed-off-by: Kees Cook <kees@kernel.org>
---
kernel/panic.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/kernel/panic.c b/kernel/panic.c
index 20feada5319d..42e5ebde4585 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -1214,14 +1214,9 @@ static int panic_print_set(const char *val, const struct kernel_param *kp)
return param_set_ulong(val, kp);
}
-static int panic_print_get(char *val, const struct kernel_param *kp)
-{
- return param_get_ulong(val, kp);
-}
-
static const struct kernel_param_ops panic_print_ops = {
.set = panic_print_set,
- .get = panic_print_get,
+ .get = param_get_ulong,
};
__core_param_cb(panic_print, &panic_print_ops, &panic_print, 0644);
--
2.34.1
^ permalink raw reply related
* [PATCH 03/11] moduleparam: Add DEFINE_KERNEL_PARAM_OPS macro family
From: Kees Cook @ 2026-05-21 13:33 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Kees Cook, Pengpeng Hou, Petr Pavlu, Richard Weinberger,
Anton Ivanov, Johannes Berg, Rafael J. Wysocki, Len Brown,
Corey Minyard, Gabriel Somlo, Michael S. Tsirkin, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, David Airlie,
Simona Vetter, Bart Van Assche, Jason Gunthorpe, Leon Romanovsky,
Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Bjorn Helgaas, Hannes Reinecke, James E.J. Bottomley,
Martin K. Petersen, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Greg Kroah-Hartman, Jiri Slaby, Alan Stern, Jason Wang, Xuan Zhuo,
Eugenio Pérez, Jason Baron, Jim Cromie, Tiwei Bie,
Benjamin Berg, Ilpo Järvinen, David E. Box,
Maciej W. Rozycki, Srinivas Pandruvada, Peter Zijlstra,
Heiko Carstens, Vasily Gorbik, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Vinod Koul, Frank Li, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Alexander Potapenko, Marco Elver, Dmitry Vyukov,
Andrew Morton, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andy Shevchenko, Georgia Garcia, kvm, dmaengine,
linux-modules, kasan-dev, linux-mm, apparmor,
linux-security-module, linux-um, linux-acpi, openipmi-developer,
qemu-devel, intel-gfx, dri-devel, linux-rdma, linux-media,
linux-pci, linux-scsi, linux-pm, linuxppc-dev, linux-serial,
linux-usb, usb-storage, virtualization, linux-kernel, linux-arch,
netdev, linux-fsdevel, linux-hardening
In-Reply-To: <20260521133315.work.845-kees@kernel.org>
Add macros that define a struct kernel_param_ops initializer through a
macro so the underlying field layout can evolve without touching every
call site. Three variants cover the three cases:
DEFINE_KERNEL_PARAM_OPS(name, set, get) // basic
DEFINE_KERNEL_PARAM_OPS_NOARG(name, set, get) // set KERNEL_PARAM_OPS_FL_NOARG
DEFINE_KERNEL_PARAM_OPS_FREE(name, set, get, free) // also set .free
Callers prefix their own visibility qualifiers, e.g.:
static DEFINE_KERNEL_PARAM_OPS(my_ops, my_set, my_get);
Also update module_param_call() and STANDARD_PARAM_DEF() to use
DEFINE_KERNEL_PARAM_OPS internally so the generated ops table will go
through the same macro as everything else.
Subsequent commits convert all open-coded struct kernel_param_ops
definitions to use these macros, in preparation for migrating to a
seq_buf .get API.
Signed-off-by: Kees Cook <kees@kernel.org>
---
include/linux/moduleparam.h | 36 ++++++++++++++++++++++++++++++++++--
kernel/params.c | 6 ++----
2 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 075f28585074..26bf45b36d02 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -68,6 +68,39 @@ struct kernel_param_ops {
void (*free)(void *arg);
};
+/*
+ * Define a const struct kernel_param_ops initializer. Callers prefix with
+ * any required visibility qualifiers (typically "static"):
+ *
+ * static DEFINE_KERNEL_PARAM_OPS(my_ops, my_set, my_get);
+ *
+ * Routing the @_set and @_get function pointers through the macro
+ * (rather than naming the struct fields at every call site) lets the
+ * field layout change in one place when callbacks are migrated to a
+ * new signature.
+ */
+#define DEFINE_KERNEL_PARAM_OPS(_name, _set, _get) \
+ const struct kernel_param_ops _name = { \
+ .set = (_set), \
+ .get = (_get), \
+ }
+
+/* As DEFINE_KERNEL_PARAM_OPS, with KERNEL_PARAM_OPS_FL_NOARG set. */
+#define DEFINE_KERNEL_PARAM_OPS_NOARG(_name, _set, _get) \
+ const struct kernel_param_ops _name = { \
+ .flags = KERNEL_PARAM_OPS_FL_NOARG, \
+ .set = (_set), \
+ .get = (_get), \
+ }
+
+/* As DEFINE_KERNEL_PARAM_OPS, with an additional .free callback. */
+#define DEFINE_KERNEL_PARAM_OPS_FREE(_name, _set, _get, _free) \
+ const struct kernel_param_ops _name = { \
+ .set = (_set), \
+ .get = (_get), \
+ .free = (_free), \
+ }
+
/*
* Flags available for kernel_param
*
@@ -311,8 +344,7 @@ struct kparam_array
* kernel_param_ops), use module_param_cb() instead.
*/
#define module_param_call(name, _set, _get, arg, perm) \
- static const struct kernel_param_ops __param_ops_##name = \
- { .flags = 0, .set = _set, .get = _get }; \
+ static DEFINE_KERNEL_PARAM_OPS(__param_ops_##name, _set, _get); \
__module_param_call(MODULE_PARAM_PREFIX, \
name, &__param_ops_##name, arg, perm, -1, 0)
diff --git a/kernel/params.c b/kernel/params.c
index 752721922a15..2cbad1f4dd06 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -222,10 +222,8 @@ char *parse_args(const char *doing,
return scnprintf(buffer, PAGE_SIZE, format "\n", \
*((type *)kp->arg)); \
} \
- const struct kernel_param_ops param_ops_##name = { \
- .set = param_set_##name, \
- .get = param_get_##name, \
- }; \
+ DEFINE_KERNEL_PARAM_OPS(param_ops_##name, \
+ param_set_##name, param_get_##name); \
EXPORT_SYMBOL(param_set_##name); \
EXPORT_SYMBOL(param_get_##name); \
EXPORT_SYMBOL(param_ops_##name)
--
2.34.1
^ 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