* Re: [PATCH v2] memfd,selinux: call security_inode_init_security_anon
From: Stephen Smalley @ 2025-09-08 16:27 UTC (permalink / raw)
To: Thiébaud Weksteen
Cc: Paul Moore, James Morris, Hugh Dickins, Jeff Vander Stoep,
Nick Kralevich, Jeff Xu, Baolin Wang, Isaac Manjarres,
linux-kernel, linux-security-module, selinux, linux-mm
In-Reply-To: <20250908013419.4186627-1-tweek@google.com>
On Sun, Sep 7, 2025 at 9:34 PM Thiébaud Weksteen <tweek@google.com> wrote:
>
> Prior to this change, no security hooks were called at the creation of a
> memfd file. It means that, for SELinux as an example, it will receive
> the default type of the filesystem that backs the in-memory inode. In
> most cases, that would be tmpfs, but if MFD_HUGETLB is passed, it will
> be hugetlbfs. Both can be considered implementation details of memfd.
>
> It also means that it is not possible to differentiate between a file
> coming from memfd_create and a file coming from a standard tmpfs mount
> point.
>
> Additionally, no permission is validated at creation, which differs from
> the similar memfd_secret syscall.
>
> Call security_inode_init_security_anon during creation. This ensures
> that the file is setup similarly to other anonymous inodes. On SELinux,
> it means that the file will receive the security context of its task.
>
> The ability to limit fexecve on memfd has been of interest to avoid
> potential pitfalls where /proc/self/exe or similar would be executed
> [1][2]. Reuse the "execute_no_trans" and "entrypoint" access vectors,
> similarly to the file class. These access vectors may not make sense for
> the existing "anon_inode" class. Therefore, define and assign a new
> class "memfd_file" to support such access vectors.
>
> Guard these changes behind a new policy capability named "memfd_class".
>
> [1] https://crbug.com/1305267
> [2] https://lore.kernel.org/lkml/20221215001205.51969-1-jeffxu@google.com/
>
> Signed-off-by: Thiébaud Weksteen <tweek@google.com>
> Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
When you revise a patch, you aren't supposed to retain other's tags
since they haven't technically reviewed, agreed to, or tested the
revised change.
That said, I have now done so and thus these tags can remain!
> ---
> Changes since v1:
> - Move test of class earlier in selinux_bprm_creds_for_exec
> - Remove duplicate call to security_transition_sid
>
> Changes since RFC:
> - Remove enum argument, simply compare the anon inode name
> - Introduce a policy capability for compatility
> - Add validation of class in selinux_bprm_creds_for_exec
>
> include/linux/memfd.h | 2 ++
> mm/memfd.c | 14 ++++++++++--
> security/selinux/hooks.c | 26 +++++++++++++++++-----
> security/selinux/include/classmap.h | 2 ++
> security/selinux/include/policycap.h | 1 +
> security/selinux/include/policycap_names.h | 1 +
> security/selinux/include/security.h | 5 +++++
> 7 files changed, 44 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/memfd.h b/include/linux/memfd.h
> index 6f606d9573c3..cc74de3dbcfe 100644
> --- a/include/linux/memfd.h
> +++ b/include/linux/memfd.h
> @@ -4,6 +4,8 @@
>
> #include <linux/file.h>
>
> +#define MEMFD_ANON_NAME "[memfd]"
> +
> #ifdef CONFIG_MEMFD_CREATE
> extern long memfd_fcntl(struct file *file, unsigned int cmd, unsigned int arg);
> struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx);
> diff --git a/mm/memfd.c b/mm/memfd.c
> index bbe679895ef6..63b439eb402a 100644
> --- a/mm/memfd.c
> +++ b/mm/memfd.c
> @@ -433,6 +433,8 @@ static struct file *alloc_file(const char *name, unsigned int flags)
> {
> unsigned int *file_seals;
> struct file *file;
> + struct inode *inode;
> + int err = 0;
>
> if (flags & MFD_HUGETLB) {
> file = hugetlb_file_setup(name, 0, VM_NORESERVE,
> @@ -444,12 +446,20 @@ static struct file *alloc_file(const char *name, unsigned int flags)
> }
> if (IS_ERR(file))
> return file;
> +
> + inode = file_inode(file);
> + err = security_inode_init_security_anon(inode,
> + &QSTR(MEMFD_ANON_NAME), NULL);
> + if (err) {
> + fput(file);
> + file = ERR_PTR(err);
> + return file;
> + }
> +
> file->f_mode |= FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE;
> file->f_flags |= O_LARGEFILE;
>
> if (flags & MFD_NOEXEC_SEAL) {
> - struct inode *inode = file_inode(file);
> -
> inode->i_mode &= ~0111;
> file_seals = memfd_file_seals_ptr(file);
> if (file_seals) {
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c95a5874bf7d..6adf2f393ed9 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -93,6 +93,7 @@
> #include <linux/fanotify.h>
> #include <linux/io_uring/cmd.h>
> #include <uapi/linux/lsm.h>
> +#include <linux/memfd.h>
>
> #include "avc.h"
> #include "objsec.h"
> @@ -2315,6 +2316,9 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> new_tsec = selinux_cred(bprm->cred);
> isec = inode_security(inode);
>
> + if (isec->sclass != SECCLASS_FILE && isec->sclass != SECCLASS_MEMFD_FILE)
> + return -EPERM;
> +
> /* Default to the current task SID. */
> new_tsec->sid = old_tsec->sid;
> new_tsec->osid = old_tsec->sid;
> @@ -2366,9 +2370,10 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> ad.type = LSM_AUDIT_DATA_FILE;
> ad.u.file = bprm->file;
>
> +
> if (new_tsec->sid == old_tsec->sid) {
> - rc = avc_has_perm(old_tsec->sid, isec->sid,
> - SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad);
> + rc = avc_has_perm(old_tsec->sid, isec->sid, isec->sclass,
> + FILE__EXECUTE_NO_TRANS, &ad);
> if (rc)
> return rc;
> } else {
> @@ -2378,8 +2383,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
> if (rc)
> return rc;
>
> - rc = avc_has_perm(new_tsec->sid, isec->sid,
> - SECCLASS_FILE, FILE__ENTRYPOINT, &ad);
> + rc = avc_has_perm(new_tsec->sid, isec->sid, isec->sclass,
> + FILE__ENTRYPOINT, &ad);
> if (rc)
> return rc;
>
> @@ -2974,10 +2979,18 @@ static int selinux_inode_init_security_anon(struct inode *inode,
> struct common_audit_data ad;
> struct inode_security_struct *isec;
> int rc;
> + bool is_memfd = false;
>
> if (unlikely(!selinux_initialized()))
> return 0;
>
> + if (name != NULL && name->name != NULL &&
> + !strcmp(name->name, MEMFD_ANON_NAME)) {
> + if (!selinux_policycap_memfd_class())
> + return 0;
> + is_memfd = true;
> + }
> +
> isec = selinux_inode(inode);
>
> /*
> @@ -2997,7 +3010,10 @@ static int selinux_inode_init_security_anon(struct inode *inode,
> isec->sclass = context_isec->sclass;
> isec->sid = context_isec->sid;
> } else {
> - isec->sclass = SECCLASS_ANON_INODE;
> + if (is_memfd)
> + isec->sclass = SECCLASS_MEMFD_FILE;
> + else
> + isec->sclass = SECCLASS_ANON_INODE;
> rc = security_transition_sid(
> sid, sid,
> isec->sclass, name, &isec->sid);
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 5665aa5e7853..3ec85142771f 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -179,6 +179,8 @@ const struct security_class_mapping secclass_map[] = {
> { "anon_inode", { COMMON_FILE_PERMS, NULL } },
> { "io_uring", { "override_creds", "sqpoll", "cmd", "allowed", NULL } },
> { "user_namespace", { "create", NULL } },
> + { "memfd_file",
> + { COMMON_FILE_PERMS, "execute_no_trans", "entrypoint", NULL } },
> /* last one */ { NULL, {} }
> };
>
> diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h
> index 7405154e6c42..dabcc9f14dde 100644
> --- a/security/selinux/include/policycap.h
> +++ b/security/selinux/include/policycap.h
> @@ -17,6 +17,7 @@ enum {
> POLICYDB_CAP_NETLINK_XPERM,
> POLICYDB_CAP_NETIF_WILDCARD,
> POLICYDB_CAP_GENFS_SECLABEL_WILDCARD,
> + POLICYDB_CAP_MEMFD_CLASS,
> __POLICYDB_CAP_MAX
> };
> #define POLICYDB_CAP_MAX (__POLICYDB_CAP_MAX - 1)
> diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h
> index d8962fcf2ff9..8e96f2a816b6 100644
> --- a/security/selinux/include/policycap_names.h
> +++ b/security/selinux/include/policycap_names.h
> @@ -20,6 +20,7 @@ const char *const selinux_policycap_names[__POLICYDB_CAP_MAX] = {
> "netlink_xperm",
> "netif_wildcard",
> "genfs_seclabel_wildcard",
> + "memfd_class",
> };
> /* clang-format on */
>
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 8201e6a3ac0f..72c963f54148 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -209,6 +209,11 @@ static inline bool selinux_policycap_netif_wildcard(void)
> selinux_state.policycap[POLICYDB_CAP_NETIF_WILDCARD]);
> }
>
> +static inline bool selinux_policycap_memfd_class(void)
> +{
> + return READ_ONCE(selinux_state.policycap[POLICYDB_CAP_MEMFD_CLASS]);
> +}
> +
> struct selinux_policy_convert_data;
>
> struct selinux_load_state {
> --
> 2.51.0.384.g4c02a37b29-goog
>
^ permalink raw reply
* Re: [PATCH] ima: don't clear IMA_DIGSIG flag when setting non-IMA xattr
From: Mimi Zohar @ 2025-09-08 14:53 UTC (permalink / raw)
To: Coiby Xu
Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <53wb5tzech2k4k25xy2heq7ohmp2elw2a7l4x3nfk6fajfydur@5thsinydau5x>
Hi Coiby,
On Mon, 2025-09-08 at 19:12 +0800, Coiby Xu wrote:
> >
> > Even without an IMA appraise policy, the security xattrs are written out to the
> > filesystem, but the IMA_DIGSIG flag is not cached.
>
> It seems I miss some context for the above sentence. If no IMA policy is
> configured, no ima_iint_cache will be created. If you mean non-appraisal
> policy, will not caching IMA_DIGSIG flag cause any problem?
Sorry. What I was trying to say is that your test program illustrates the
problem both with or without any of the boot command line options as you
suggested - "ima_appraise=fix evm=fix ima_policy=appraise_tcb". Writing some
other security xattr is a generic problem, whether the file is in policy or not,
whether IMA or EVM are in fix mode or not. The rpm-plugin-ima should install
the IMA signature regardless.
SELinux doesn't usually re-write the security.selinux xattr, so the problem is
hard to reproduce after installing the rpm-plugin-ima with "dnf reinstall
<package>".
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v3 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Tetsuo Handa @ 2025-09-08 13:05 UTC (permalink / raw)
To: Paul Moore, John Johansen
Cc: Roberto Sassu, linux-security-module, linux-integrity, selinux,
Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <6e4bb79d-ba8f-47fa-ad12-0bb79d4442e0@I-love.SAKURA.ne.jp>
On 2025/09/07 16:35, Tetsuo Handa wrote:
> On 2025/09/05 2:52, Paul Moore wrote:
>> + if (!str) {
>> + str = str_tmp;
>> + len = len_tmp - 1;
>
> This needs to be
>
> len = len_tmp - 1;
> mb();
> str = str_tmp;
>
> , or concurrent access might reach simple_read_from_buffer()
> with str != 0 and len == 0. (If you don't want mb(), you can use
>
> - if (unlikely(!str)) {
> + if (unlikely(!str || !len)) {
>
> instead).
Well, memory barrier is more complicated; it will be
len = len_tmp - 1;
wmb();
str = str_tmp;
and
}
rmb();
return simple_read_from_buffer(buf, count, ppos, str, len);
pair.
Just splitting the whole { } block that follows "if (unlikely(!str))"
out as an initcall function is much simpler; no need to use spinlock
(because the userspace threads has not started yet), no need to worry
about kmalloc() failure (because the allocation failure will panic()
because the userspace threads has not started yet), and the memory size
saved by use of __init function will be larger than the memory size
wasted by /sys/kernel/security/lsm being never accessed...
^ permalink raw reply
* Re: [PATCH] ima: don't clear IMA_DIGSIG flag when setting non-IMA xattr
From: Coiby Xu @ 2025-09-08 11:12 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <d252b04934908e7e65a3299bfeffc282c7b0b12f.camel@linux.ibm.com>
On Thu, Sep 04, 2025 at 10:41:42PM -0400, Mimi Zohar wrote:
>On Tue, 2025-09-02 at 12:25 +0800, Coiby Xu wrote:
>> Currently when both IMA and EVM are in fix mode, the IMA signature will
>> be reset to IMA hash if a program first stores IMA signature in
>> security.ima and then sets security.selinux for a file. For example, on
>> Fedora, after booting the kernel with "ima_appraise=fix evm=fix
>> ima_policy=appraise_tcb" and installing rpm-plugin-ima, reinstalling a
>> package will not make good reference IMA signature generated. Instead
>> IMA hash is generated,
>> # getfattr -m - -d -e hex /usr/bin/bash
>> # file: usr/bin/bash
>> security.ima=0x0404...
>>
>> This happens because when setting selinux.selinux, the IMA_DIGSIG flag
>> that had been set early was cleared. As a result, IMA hash is generated
>> when the file is closed.
>>
>> Here's a minimal C reproducer,
>>
>> #include <stdio.h>
>> #include <sys/xattr.h>
>> #include <fcntl.h>
>> #include <unistd.h>
>> #include <string.h>
>> #include <stdlib.h>
>>
>> int main() {
>> const char* file_path = "/usr/sbin/test_binary";
>> const char* hex_string = "030204d33204490066306402304";
>> int length = strlen(hex_string);
>> char* ima_attr_value;
>> int fd;
>>
>> fd = open(file_path, O_WRONLY|O_CREAT|O_EXCL, 0644);
>> if (fd == -1) {
>> perror("Error opening file");
>> return 1;
>> }
>>
>> ima_attr_value = (char*)malloc(length / 2 );
>> for (int i = 0, j = 0; i < length; i += 2, j++) {
>> sscanf(hex_string + i, "%2hhx", &ima_attr_value[j]);
>> }
>>
>> if (fsetxattr(fd, "security.ima", ima_attr_value, length/2, 0) == -1) {
>> perror("Error setting extended attribute");
>> close(fd);
>> return 1;
>> }
>>
>> const char* selinux_value= "system_u:object_r:bin_t:s0";
>> if (fsetxattr(fd, "security.selinux", selinux_value, strlen(selinux_value), 0) == -1) {
>> perror("Error setting extended attribute");
>> close(fd);
>> return 1;
>> }
>>
>> close(fd);
>>
>> return 0;
>> }
>>
>> Signed-off-by: Coiby Xu <coxu@redhat.com>
>
>Thanks, Coiby. Agreed, the ability to clear the IMA_DIGSIG flag should be
>limited to security.ima xattr and probably security.evm xattr. Writing other
>security xattrs should not affect the IMA_DIGSIG flag.
Thanks for confirming it!
>
>Even without an IMA appraise policy, the security xattrs are written out to the
>filesystem, but the IMA_DIGSIG flag is not cached.
It seems I miss some context for the above sentence. If no IMA policy is
configured, no ima_iint_cache will be created. If you mean non-appraisal
policy, will not caching IMA_DIGSIG flag cause any problem?
>
>Please document the tristate values:
>0: clear IMA_DIGSIG
>1: set IMA_DIGSIG
>-1: don't change IMA_DIGSIG
Addressed in v2. Thanks for the suggestion!
>
>> ---
>> security/integrity/ima/ima_appraise.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
>> index f435eff4667f..fc82161f8b30 100644
>> --- a/security/integrity/ima/ima_appraise.c
>> +++ b/security/integrity/ima/ima_appraise.c
>> @@ -708,7 +708,7 @@ static void ima_reset_appraise_flags(struct inode *inode, int digsig)
>> set_bit(IMA_CHANGE_XATTR, &iint->atomic_flags);
>> if (digsig)
>> set_bit(IMA_DIGSIG, &iint->atomic_flags);
>
>This matches both -1 and 1. Test "digsig == 1" here.
>
>> - else
>> + else if (digsig != -1)
>
>and test "digsig == 0" here.
>
>> clear_bit(IMA_DIGSIG, &iint->atomic_flags);
>> }
>>
>> @@ -794,6 +794,8 @@ static int ima_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
>> digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
>> } else if (!strcmp(xattr_name, XATTR_NAME_EVM) && xattr_value_len > 0) {
>> digsig = (xvalue->type == EVM_XATTR_PORTABLE_DIGSIG);
>> + } else if (result != 1) {
>
>The "if (result != 1)" test is redundant.
I've fixed them in v2. Thanks for reviewing the patch and correcting my
careless mistakes! I'll check if my mind is in a clear thinking state
next time.
>
>thanks,
>
>Mimi
>
>> + digsig = -1;
>> }
>> if (result == 1 || evm_revalidate_status(xattr_name)) {
>> ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
>
--
Best regards,
Coiby
^ permalink raw reply
* [PATCH v2] ima: don't clear IMA_DIGSIG flag when setting non-IMA xattr
From: Coiby Xu @ 2025-09-08 10:58 UTC (permalink / raw)
To: linux-integrity, Mimi Zohar
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM,
open list
In-Reply-To: <20250902042515.759750-1-coxu@redhat.com>
Currently when both IMA and EVM are in fix mode, the IMA signature will
be reset to IMA hash if a program first stores IMA signature in
security.ima and then sets security.selinux for a file. For example, on
Fedora, after booting the kernel with "ima_appraise=fix evm=fix
ima_policy=appraise_tcb" and installing rpm-plugin-ima, reinstalling a
package will not make good reference IMA signature generated. Instead
IMA hash is generated,
# getfattr -m - -d -e hex /usr/bin/bash
# file: usr/bin/bash
security.ima=0x0404...
This happens because when setting selinux.selinux, the IMA_DIGSIG flag
that had been set early was cleared. As a result, IMA hash is generated
when the file is closed.
Here's a minimal C reproducer,
#include <stdio.h>
#include <sys/xattr.h>
#include <fcntl.h>
#include <unistd.h>
#include <string.h>
#include <stdlib.h>
int main() {
const char* file_path = "/usr/sbin/test_binary";
const char* hex_string = "030204d33204490066306402304";
int length = strlen(hex_string);
char* ima_attr_value;
int fd;
fd = open(file_path, O_WRONLY|O_CREAT|O_EXCL, 0644);
if (fd == -1) {
perror("Error opening file");
return 1;
}
ima_attr_value = (char*)malloc(length / 2 );
for (int i = 0, j = 0; i < length; i += 2, j++) {
sscanf(hex_string + i, "%2hhx", &ima_attr_value[j]);
}
if (fsetxattr(fd, "security.ima", ima_attr_value, length/2, 0) == -1) {
perror("Error setting extended attribute");
close(fd);
return 1;
}
const char* selinux_value= "system_u:object_r:bin_t:s0";
if (fsetxattr(fd, "security.selinux", selinux_value, strlen(selinux_value), 0) == -1) {
perror("Error setting extended attribute");
close(fd);
return 1;
}
close(fd);
return 0;
}
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
security/integrity/ima/ima_appraise.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index f435eff4667f..4e4750ea41ad 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -694,6 +694,15 @@ static int ima_protect_xattr(struct dentry *dentry, const char *xattr_name,
return 0;
}
+/*
+ * ima_reset_appraise_flags - reset ima_iint_cache flags
+ *
+ * @digsig: whether to clear/set IMA_DIGSIG flag, tristate values
+ * 0: clear IMA_DIGSIG
+ * 1: set IMA_DIGSIG
+ * -1: don't change IMA_DIGSIG
+ *
+ */
static void ima_reset_appraise_flags(struct inode *inode, int digsig)
{
struct ima_iint_cache *iint;
@@ -706,9 +715,9 @@ static void ima_reset_appraise_flags(struct inode *inode, int digsig)
return;
iint->measured_pcrs = 0;
set_bit(IMA_CHANGE_XATTR, &iint->atomic_flags);
- if (digsig)
+ if (digsig == 1)
set_bit(IMA_DIGSIG, &iint->atomic_flags);
- else
+ else if (digsig == 0)
clear_bit(IMA_DIGSIG, &iint->atomic_flags);
}
@@ -794,6 +803,8 @@ static int ima_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
} else if (!strcmp(xattr_name, XATTR_NAME_EVM) && xattr_value_len > 0) {
digsig = (xvalue->type == EVM_XATTR_PORTABLE_DIGSIG);
+ } else {
+ digsig = -1;
}
if (result == 1 || evm_revalidate_status(xattr_name)) {
ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH net-next v2 2/8] ipv4: icmp: Pass IPv4 control block structure as an argument to __icmp_send()
From: Eric Dumazet @ 2025-09-08 7:55 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, kuba, pabeni, horms, paul, dsahern, petrm,
linux-security-module
In-Reply-To: <20250908073238.119240-3-idosch@nvidia.com>
On Mon, Sep 8, 2025 at 12:35 AM Ido Schimmel <idosch@nvidia.com> wrote:
>
> __icmp_send() is used to generate ICMP error messages in response to
> various situations such as MTU errors (i.e., "Fragmentation Required")
> and too many hops (i.e., "Time Exceeded").
>
> The skb that generated the error does not necessarily come from the IPv4
> layer and does not always have a valid IPv4 control block in skb->cb.
>
> Therefore, commit 9ef6b42ad6fd ("net: Add __icmp_send helper.") changed
> the function to take the IP options structure as argument instead of
> deriving it from the skb's control block. Some callers of this function
> such as icmp_send() pass the IP options structure from the skb's control
> block as in these call paths the control block is known to be valid, but
> other callers simply pass a zeroed structure.
>
> A subsequent patch will need __icmp_send() to access more information
> from the IPv4 control block (specifically, the ifindex of the input
> interface). As a preparation for this change, change the function to
> take the IPv4 control block structure as an argument instead of the IP
> options structure. This makes the function similar to its IPv6
> counterpart that already takes the IPv6 control block structure as an
> argument.
>
> No functional changes intended.
>
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net-next v2 1/8] ipv4: cipso: Simplify IP options handling in cipso_v4_error()
From: Eric Dumazet @ 2025-09-08 7:51 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, kuba, pabeni, horms, paul, dsahern, petrm,
linux-security-module
In-Reply-To: <20250908073238.119240-2-idosch@nvidia.com>
On Mon, Sep 8, 2025 at 12:35 AM Ido Schimmel <idosch@nvidia.com> wrote:
>
> When __ip_options_compile() is called with an skb, the IP options are
> parsed from the skb data into the provided IP option argument. This is
> in contrast to the case where the skb argument is NULL and the options
> are parsed from opt->__data.
>
> Given that cipso_v4_error() always passes an skb to
> __ip_options_compile(), there is no need to allocate an extra 40 bytes
> (maximum IP options size).
>
> Therefore, simplify the function by removing these extra bytes and make
> the function similar to ipv4_send_dest_unreach() which also calls both
> __ip_options_compile() and __icmp_send().
>
> This is a preparation for changing the arguments being passed to
> __icmp_send().
>
> No functional changes intended.
>
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> Reviewed-by: David Ahern <dsahern@kernel.org>
> Acked-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH net-next v2 8/8] selftests: traceroute: Add VRF tests
From: Ido Schimmel @ 2025-09-08 7:32 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, horms, paul, dsahern, petrm,
linux-security-module, Ido Schimmel
In-Reply-To: <20250908073238.119240-1-idosch@nvidia.com>
Create versions of the existing test cases where the routers generating
the ICMP error messages are using VRFs. Check that the source IPs of
these messages do not change in the presence of VRFs.
IPv6 always behaved correctly, but IPv4 fails when reverting "ipv4:
icmp: Fix source IP derivation in presence of VRFs".
Without IPv4 change:
# ./traceroute.sh
TEST: IPv6 traceroute [ OK ]
TEST: IPv6 traceroute with VRF [ OK ]
TEST: IPv4 traceroute [ OK ]
TEST: IPv4 traceroute with VRF [FAIL]
traceroute did not return 1.0.3.1
$ echo $?
1
The test fails because the ICMP error message is sent with the VRF
device's IP (1.0.4.1):
# traceroute -n -s 1.0.1.3 1.0.2.4
traceroute to 1.0.2.4 (1.0.2.4), 30 hops max, 60 byte packets
1 1.0.4.1 0.165 ms 0.110 ms 0.103 ms
2 1.0.2.4 0.098 ms 0.085 ms 0.078 ms
# traceroute -n -s 1.0.3.3 1.0.2.4
traceroute to 1.0.2.4 (1.0.2.4), 30 hops max, 60 byte packets
1 1.0.4.1 0.201 ms 0.138 ms 0.129 ms
2 1.0.2.4 0.123 ms 0.105 ms 0.098 ms
With IPv4 change:
# ./traceroute.sh
TEST: IPv6 traceroute [ OK ]
TEST: IPv6 traceroute with VRF [ OK ]
TEST: IPv4 traceroute [ OK ]
TEST: IPv4 traceroute with VRF [ OK ]
$ echo $?
0
Reviewed-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
tools/testing/selftests/net/traceroute.sh | 178 ++++++++++++++++++++++
1 file changed, 178 insertions(+)
diff --git a/tools/testing/selftests/net/traceroute.sh b/tools/testing/selftests/net/traceroute.sh
index 0ab9eccf1499..dbb34c7e09ce 100755
--- a/tools/testing/selftests/net/traceroute.sh
+++ b/tools/testing/selftests/net/traceroute.sh
@@ -193,6 +193,110 @@ run_traceroute6()
cleanup_traceroute6
}
+################################################################################
+# traceroute6 with VRF test
+#
+# Verify that in this scenario
+#
+# ------------------------ N2
+# | |
+# ------ ------ N3 ----
+# | R1 | | R2 |------|H2|
+# ------ ------ ----
+# | |
+# ------------------------ N1
+# |
+# ----
+# |H1|
+# ----
+#
+# Where H1's default route goes through R1 and R1's default route goes through
+# R2 over N2, traceroute6 from H1 to H2 reports R2's address on N2 and not N1.
+# The interfaces connecting R2 to the different subnets are membmer in a VRF
+# and the intention is to check that traceroute6 does not report the VRF's
+# address.
+#
+# Addresses are assigned as follows:
+#
+# N1: 2000:101::/64
+# N2: 2000:102::/64
+# N3: 2000:103::/64
+#
+# R1's host part of address: 1
+# R2's host part of address: 2
+# H1's host part of address: 3
+# H2's host part of address: 4
+#
+# For example:
+# the IPv6 address of R1's interface on N2 is 2000:102::1/64
+
+cleanup_traceroute6_vrf()
+{
+ cleanup_all_ns
+}
+
+setup_traceroute6_vrf()
+{
+ # Start clean
+ cleanup_traceroute6_vrf
+
+ setup_ns h1 h2 r1 r2
+ create_ns "$h1"
+ create_ns "$h2"
+ create_ns "$r1"
+ create_ns "$r2"
+
+ ip -n "$r2" link add name vrf100 up type vrf table 100
+ ip -n "$r2" addr add 2001:db8:100::1/64 dev vrf100
+
+ # Setup N3
+ connect_ns "$r2" eth3 - 2000:103::2/64 "$h2" eth3 - 2000:103::4/64
+
+ ip -n "$r2" link set dev eth3 master vrf100
+
+ ip -n "$h2" route add default via 2000:103::2
+
+ # Setup N2
+ connect_ns "$r1" eth2 - 2000:102::1/64 "$r2" eth2 - 2000:102::2/64
+
+ ip -n "$r1" route add default via 2000:102::2
+
+ ip -n "$r2" link set dev eth2 master vrf100
+
+ # Setup N1. host-1 and router-2 connect to a bridge in router-1.
+ ip -n "$r1" link add name br100 up type bridge
+ ip -n "$r1" addr add 2000:101::1/64 dev br100
+
+ connect_ns "$h1" eth0 - 2000:101::3/64 "$r1" eth0 - -
+
+ ip -n "$h1" route add default via 2000:101::1
+
+ ip -n "$r1" link set dev eth0 master br100
+
+ connect_ns "$r2" eth1 - 2000:101::2/64 "$r1" eth1 - -
+
+ ip -n "$r2" link set dev eth1 master vrf100
+
+ ip -n "$r1" link set dev eth1 master br100
+
+ # Prime the network
+ ip netns exec "$h1" ping6 -c5 2000:103::4 >/dev/null 2>&1
+}
+
+run_traceroute6_vrf()
+{
+ setup_traceroute6_vrf
+
+ RET=0
+
+ # traceroute6 host-2 from host-1 (expects 2000:102::2)
+ run_cmd "$h1" "traceroute6 2000:103::4 | grep 2000:102::2"
+ check_err $? "traceroute6 did not return 2000:102::2"
+ log_test "IPv6 traceroute with VRF"
+
+ cleanup_traceroute6_vrf
+}
+
################################################################################
# traceroute test
#
@@ -261,13 +365,87 @@ run_traceroute()
cleanup_traceroute
}
+################################################################################
+# traceroute with VRF test
+#
+# Verify that traceroute from H1 to H2 shows 1.0.3.1 and 1.0.1.1 when
+# traceroute uses 1.0.3.3 and 1.0.1.3 as the source IP, respectively. The
+# intention is to check that the kernel does not choose an IP assigned to the
+# VRF device, but rather an address from the VRF port (eth1) that received the
+# packet that generates the ICMP error message.
+#
+# 1.0.4.1/24 (vrf100)
+# 1.0.3.3/24 1.0.3.1/24
+# ---- 1.0.1.3/24 1.0.1.1/24 ---- 1.0.2.1/24 1.0.2.4/24 ----
+# |H1|--------------------------|R1|--------------------------|H2|
+# ---- N1 ---- N2 ----
+
+cleanup_traceroute_vrf()
+{
+ cleanup_all_ns
+}
+
+setup_traceroute_vrf()
+{
+ # Start clean
+ cleanup_traceroute_vrf
+
+ setup_ns h1 h2 router
+ create_ns "$h1"
+ create_ns "$h2"
+ create_ns "$router"
+
+ ip -n "$router" link add name vrf100 up type vrf table 100
+ ip -n "$router" addr add 1.0.4.1/24 dev vrf100
+
+ connect_ns "$h1" eth0 1.0.1.3/24 - \
+ "$router" eth1 1.0.1.1/24 -
+
+ ip -n "$h1" addr add 1.0.3.3/24 dev eth0
+ ip -n "$h1" route add default via 1.0.1.1
+
+ ip -n "$router" link set dev eth1 master vrf100
+ ip -n "$router" addr add 1.0.3.1/24 dev eth1
+ ip netns exec "$router" sysctl -qw \
+ net.ipv4.icmp_errors_use_inbound_ifaddr=1
+
+ connect_ns "$h2" eth0 1.0.2.4/24 - \
+ "$router" eth2 1.0.2.1/24 -
+
+ ip -n "$h2" route add default via 1.0.2.1
+
+ ip -n "$router" link set dev eth2 master vrf100
+
+ # Prime the network
+ ip netns exec "$h1" ping -c5 1.0.2.4 >/dev/null 2>&1
+}
+
+run_traceroute_vrf()
+{
+ setup_traceroute_vrf
+
+ RET=0
+
+ # traceroute host-2 from host-1. Expect a source IP that is on the same
+ # subnet as destination IP of the ICMP error message.
+ run_cmd "$h1" "traceroute -s 1.0.1.3 1.0.2.4 | grep 1.0.1.1"
+ check_err $? "traceroute did not return 1.0.1.1"
+ run_cmd "$h1" "traceroute -s 1.0.3.3 1.0.2.4 | grep 1.0.3.1"
+ check_err $? "traceroute did not return 1.0.3.1"
+ log_test "IPv4 traceroute with VRF"
+
+ cleanup_traceroute_vrf
+}
+
################################################################################
# Run tests
run_tests()
{
run_traceroute6
+ run_traceroute6_vrf
run_traceroute
+ run_traceroute_vrf
}
################################################################################
--
2.51.0
^ permalink raw reply related
* [PATCH net-next v2 7/8] selftests: traceroute: Test traceroute with different source IPs
From: Ido Schimmel @ 2025-09-08 7:32 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, horms, paul, dsahern, petrm,
linux-security-module, Ido Schimmel
In-Reply-To: <20250908073238.119240-1-idosch@nvidia.com>
When generating ICMP error messages, the kernel will prefer a source IP
that is on the same subnet as the destination IP (see
inet_select_addr()). Test this behavior by invoking traceroute with
different source IPs and checking that the ICMP error message is
generated with a source IP in the same subnet.
Reviewed-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
tools/testing/selftests/net/traceroute.sh | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/net/traceroute.sh b/tools/testing/selftests/net/traceroute.sh
index 8dc4e5d03e43..0ab9eccf1499 100755
--- a/tools/testing/selftests/net/traceroute.sh
+++ b/tools/testing/selftests/net/traceroute.sh
@@ -196,9 +196,10 @@ run_traceroute6()
################################################################################
# traceroute test
#
-# Verify that traceroute from H1 to H2 shows 1.0.1.1 in this scenario
+# Verify that traceroute from H1 to H2 shows 1.0.3.1 and 1.0.1.1 when
+# traceroute uses 1.0.3.3 and 1.0.1.3 as the source IP, respectively.
#
-# 1.0.3.1/24
+# 1.0.3.3/24 1.0.3.1/24
# ---- 1.0.1.3/24 1.0.1.1/24 ---- 1.0.2.1/24 1.0.2.4/24 ----
# |H1|--------------------------|R1|--------------------------|H2|
# ---- N1 ---- N2 ----
@@ -226,6 +227,7 @@ setup_traceroute()
connect_ns $h1 eth0 1.0.1.3/24 - \
$router eth1 1.0.3.1/24 -
+ ip -n "$h1" addr add 1.0.3.3/24 dev eth0
ip netns exec $h1 ip route add default via 1.0.1.1
ip netns exec $router ip addr add 1.0.1.1/24 dev eth1
@@ -248,9 +250,12 @@ run_traceroute()
RET=0
- # traceroute host-2 from host-1 (expects 1.0.1.1). Takes a while.
- run_cmd $h1 "traceroute 1.0.2.4 | grep -q 1.0.1.1"
+ # traceroute host-2 from host-1. Expect a source IP that is on the same
+ # subnet as destination IP of the ICMP error message.
+ run_cmd "$h1" "traceroute -s 1.0.1.3 1.0.2.4 | grep -q 1.0.1.1"
check_err $? "traceroute did not return 1.0.1.1"
+ run_cmd "$h1" "traceroute -s 1.0.3.3 1.0.2.4 | grep -q 1.0.3.1"
+ check_err $? "traceroute did not return 1.0.3.1"
log_test "IPv4 traceroute"
cleanup_traceroute
--
2.51.0
^ permalink raw reply related
* [PATCH net-next v2 4/8] selftests: traceroute: Return correct value on failure
From: Ido Schimmel @ 2025-09-08 7:32 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, horms, paul, dsahern, petrm,
linux-security-module, Ido Schimmel
In-Reply-To: <20250908073238.119240-1-idosch@nvidia.com>
The test always returns success even if some tests were modified to
fail. Fix by converting the test to use the appropriate library
functions instead of using its own functions.
Before:
# ./traceroute.sh
TEST: IPV6 traceroute [FAIL]
TEST: IPV4 traceroute [ OK ]
Tests passed: 1
Tests failed: 1
$ echo $?
0
After:
# ./traceroute.sh
TEST: IPv6 traceroute [FAIL]
traceroute6 did not return 2000:102::2
TEST: IPv4 traceroute [ OK ]
$ echo $?
1
Reviewed-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
tools/testing/selftests/net/traceroute.sh | 38 ++++++-----------------
1 file changed, 9 insertions(+), 29 deletions(-)
diff --git a/tools/testing/selftests/net/traceroute.sh b/tools/testing/selftests/net/traceroute.sh
index 282f14760940..46cb37e124ce 100755
--- a/tools/testing/selftests/net/traceroute.sh
+++ b/tools/testing/selftests/net/traceroute.sh
@@ -10,28 +10,6 @@ PAUSE_ON_FAIL=no
################################################################################
#
-log_test()
-{
- local rc=$1
- local expected=$2
- local msg="$3"
-
- if [ ${rc} -eq ${expected} ]; then
- printf "TEST: %-60s [ OK ]\n" "${msg}"
- nsuccess=$((nsuccess+1))
- else
- ret=1
- nfail=$((nfail+1))
- printf "TEST: %-60s [FAIL]\n" "${msg}"
- if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
- echo
- echo "hit enter to continue, 'q' to quit"
- read a
- [ "$a" = "q" ] && exit 1
- fi
- fi
-}
-
run_cmd()
{
local ns
@@ -210,9 +188,12 @@ run_traceroute6()
setup_traceroute6
+ RET=0
+
# traceroute6 host-2 from host-1 (expects 2000:102::2)
run_cmd $h1 "traceroute6 2000:103::4 | grep -q 2000:102::2"
- log_test $? 0 "IPV6 traceroute"
+ check_err $? "traceroute6 did not return 2000:102::2"
+ log_test "IPv6 traceroute"
cleanup_traceroute6
}
@@ -275,9 +256,12 @@ run_traceroute()
setup_traceroute
+ RET=0
+
# traceroute host-2 from host-1 (expects 1.0.1.1). Takes a while.
run_cmd $h1 "traceroute 1.0.2.4 | grep -q 1.0.1.1"
- log_test $? 0 "IPV4 traceroute"
+ check_err $? "traceroute did not return 1.0.1.1"
+ log_test "IPv4 traceroute"
cleanup_traceroute
}
@@ -294,9 +278,6 @@ run_tests()
################################################################################
# main
-declare -i nfail=0
-declare -i nsuccess=0
-
while getopts :pv o
do
case $o in
@@ -308,5 +289,4 @@ done
run_tests
-printf "\nTests passed: %3d\n" ${nsuccess}
-printf "Tests failed: %3d\n" ${nfail}
+exit "${EXIT_STATUS}"
--
2.51.0
^ permalink raw reply related
* [PATCH net-next v2 5/8] selftests: traceroute: Use require_command()
From: Ido Schimmel @ 2025-09-08 7:32 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, horms, paul, dsahern, petrm,
linux-security-module, Ido Schimmel
In-Reply-To: <20250908073238.119240-1-idosch@nvidia.com>
Use require_command() so that the test will return SKIP (4) when a
required command is not present.
Before:
# ./traceroute.sh
SKIP: Could not run IPV6 test without traceroute6
SKIP: Could not run IPV4 test without traceroute
$ echo $?
0
After:
# ./traceroute.sh
TEST: traceroute6 not installed [SKIP]
$ echo $?
4
Reviewed-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
tools/testing/selftests/net/traceroute.sh | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/tools/testing/selftests/net/traceroute.sh b/tools/testing/selftests/net/traceroute.sh
index 46cb37e124ce..1ac91eebd16f 100755
--- a/tools/testing/selftests/net/traceroute.sh
+++ b/tools/testing/selftests/net/traceroute.sh
@@ -181,11 +181,6 @@ setup_traceroute6()
run_traceroute6()
{
- if [ ! -x "$(command -v traceroute6)" ]; then
- echo "SKIP: Could not run IPV6 test without traceroute6"
- return
- fi
-
setup_traceroute6
RET=0
@@ -249,11 +244,6 @@ setup_traceroute()
run_traceroute()
{
- if [ ! -x "$(command -v traceroute)" ]; then
- echo "SKIP: Could not run IPV4 test without traceroute"
- return
- fi
-
setup_traceroute
RET=0
@@ -287,6 +277,9 @@ do
esac
done
+require_command traceroute6
+require_command traceroute
+
run_tests
exit "${EXIT_STATUS}"
--
2.51.0
^ permalink raw reply related
* [PATCH net-next v2 6/8] selftests: traceroute: Reword comment
From: Ido Schimmel @ 2025-09-08 7:32 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, horms, paul, dsahern, petrm,
linux-security-module, Ido Schimmel
In-Reply-To: <20250908073238.119240-1-idosch@nvidia.com>
Both of the addresses are configured as primary addresses, but the
kernel is expected to choose 10.0.1.1/24 as the source IP of the ICMP
error message since it is on the same subnet as the destination IP of
the message (10.0.1.3/24). Reword the comment to reflect that.
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
tools/testing/selftests/net/traceroute.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/net/traceroute.sh b/tools/testing/selftests/net/traceroute.sh
index 1ac91eebd16f..8dc4e5d03e43 100755
--- a/tools/testing/selftests/net/traceroute.sh
+++ b/tools/testing/selftests/net/traceroute.sh
@@ -203,10 +203,10 @@ run_traceroute6()
# |H1|--------------------------|R1|--------------------------|H2|
# ---- N1 ---- N2 ----
#
-# where net.ipv4.icmp_errors_use_inbound_ifaddr is set on R1 and
-# 1.0.3.1/24 and 1.0.1.1/24 are respectively R1's primary and secondary
-# address on N1.
-#
+# where net.ipv4.icmp_errors_use_inbound_ifaddr is set on R1 and 1.0.3.1/24 and
+# 1.0.1.1/24 are R1's primary addresses on N1. The kernel is expected to prefer
+# a source address that is on the same subnet as the destination IP of the ICMP
+# error message.
cleanup_traceroute()
{
--
2.51.0
^ permalink raw reply related
* [PATCH net-next v2 3/8] ipv4: icmp: Fix source IP derivation in presence of VRFs
From: Ido Schimmel @ 2025-09-08 7:32 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, horms, paul, dsahern, petrm,
linux-security-module, Ido Schimmel
In-Reply-To: <20250908073238.119240-1-idosch@nvidia.com>
When the "icmp_errors_use_inbound_ifaddr" sysctl is enabled, the source
IP of ICMP error messages should be the "primary address of the
interface that received the packet that caused the icmp error".
The IPv4 ICMP code determines this interface using inet_iif() which in
the input path translates to skb->skb_iif. If the interface that
received the packet is a VRF port, skb->skb_iif will contain the ifindex
of the VRF device and not that of the receiving interface. This is
because in the input path the VRF driver overrides skb->skb_iif with the
ifindex of the VRF device itself (see vrf_ip_rcv()).
As such, the source IP that will be chosen for the ICMP error message is
either an address assigned to the VRF device itself (if present) or an
address assigned to some VRF port, not necessarily the input or output
interface.
This behavior is especially problematic when the error messages are
"Time Exceeded" messages as it means that utilities like traceroute will
show an incorrect packet path.
Solve this by determining the input interface based on the iif field in
the control block, if present. This field is set in the input path to
skb->skb_iif and is not later overridden by the VRF driver, unlike
skb->skb_iif.
This behavior is consistent with the IPv6 counterpart that already uses
the iif from the control block.
Reported-by: Andy Roulin <aroulin@nvidia.com>
Reported-by: Rajkumar Srinivasan <rajsrinivasa@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
net/ipv4/icmp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 59fd0e1993a6..1b7fb5d935ed 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -710,7 +710,8 @@ void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
rcu_read_lock();
if (rt_is_input_route(rt) &&
READ_ONCE(net->ipv4.sysctl_icmp_errors_use_inbound_ifaddr))
- dev = dev_get_by_index_rcu(net, inet_iif(skb_in));
+ dev = dev_get_by_index_rcu(net, parm->iif ? parm->iif :
+ inet_iif(skb_in));
if (dev)
saddr = inet_select_addr(dev, iph->saddr,
--
2.51.0
^ permalink raw reply related
* [PATCH net-next v2 2/8] ipv4: icmp: Pass IPv4 control block structure as an argument to __icmp_send()
From: Ido Schimmel @ 2025-09-08 7:32 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, horms, paul, dsahern, petrm,
linux-security-module, Ido Schimmel
In-Reply-To: <20250908073238.119240-1-idosch@nvidia.com>
__icmp_send() is used to generate ICMP error messages in response to
various situations such as MTU errors (i.e., "Fragmentation Required")
and too many hops (i.e., "Time Exceeded").
The skb that generated the error does not necessarily come from the IPv4
layer and does not always have a valid IPv4 control block in skb->cb.
Therefore, commit 9ef6b42ad6fd ("net: Add __icmp_send helper.") changed
the function to take the IP options structure as argument instead of
deriving it from the skb's control block. Some callers of this function
such as icmp_send() pass the IP options structure from the skb's control
block as in these call paths the control block is known to be valid, but
other callers simply pass a zeroed structure.
A subsequent patch will need __icmp_send() to access more information
from the IPv4 control block (specifically, the ifindex of the input
interface). As a preparation for this change, change the function to
take the IPv4 control block structure as an argument instead of the IP
options structure. This makes the function similar to its IPv6
counterpart that already takes the IPv6 control block structure as an
argument.
No functional changes intended.
Reviewed-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
include/net/icmp.h | 10 ++++++----
net/ipv4/cipso_ipv4.c | 12 ++++++------
net/ipv4/icmp.c | 12 +++++++-----
net/ipv4/route.c | 10 +++++-----
4 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/include/net/icmp.h b/include/net/icmp.h
index caddf4a59ad1..935ee13d9ae9 100644
--- a/include/net/icmp.h
+++ b/include/net/icmp.h
@@ -37,10 +37,10 @@ struct sk_buff;
struct net;
void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
- const struct ip_options *opt);
+ const struct inet_skb_parm *parm);
static inline void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
{
- __icmp_send(skb_in, type, code, info, &IPCB(skb_in)->opt);
+ __icmp_send(skb_in, type, code, info, IPCB(skb_in));
}
#if IS_ENABLED(CONFIG_NF_NAT)
@@ -48,8 +48,10 @@ void icmp_ndo_send(struct sk_buff *skb_in, int type, int code, __be32 info);
#else
static inline void icmp_ndo_send(struct sk_buff *skb_in, int type, int code, __be32 info)
{
- struct ip_options opts = { 0 };
- __icmp_send(skb_in, type, code, info, &opts);
+ struct inet_skb_parm parm;
+
+ memset(&parm, 0, sizeof(parm));
+ __icmp_send(skb_in, type, code, info, &parm);
}
#endif
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index c7c949c37e2d..709021197e1c 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -1715,7 +1715,7 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option)
*/
void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
{
- struct ip_options opt;
+ struct inet_skb_parm parm;
int res;
if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES)
@@ -1726,19 +1726,19 @@ void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
* so we can not use icmp_send and IPCB here.
*/
- memset(&opt, 0, sizeof(opt));
- opt.optlen = ip_hdr(skb)->ihl * 4 - sizeof(struct iphdr);
+ memset(&parm, 0, sizeof(parm));
+ parm.opt.optlen = ip_hdr(skb)->ihl * 4 - sizeof(struct iphdr);
rcu_read_lock();
- res = __ip_options_compile(dev_net(skb->dev), &opt, skb, NULL);
+ res = __ip_options_compile(dev_net(skb->dev), &parm.opt, skb, NULL);
rcu_read_unlock();
if (res)
return;
if (gateway)
- __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, &opt);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, &parm);
else
- __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, &opt);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, &parm);
}
/**
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 863bf5023f2a..59fd0e1993a6 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -594,7 +594,7 @@ static struct rtable *icmp_route_lookup(struct net *net, struct flowi4 *fl4,
*/
void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
- const struct ip_options *opt)
+ const struct inet_skb_parm *parm)
{
struct iphdr *iph;
int room;
@@ -725,7 +725,8 @@ void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
iph->tos;
mark = IP4_REPLY_MARK(net, skb_in->mark);
- if (__ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in, opt))
+ if (__ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in,
+ &parm->opt))
goto out_unlock;
@@ -799,15 +800,16 @@ EXPORT_SYMBOL(__icmp_send);
void icmp_ndo_send(struct sk_buff *skb_in, int type, int code, __be32 info)
{
struct sk_buff *cloned_skb = NULL;
- struct ip_options opts = { 0 };
enum ip_conntrack_info ctinfo;
enum ip_conntrack_dir dir;
+ struct inet_skb_parm parm;
struct nf_conn *ct;
__be32 orig_ip;
+ memset(&parm, 0, sizeof(parm));
ct = nf_ct_get(skb_in, &ctinfo);
if (!ct || !(READ_ONCE(ct->status) & IPS_NAT_MASK)) {
- __icmp_send(skb_in, type, code, info, &opts);
+ __icmp_send(skb_in, type, code, info, &parm);
return;
}
@@ -823,7 +825,7 @@ void icmp_ndo_send(struct sk_buff *skb_in, int type, int code, __be32 info)
orig_ip = ip_hdr(skb_in)->saddr;
dir = CTINFO2DIR(ctinfo);
ip_hdr(skb_in)->saddr = ct->tuplehash[dir].tuple.src.u3.ip;
- __icmp_send(skb_in, type, code, info, &opts);
+ __icmp_send(skb_in, type, code, info, &parm);
ip_hdr(skb_in)->saddr = orig_ip;
out:
consume_skb(cloned_skb);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 50309f2ab132..6d27d3610c1c 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1222,8 +1222,8 @@ EXPORT_INDIRECT_CALLABLE(ipv4_dst_check);
static void ipv4_send_dest_unreach(struct sk_buff *skb)
{
+ struct inet_skb_parm parm;
struct net_device *dev;
- struct ip_options opt;
int res;
/* Recompile ip options since IPCB may not be valid anymore.
@@ -1233,21 +1233,21 @@ static void ipv4_send_dest_unreach(struct sk_buff *skb)
ip_hdr(skb)->version != 4 || ip_hdr(skb)->ihl < 5)
return;
- memset(&opt, 0, sizeof(opt));
+ memset(&parm, 0, sizeof(parm));
if (ip_hdr(skb)->ihl > 5) {
if (!pskb_network_may_pull(skb, ip_hdr(skb)->ihl * 4))
return;
- opt.optlen = ip_hdr(skb)->ihl * 4 - sizeof(struct iphdr);
+ parm.opt.optlen = ip_hdr(skb)->ihl * 4 - sizeof(struct iphdr);
rcu_read_lock();
dev = skb->dev ? skb->dev : skb_rtable(skb)->dst.dev;
- res = __ip_options_compile(dev_net(dev), &opt, skb, NULL);
+ res = __ip_options_compile(dev_net(dev), &parm.opt, skb, NULL);
rcu_read_unlock();
if (res)
return;
}
- __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_UNREACH, 0, &opt);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_UNREACH, 0, &parm);
}
static void ipv4_link_failure(struct sk_buff *skb)
--
2.51.0
^ permalink raw reply related
* [PATCH net-next v2 1/8] ipv4: cipso: Simplify IP options handling in cipso_v4_error()
From: Ido Schimmel @ 2025-09-08 7:32 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, horms, paul, dsahern, petrm,
linux-security-module, Ido Schimmel
In-Reply-To: <20250908073238.119240-1-idosch@nvidia.com>
When __ip_options_compile() is called with an skb, the IP options are
parsed from the skb data into the provided IP option argument. This is
in contrast to the case where the skb argument is NULL and the options
are parsed from opt->__data.
Given that cipso_v4_error() always passes an skb to
__ip_options_compile(), there is no need to allocate an extra 40 bytes
(maximum IP options size).
Therefore, simplify the function by removing these extra bytes and make
the function similar to ipv4_send_dest_unreach() which also calls both
__ip_options_compile() and __icmp_send().
This is a preparation for changing the arguments being passed to
__icmp_send().
No functional changes intended.
Reviewed-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Acked-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
net/ipv4/cipso_ipv4.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 740af8541d2f..c7c949c37e2d 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -1715,8 +1715,7 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option)
*/
void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
{
- unsigned char optbuf[sizeof(struct ip_options) + 40];
- struct ip_options *opt = (struct ip_options *)optbuf;
+ struct ip_options opt;
int res;
if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES)
@@ -1727,19 +1726,19 @@ void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
* so we can not use icmp_send and IPCB here.
*/
- memset(opt, 0, sizeof(struct ip_options));
- opt->optlen = ip_hdr(skb)->ihl*4 - sizeof(struct iphdr);
+ memset(&opt, 0, sizeof(opt));
+ opt.optlen = ip_hdr(skb)->ihl * 4 - sizeof(struct iphdr);
rcu_read_lock();
- res = __ip_options_compile(dev_net(skb->dev), opt, skb, NULL);
+ res = __ip_options_compile(dev_net(skb->dev), &opt, skb, NULL);
rcu_read_unlock();
if (res)
return;
if (gateway)
- __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, opt);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, &opt);
else
- __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, opt);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, &opt);
}
/**
--
2.51.0
^ permalink raw reply related
* [PATCH net-next v2 0/8] ipv4: icmp: Fix source IP derivation in presence of VRFs
From: Ido Schimmel @ 2025-09-08 7:32 UTC (permalink / raw)
To: netdev
Cc: davem, kuba, pabeni, edumazet, horms, paul, dsahern, petrm,
linux-security-module, Ido Schimmel
Align IPv4 with IPv6 and in the presence of VRFs generate ICMP error
messages with a source IP that is derived from the receiving interface
and not from its VRF master. This is especially important when the error
messages are "Time Exceeded" messages as it means that utilities like
traceroute will show an incorrect packet path.
Patches #1-#2 are preparations.
Patch #3 is the actual change.
Patches #4-#7 make small improvements in the existing traceroute test.
Patch #8 extends the traceroute test with VRF test cases for both IPv4
and IPv6.
Changes since v1 [1]:
* Rebase.
[1] https://lore.kernel.org/netdev/20250901083027.183468-1-idosch@nvidia.com/
Ido Schimmel (8):
ipv4: cipso: Simplify IP options handling in cipso_v4_error()
ipv4: icmp: Pass IPv4 control block structure as an argument to
__icmp_send()
ipv4: icmp: Fix source IP derivation in presence of VRFs
selftests: traceroute: Return correct value on failure
selftests: traceroute: Use require_command()
selftests: traceroute: Reword comment
selftests: traceroute: Test traceroute with different source IPs
selftests: traceroute: Add VRF tests
include/net/icmp.h | 10 +-
net/ipv4/cipso_ipv4.c | 13 +-
net/ipv4/icmp.c | 15 +-
net/ipv4/route.c | 10 +-
tools/testing/selftests/net/traceroute.sh | 250 ++++++++++++++++++----
5 files changed, 229 insertions(+), 69 deletions(-)
--
2.51.0
^ permalink raw reply
* Re: [PATCH v3 29/34] apparmor: move initcalls to the LSM framework
From: John Johansen @ 2025-09-08 7:12 UTC (permalink / raw)
To: Paul Moore, linux-security-module, linux-integrity, selinux
Cc: Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <20250814225159.275901-65-paul@paul-moore.com>
On 8/14/25 15:50, Paul Moore wrote:
> Reviewed-by: Kees Cook <kees@kernel.org>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Sorry this took so long.
So the patch itself looks good, and can have my
Acked-by: John Johansen <john.johansen@canonical.com>
with that said I have done 3 different builds with this series. Tweaking the base,
and the config, and I haven't been able to successfully booted any of them. I am
not sure what I am missing yet. I working on a bisect, but its just a side project
atm.
Until I can get a successful boot, and test. I am going to refrain from finishing
out the review.
> ---
> security/apparmor/apparmorfs.c | 4 +---
> security/apparmor/crypto.c | 3 +--
> security/apparmor/include/apparmorfs.h | 2 ++
> security/apparmor/include/crypto.h | 1 +
> security/apparmor/lsm.c | 9 ++++++++-
> 5 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 391a586d0557..ee04c1ac9d6e 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -2649,7 +2649,7 @@ static const struct inode_operations policy_link_iops = {
> *
> * Returns: error on failure
> */
> -static int __init aa_create_aafs(void)
> +int __init aa_create_aafs(void)
> {
> struct dentry *dent;
> int error;
> @@ -2728,5 +2728,3 @@ static int __init aa_create_aafs(void)
> AA_ERROR("Error creating AppArmor securityfs\n");
> return error;
> }
> -
> -fs_initcall(aa_create_aafs);
> diff --git a/security/apparmor/crypto.c b/security/apparmor/crypto.c
> index 227d47c14907..d8a7bde94d79 100644
> --- a/security/apparmor/crypto.c
> +++ b/security/apparmor/crypto.c
> @@ -53,10 +53,9 @@ int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start,
> return 0;
> }
>
> -static int __init init_profile_hash(void)
> +int __init init_profile_hash(void)
> {
> if (apparmor_initialized)
> aa_info_message("AppArmor sha256 policy hashing enabled");
> return 0;
> }
> -late_initcall(init_profile_hash);
> diff --git a/security/apparmor/include/apparmorfs.h b/security/apparmor/include/apparmorfs.h
> index 1e94904f68d9..dd580594dfb7 100644
> --- a/security/apparmor/include/apparmorfs.h
> +++ b/security/apparmor/include/apparmorfs.h
> @@ -104,6 +104,8 @@ enum aafs_prof_type {
> #define prof_dir(X) ((X)->dents[AAFS_PROF_DIR])
> #define prof_child_dir(X) ((X)->dents[AAFS_PROF_PROFS])
>
> +int aa_create_aafs(void);
> +
> void __aa_bump_ns_revision(struct aa_ns *ns);
> void __aafs_profile_rmdir(struct aa_profile *profile);
> void __aafs_profile_migrate_dents(struct aa_profile *old,
> diff --git a/security/apparmor/include/crypto.h b/security/apparmor/include/crypto.h
> index 636a04e20d91..f3ffd388cc58 100644
> --- a/security/apparmor/include/crypto.h
> +++ b/security/apparmor/include/crypto.h
> @@ -13,6 +13,7 @@
> #include "policy.h"
>
> #ifdef CONFIG_SECURITY_APPARMOR_HASH
> +int init_profile_hash(void);
> unsigned int aa_hash_size(void);
> char *aa_calc_hash(void *data, size_t len);
> int aa_calc_profile_hash(struct aa_profile *profile, u32 version, void *start,
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 45b3a304d525..647c13e13e63 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -32,6 +32,7 @@
> #include "include/audit.h"
> #include "include/capability.h"
> #include "include/cred.h"
> +#include "include/crypto.h"
> #include "include/file.h"
> #include "include/ipc.h"
> #include "include/net.h"
> @@ -2426,7 +2427,6 @@ static int __init apparmor_nf_ip_init(void)
>
> return 0;
> }
> -__initcall(apparmor_nf_ip_init);
> #endif
>
> static char nulldfa_src[] __aligned(8) = {
> @@ -2557,4 +2557,11 @@ DEFINE_LSM(apparmor) = {
> .enabled = &apparmor_enabled,
> .blobs = &apparmor_blob_sizes,
> .init = apparmor_init,
> + .initcall_fs = aa_create_aafs,
> +#if defined(CONFIG_NETFILTER) && defined(CONFIG_NETWORK_SECMARK)
> + .initcall_device = apparmor_nf_ip_init,
> +#endif
> +#ifdef CONFIG_SECURITY_APPARMOR_HASH
> + .initcall_late = init_profile_hash,
> +#endif
>
^ permalink raw reply
* Re: [PATCH] ima,evm: move initcalls to the LSM framework
From: Mimi Zohar @ 2025-09-08 2:45 UTC (permalink / raw)
To: Paul Moore
Cc: Roberto Sassu, roberto.sassu, linux-security-module,
linux-integrity, selinux, john.johansen, wufan, mic, kees,
mortonm, casey, penguin-kernel, nicolas.bouchinet, xiujianfeng
In-Reply-To: <CAHC9VhTGAcMTXHReinybpLzer7seCN+NUTHcFte+aU2oRNtNNg@mail.gmail.com>
On Sun, 2025-09-07 at 21:08 -0400, Paul Moore wrote:
> On Sun, Sep 7, 2025 at 5:18 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Tue, 2025-09-02 at 14:54 +0200, Roberto Sassu wrote:
> > > From: Paul Moore <paul@paul-moore.com>
> >
> > Remove above ...
> >
> > >
> > > This patch converts IMA and EVM to use the LSM frameworks's initcall
> > > mechanism. It moved the integrity_fs_init() call to ima_fs_init() and
> > > evm_init_secfs(), to work around the fact that there is no "integrity" LSM,
> > > and introduced integrity_fs_fini() to remove the integrity directory, if
> > > empty. Both integrity_fs_init() and integrity_fs_fini() support the
> > > scenario of being called by both the IMA and EVM LSMs.
> > >
> > > It is worth mentioning that this patch does not touch any of the
> > > "platform certs" code that lives in the security/integrity/platform_certs
> > > directory as the IMA/EVM maintainers have assured me that this code is
> > > unrelated to IMA/EVM, despite the location, and will be moved to a more
> >
> > This wording "unrelated to IMA/EVM" was taken from Paul's patch description, but
> > needs to be tweaked. Please refer to my comment on Paul's patch.
>
> Minim, Roberto, would both of you be okay if I changed the second
> paragraph to read as follows:
>
> "This patch does not touch any of the platform certificate code that
> lives under the security/integrity/platform_certs directory as the
> IMA/EVM developers would prefer to address that in a future patchset."
That's fine.
>
> > > relevant subsystem in the future.
> > >
> > > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> >
> > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>, but not yet tested.
>
^ permalink raw reply
* [PATCH v2] memfd,selinux: call security_inode_init_security_anon
From: Thiébaud Weksteen @ 2025-09-08 1:34 UTC (permalink / raw)
To: Paul Moore, James Morris, Stephen Smalley, Hugh Dickins,
Jeff Vander Stoep, Nick Kralevich, Jeff Xu, Baolin Wang,
Isaac Manjarres
Cc: Thiébaud Weksteen, linux-kernel, linux-security-module,
selinux, linux-mm
Prior to this change, no security hooks were called at the creation of a
memfd file. It means that, for SELinux as an example, it will receive
the default type of the filesystem that backs the in-memory inode. In
most cases, that would be tmpfs, but if MFD_HUGETLB is passed, it will
be hugetlbfs. Both can be considered implementation details of memfd.
It also means that it is not possible to differentiate between a file
coming from memfd_create and a file coming from a standard tmpfs mount
point.
Additionally, no permission is validated at creation, which differs from
the similar memfd_secret syscall.
Call security_inode_init_security_anon during creation. This ensures
that the file is setup similarly to other anonymous inodes. On SELinux,
it means that the file will receive the security context of its task.
The ability to limit fexecve on memfd has been of interest to avoid
potential pitfalls where /proc/self/exe or similar would be executed
[1][2]. Reuse the "execute_no_trans" and "entrypoint" access vectors,
similarly to the file class. These access vectors may not make sense for
the existing "anon_inode" class. Therefore, define and assign a new
class "memfd_file" to support such access vectors.
Guard these changes behind a new policy capability named "memfd_class".
[1] https://crbug.com/1305267
[2] https://lore.kernel.org/lkml/20221215001205.51969-1-jeffxu@google.com/
Signed-off-by: Thiébaud Weksteen <tweek@google.com>
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
---
Changes since v1:
- Move test of class earlier in selinux_bprm_creds_for_exec
- Remove duplicate call to security_transition_sid
Changes since RFC:
- Remove enum argument, simply compare the anon inode name
- Introduce a policy capability for compatility
- Add validation of class in selinux_bprm_creds_for_exec
include/linux/memfd.h | 2 ++
mm/memfd.c | 14 ++++++++++--
security/selinux/hooks.c | 26 +++++++++++++++++-----
security/selinux/include/classmap.h | 2 ++
security/selinux/include/policycap.h | 1 +
security/selinux/include/policycap_names.h | 1 +
security/selinux/include/security.h | 5 +++++
7 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/include/linux/memfd.h b/include/linux/memfd.h
index 6f606d9573c3..cc74de3dbcfe 100644
--- a/include/linux/memfd.h
+++ b/include/linux/memfd.h
@@ -4,6 +4,8 @@
#include <linux/file.h>
+#define MEMFD_ANON_NAME "[memfd]"
+
#ifdef CONFIG_MEMFD_CREATE
extern long memfd_fcntl(struct file *file, unsigned int cmd, unsigned int arg);
struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx);
diff --git a/mm/memfd.c b/mm/memfd.c
index bbe679895ef6..63b439eb402a 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c
@@ -433,6 +433,8 @@ static struct file *alloc_file(const char *name, unsigned int flags)
{
unsigned int *file_seals;
struct file *file;
+ struct inode *inode;
+ int err = 0;
if (flags & MFD_HUGETLB) {
file = hugetlb_file_setup(name, 0, VM_NORESERVE,
@@ -444,12 +446,20 @@ static struct file *alloc_file(const char *name, unsigned int flags)
}
if (IS_ERR(file))
return file;
+
+ inode = file_inode(file);
+ err = security_inode_init_security_anon(inode,
+ &QSTR(MEMFD_ANON_NAME), NULL);
+ if (err) {
+ fput(file);
+ file = ERR_PTR(err);
+ return file;
+ }
+
file->f_mode |= FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE;
file->f_flags |= O_LARGEFILE;
if (flags & MFD_NOEXEC_SEAL) {
- struct inode *inode = file_inode(file);
-
inode->i_mode &= ~0111;
file_seals = memfd_file_seals_ptr(file);
if (file_seals) {
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c95a5874bf7d..6adf2f393ed9 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -93,6 +93,7 @@
#include <linux/fanotify.h>
#include <linux/io_uring/cmd.h>
#include <uapi/linux/lsm.h>
+#include <linux/memfd.h>
#include "avc.h"
#include "objsec.h"
@@ -2315,6 +2316,9 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
new_tsec = selinux_cred(bprm->cred);
isec = inode_security(inode);
+ if (isec->sclass != SECCLASS_FILE && isec->sclass != SECCLASS_MEMFD_FILE)
+ return -EPERM;
+
/* Default to the current task SID. */
new_tsec->sid = old_tsec->sid;
new_tsec->osid = old_tsec->sid;
@@ -2366,9 +2370,10 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
ad.type = LSM_AUDIT_DATA_FILE;
ad.u.file = bprm->file;
+
if (new_tsec->sid == old_tsec->sid) {
- rc = avc_has_perm(old_tsec->sid, isec->sid,
- SECCLASS_FILE, FILE__EXECUTE_NO_TRANS, &ad);
+ rc = avc_has_perm(old_tsec->sid, isec->sid, isec->sclass,
+ FILE__EXECUTE_NO_TRANS, &ad);
if (rc)
return rc;
} else {
@@ -2378,8 +2383,8 @@ static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm)
if (rc)
return rc;
- rc = avc_has_perm(new_tsec->sid, isec->sid,
- SECCLASS_FILE, FILE__ENTRYPOINT, &ad);
+ rc = avc_has_perm(new_tsec->sid, isec->sid, isec->sclass,
+ FILE__ENTRYPOINT, &ad);
if (rc)
return rc;
@@ -2974,10 +2979,18 @@ static int selinux_inode_init_security_anon(struct inode *inode,
struct common_audit_data ad;
struct inode_security_struct *isec;
int rc;
+ bool is_memfd = false;
if (unlikely(!selinux_initialized()))
return 0;
+ if (name != NULL && name->name != NULL &&
+ !strcmp(name->name, MEMFD_ANON_NAME)) {
+ if (!selinux_policycap_memfd_class())
+ return 0;
+ is_memfd = true;
+ }
+
isec = selinux_inode(inode);
/*
@@ -2997,7 +3010,10 @@ static int selinux_inode_init_security_anon(struct inode *inode,
isec->sclass = context_isec->sclass;
isec->sid = context_isec->sid;
} else {
- isec->sclass = SECCLASS_ANON_INODE;
+ if (is_memfd)
+ isec->sclass = SECCLASS_MEMFD_FILE;
+ else
+ isec->sclass = SECCLASS_ANON_INODE;
rc = security_transition_sid(
sid, sid,
isec->sclass, name, &isec->sid);
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index 5665aa5e7853..3ec85142771f 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -179,6 +179,8 @@ const struct security_class_mapping secclass_map[] = {
{ "anon_inode", { COMMON_FILE_PERMS, NULL } },
{ "io_uring", { "override_creds", "sqpoll", "cmd", "allowed", NULL } },
{ "user_namespace", { "create", NULL } },
+ { "memfd_file",
+ { COMMON_FILE_PERMS, "execute_no_trans", "entrypoint", NULL } },
/* last one */ { NULL, {} }
};
diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h
index 7405154e6c42..dabcc9f14dde 100644
--- a/security/selinux/include/policycap.h
+++ b/security/selinux/include/policycap.h
@@ -17,6 +17,7 @@ enum {
POLICYDB_CAP_NETLINK_XPERM,
POLICYDB_CAP_NETIF_WILDCARD,
POLICYDB_CAP_GENFS_SECLABEL_WILDCARD,
+ POLICYDB_CAP_MEMFD_CLASS,
__POLICYDB_CAP_MAX
};
#define POLICYDB_CAP_MAX (__POLICYDB_CAP_MAX - 1)
diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h
index d8962fcf2ff9..8e96f2a816b6 100644
--- a/security/selinux/include/policycap_names.h
+++ b/security/selinux/include/policycap_names.h
@@ -20,6 +20,7 @@ const char *const selinux_policycap_names[__POLICYDB_CAP_MAX] = {
"netlink_xperm",
"netif_wildcard",
"genfs_seclabel_wildcard",
+ "memfd_class",
};
/* clang-format on */
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 8201e6a3ac0f..72c963f54148 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -209,6 +209,11 @@ static inline bool selinux_policycap_netif_wildcard(void)
selinux_state.policycap[POLICYDB_CAP_NETIF_WILDCARD]);
}
+static inline bool selinux_policycap_memfd_class(void)
+{
+ return READ_ONCE(selinux_state.policycap[POLICYDB_CAP_MEMFD_CLASS]);
+}
+
struct selinux_policy_convert_data;
struct selinux_load_state {
--
2.51.0.384.g4c02a37b29-goog
^ permalink raw reply related
* Re: [PATCH] ima,evm: move initcalls to the LSM framework
From: Paul Moore @ 2025-09-08 1:08 UTC (permalink / raw)
To: Mimi Zohar
Cc: Roberto Sassu, roberto.sassu, linux-security-module,
linux-integrity, selinux, john.johansen, wufan, mic, kees,
mortonm, casey, penguin-kernel, nicolas.bouchinet, xiujianfeng
In-Reply-To: <82f22f97486622408bec772a9b025e301c8fa2f4.camel@linux.ibm.com>
On Sun, Sep 7, 2025 at 5:18 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2025-09-02 at 14:54 +0200, Roberto Sassu wrote:
> > From: Paul Moore <paul@paul-moore.com>
>
> Remove above ...
>
> >
> > This patch converts IMA and EVM to use the LSM frameworks's initcall
> > mechanism. It moved the integrity_fs_init() call to ima_fs_init() and
> > evm_init_secfs(), to work around the fact that there is no "integrity" LSM,
> > and introduced integrity_fs_fini() to remove the integrity directory, if
> > empty. Both integrity_fs_init() and integrity_fs_fini() support the
> > scenario of being called by both the IMA and EVM LSMs.
> >
> > It is worth mentioning that this patch does not touch any of the
> > "platform certs" code that lives in the security/integrity/platform_certs
> > directory as the IMA/EVM maintainers have assured me that this code is
> > unrelated to IMA/EVM, despite the location, and will be moved to a more
>
> This wording "unrelated to IMA/EVM" was taken from Paul's patch description, but
> needs to be tweaked. Please refer to my comment on Paul's patch.
Minim, Roberto, would both of you be okay if I changed the second
paragraph to read as follows:
"This patch does not touch any of the platform certificate code that
lives under the security/integrity/platform_certs directory as the
IMA/EVM developers would prefer to address that in a future patchset."
> > relevant subsystem in the future.
> >
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>, but not yet tested.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 31/34] ima,evm: move initcalls to the LSM framework
From: Paul Moore @ 2025-09-08 1:05 UTC (permalink / raw)
To: Mimi Zohar
Cc: Roberto Sassu, linux-security-module, linux-integrity, selinux,
John Johansen, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Tetsuo Handa, Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <bd46c63ebb9eddfcdc8df92fe9f85473416ea8a0.camel@linux.ibm.com>
On Sun, Sep 7, 2025 at 5:13 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Fri, 2025-08-22 at 16:45 -0400, Paul Moore wrote:
> > On Thu, Aug 14, 2025 at 6:55 PM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > This patch converts IMA and EVM to use the LSM frameworks's initcall
> > > mechanism. There was a minor challenge in this conversion that wasn't
> > > seen when converting the other LSMs brought about by the resource
> > > sharing between the two related, yes independent IMA and EVM LSMs.
> > > This was resolved by registering the same initcalls for each LSM and
> > > including code in each registered initcall to ensure it only executes
> > > once during each boot.
> > >
> > > It is worth mentioning that this patch does not touch any of the
> > > "platform certs" code that lives in the security/integrity/platform_certs
> > > directory as the IMA/EVM maintainers have assured me that this code is
> > > unrelated to IMA/EVM, despite the location, and will be moved to a more
> > > relevant subsystem in the future.
>
> The "unrelated to IMA/EVM" wording misses the point. An exception was made to
> load the pre-boot keys onto the .platform keyring in order for IMA/EVM to verify
> the kexec kernel image appended signature. This exception was subsequently
> extended to verifying the pesigned kexec kernel image signature. (Other
> subsystems are abusing the keys on the .platform keyring to verify other
> signatures.)
>
> Instead of saying "unrelated to IMA/EVM", how about saying something along the
> lines of "IMA has a dependency on the platform and machine keyrings, but this
> dependency isn't limited to IMA/EVM."
>
> Paul, this patch set doesn't apply to cleanly to Linus's tree. What is the base
> commit?
It would have been based on the lsm/dev branch since the LSM tree is
the target, however, given the scope of the patchset and the fact that
it has been several weeks since it was originally posted, I wouldn't
be surprised it if needs some fuzzing when applied on top of lsm/dev
too.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] ima,evm: move initcalls to the LSM framework
From: Mimi Zohar @ 2025-09-07 21:17 UTC (permalink / raw)
To: Roberto Sassu, paul, roberto.sassu
Cc: linux-security-module, linux-integrity, selinux, john.johansen,
wufan, mic, kees, mortonm, casey, penguin-kernel,
nicolas.bouchinet, xiujianfeng
In-Reply-To: <20250902125457.2689519-1-roberto.sassu@huaweicloud.com>
On Tue, 2025-09-02 at 14:54 +0200, Roberto Sassu wrote:
> From: Paul Moore <paul@paul-moore.com>
Remove above ...
>
> This patch converts IMA and EVM to use the LSM frameworks's initcall
> mechanism. It moved the integrity_fs_init() call to ima_fs_init() and
> evm_init_secfs(), to work around the fact that there is no "integrity" LSM,
> and introduced integrity_fs_fini() to remove the integrity directory, if
> empty. Both integrity_fs_init() and integrity_fs_fini() support the
> scenario of being called by both the IMA and EVM LSMs.
>
> It is worth mentioning that this patch does not touch any of the
> "platform certs" code that lives in the security/integrity/platform_certs
> directory as the IMA/EVM maintainers have assured me that this code is
> unrelated to IMA/EVM, despite the location, and will be moved to a more
This wording "unrelated to IMA/EVM" was taken from Paul's patch description, but
needs to be tweaked. Please refer to my comment on Paul's patch.
> relevant subsystem in the future.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>, but not yet tested.
> ---
> security/integrity/evm/evm_main.c | 3 +--
> security/integrity/evm/evm_secfs.c | 11 +++++++++--
> security/integrity/iint.c | 14 ++++++++++++--
> security/integrity/ima/ima_fs.c | 11 +++++++++--
> security/integrity/ima/ima_main.c | 4 ++--
> security/integrity/integrity.h | 2 ++
> 6 files changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index db8e324ed4e6..73d500a375cb 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -1179,6 +1179,5 @@ DEFINE_LSM(evm) = {
> .init = init_evm_lsm,
> .order = LSM_ORDER_LAST,
> .blobs = &evm_blob_sizes,
> + .initcall_late = init_evm,
> };
> -
> -late_initcall(init_evm);
> diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
> index b0d2aad27850..c26724690cec 100644
> --- a/security/integrity/evm/evm_secfs.c
> +++ b/security/integrity/evm/evm_secfs.c
> @@ -302,10 +302,16 @@ int __init evm_init_secfs(void)
> int error = 0;
> struct dentry *dentry;
>
> - evm_dir = securityfs_create_dir("evm", integrity_dir);
> - if (IS_ERR(evm_dir))
> + error = integrity_fs_init();
> + if (error < 0)
> return -EFAULT;
>
> + evm_dir = securityfs_create_dir("evm", integrity_dir);
> + if (IS_ERR(evm_dir)) {
> + error = -EFAULT;
> + goto out;
> + }
> +
> dentry = securityfs_create_file("evm", 0660,
> evm_dir, NULL, &evm_key_ops);
> if (IS_ERR(dentry)) {
> @@ -329,5 +335,6 @@ int __init evm_init_secfs(void)
> out:
> securityfs_remove(evm_symlink);
> securityfs_remove(evm_dir);
> + integrity_fs_fini();
> return error;
> }
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index 068ac6c2ae1e..8ec1a3436a71 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -42,8 +42,11 @@ void __init integrity_load_keys(void)
> evm_load_x509();
> }
>
> -static int __init integrity_fs_init(void)
> +int __init integrity_fs_init(void)
> {
> + if (integrity_dir)
> + return 0;
> +
> integrity_dir = securityfs_create_dir("integrity", NULL);
> if (IS_ERR(integrity_dir)) {
> int ret = PTR_ERR(integrity_dir);
> @@ -58,4 +61,11 @@ static int __init integrity_fs_init(void)
> return 0;
> }
>
> -late_initcall(integrity_fs_init)
> +void __init integrity_fs_fini(void)
> +{
> + if (!integrity_dir || !simple_empty(integrity_dir))
> + return;
> +
> + securityfs_remove(integrity_dir);
> + integrity_dir = NULL;
> +}
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 87045b09f120..012a58959ff0 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -499,9 +499,15 @@ int __init ima_fs_init(void)
> struct dentry *dentry;
> int ret;
>
> + ret = integrity_fs_init();
> + if (ret < 0)
> + return ret;
> +
> ima_dir = securityfs_create_dir("ima", integrity_dir);
> - if (IS_ERR(ima_dir))
> - return PTR_ERR(ima_dir);
> + if (IS_ERR(ima_dir)) {
> + ret = PTR_ERR(ima_dir);
> + goto out;
> + }
>
> ima_symlink = securityfs_create_symlink("ima", NULL, "integrity/ima",
> NULL);
> @@ -555,6 +561,7 @@ int __init ima_fs_init(void)
> out:
> securityfs_remove(ima_symlink);
> securityfs_remove(ima_dir);
> + integrity_fs_fini();
>
> return ret;
> }
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index eade8e1e3cb1..b703bfc2f470 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -1283,6 +1283,6 @@ DEFINE_LSM(ima) = {
> .init = init_ima_lsm,
> .order = LSM_ORDER_LAST,
> .blobs = &ima_blob_sizes,
> + /* Start IMA after the TPM is available */
> + .initcall_late = init_ima,
> };
> -
> -late_initcall(init_ima); /* Start IMA after the TPM is available */
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index c2c2da691123..7b388b66cf80 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -114,6 +114,8 @@ struct ima_file_id {
>
> int integrity_kernel_read(struct file *file, loff_t offset,
> void *addr, unsigned long count);
> +int __init integrity_fs_init(void);
> +void __init integrity_fs_fini(void);
>
> #define INTEGRITY_KEYRING_EVM 0
> #define INTEGRITY_KEYRING_IMA 1
^ permalink raw reply
* Re: [PATCH v3 31/34] ima,evm: move initcalls to the LSM framework
From: Mimi Zohar @ 2025-09-07 21:11 UTC (permalink / raw)
To: Paul Moore, Roberto Sassu
Cc: linux-security-module, linux-integrity, selinux, John Johansen,
Fan Wu, Mickaël Salaün, Günther Noack, Kees Cook,
Micah Morton, Casey Schaufler, Tetsuo Handa, Nicolas Bouchinet,
Xiu Jianfeng
In-Reply-To: <CAHC9VhS3KdVO9n-dgk1qFzTae0i+Oab8atMmt0CAsMEm1D4v5w@mail.gmail.com>
On Fri, 2025-08-22 at 16:45 -0400, Paul Moore wrote:
> On Thu, Aug 14, 2025 at 6:55 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > This patch converts IMA and EVM to use the LSM frameworks's initcall
> > mechanism. There was a minor challenge in this conversion that wasn't
> > seen when converting the other LSMs brought about by the resource
> > sharing between the two related, yes independent IMA and EVM LSMs.
> > This was resolved by registering the same initcalls for each LSM and
> > including code in each registered initcall to ensure it only executes
> > once during each boot.
> >
> > It is worth mentioning that this patch does not touch any of the
> > "platform certs" code that lives in the security/integrity/platform_certs
> > directory as the IMA/EVM maintainers have assured me that this code is
> > unrelated to IMA/EVM, despite the location, and will be moved to a more
> > relevant subsystem in the future.
The "unrelated to IMA/EVM" wording misses the point. An exception was made to
load the pre-boot keys onto the .platform keyring in order for IMA/EVM to verify
the kexec kernel image appended signature. This exception was subsequently
extended to verifying the pesigned kexec kernel image signature. (Other
subsystems are abusing the keys on the .platform keyring to verify other
signatures.)
Instead of saying "unrelated to IMA/EVM", how about saying something along the
lines of "IMA has a dependency on the platform and machine keyrings, but this
dependency isn't limited to IMA/EVM."
Paul, this patch set doesn't apply to cleanly to Linus's tree. What is the base
commit?
Mimi
^ permalink raw reply
* Re: [PATCH v3 27/34] tomoyo: move initcalls to the LSM framework
From: Tetsuo Handa @ 2025-09-07 7:39 UTC (permalink / raw)
To: Paul Moore; +Cc: linux-security-module
In-Reply-To: <CAHC9VhTJp91zfyz4C2jU8WFDH50V0mnEVGLcZKGhaVm6XQ_L8Q@mail.gmail.com>
On 2025/09/05 0:02, Paul Moore wrote:
> On Thu, Sep 4, 2025 at 5:53 AM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>> On 2025/09/04 5:32, Paul Moore wrote:
>>> On Thu, Aug 14, 2025 at 6:54 PM Paul Moore <paul@paul-moore.com> wrote:
>>>>
>>>> Reviewed-by: Kees Cook <kees@kernel.org>
>>>> Signed-off-by: Paul Moore <paul@paul-moore.com>
>>>> ---
>>>> security/tomoyo/common.h | 2 ++
>>>> security/tomoyo/securityfs_if.c | 4 +---
>>>> security/tomoyo/tomoyo.c | 1 +
>>>> 3 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>> Tetsuo, does this look okay to you?
>>>
>>
>> Yes.
>
> Thanks for reviewing, may I add your ACK?
>
Yes.
^ permalink raw reply
* Re: [PATCH v3 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Tetsuo Handa @ 2025-09-07 7:35 UTC (permalink / raw)
To: Paul Moore, John Johansen
Cc: Roberto Sassu, linux-security-module, linux-integrity, selinux,
Mimi Zohar, Roberto Sassu, Fan Wu, Mickaël Salaün,
Günther Noack, Kees Cook, Micah Morton, Casey Schaufler,
Nicolas Bouchinet, Xiu Jianfeng
In-Reply-To: <CAHC9VhRjQrjvsn65A-TGKKGrVFjZdnPBu+1vp=7w86SOjoyiUw@mail.gmail.com>
On 2025/09/05 2:52, Paul Moore wrote:
> + if (unlikely(!str)) {
> + char *str_tmp;
> + size_t len_tmp = 0;
> +
Wants a comment that lsm_active_cnt > 0 is guaranteed, or someone
(maybe static analyzers) thinks that we hit ZERO_SIZE_PTR pointer
dereference when lsm_active_cnt == 0.
> + for (i = 0; i < lsm_active_cnt; i++)
> + /* the '+ 1' accounts for either a comma or a NUL */
> + len_tmp += strlen(lsm_idlist[i]->name) + 1;
> +
> + str_tmp = kmalloc(len_tmp, GFP_KERNEL);
> + if (!str_tmp)
> + return -ENOMEM;
> + str_tmp[0] = '\0';
> +
> + for (i = 0; i < lsm_active_cnt; i++) {
> + if (i > 0)
> + strcat(str_tmp, ",");
> + strcat(str_tmp, lsm_idlist[i]->name);
> + }
> +
> + spin_lock(&lock);
> + if (!str) {
> + str = str_tmp;
> + len = len_tmp - 1;
This needs to be
len = len_tmp - 1;
mb();
str = str_tmp;
, or concurrent access might reach simple_read_from_buffer()
with str != 0 and len == 0. (If you don't want mb(), you can use
- if (unlikely(!str)) {
+ if (unlikely(!str || !len)) {
instead).
> + } else
> + kfree(str_tmp);
> + spin_unlock(&lock);
> + }
> +
> + return simple_read_from_buffer(buf, count, ppos, str, len);
> }
^ 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