* Re: [PATCH] pid: use ns_capable_noaudit() when determining net sysctl permissions
From: Paul Moore @ 2025-09-11 20:46 UTC (permalink / raw)
To: Christian Göttsche, linux-kernel
Cc: Christian Göttsche, Christian Brauner, linux-security-module,
selinux
In-Reply-To: <20250910192605.16431-1-cgoettsche@seltendoof.de>
On Sep 10, 2025 =?UTF-8?q?Christian=20G=C3=B6ttsche?= <cgoettsche@seltendoof.de> wrote:
>
> The capability check should not be audited since it is only being used
> to determine the inode permissions. A failed check does not indicate a
> violation of security policy but, when an LSM is enabled, a denial audit
> message was being generated.
>
> The denial audit message can either lead to the capability being
> unnecessarily allowed in a security policy, or being silenced potentially
> masking a legitimate capability check at a later point in time.
>
> Similar to commit d6169b0206db ("net: Use ns_capable_noaudit() when
> determining net sysctl permissions")
>
> Fixes: 7863dcc72d0f ("pid: allow pid_max to be set per pid namespace")
> CC: Christian Brauner <brauner@kernel.org>
> CC: linux-security-module@vger.kernel.org
> CC: selinux@vger.kernel.org
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
> kernel/pid.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] security: keys: use menuconfig for KEYS symbol
From: Paul Moore @ 2025-09-11 20:34 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Randy Dunlap, linux-kernel, David Howells, keyrings,
linux-security-module, James Morris, Serge E. Hallyn
In-Reply-To: <aKzot67f7F3wtHs7@kernel.org>
On Mon, Aug 25, 2025 at 6:50 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> On Sun, Aug 24, 2025 at 03:28:13PM -0700, Randy Dunlap wrote:
> > Give the KEYS kconfig symbol and its associated symbols a separate
> > menu space under Security options by using "menuconfig" instead of
> > "config".
> >
> > This also makes it easier to find the security and LSM options.
> >
> > Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> > ---
> > Cc: David Howells <dhowells@redhat.com>
> > Cc: Jarkko Sakkinen <jarkko@kernel.org>
> > Cc: keyrings@vger.kernel.org
> > Cc: linux-security-module@vger.kernel.org
> > Cc: Paul Moore <paul@paul-moore.com>
> > Cc: James Morris <jmorris@namei.org>
> > Cc: "Serge E. Hallyn" <serge@hallyn.com>
> >
> > security/keys/Kconfig | 14 ++++++--------
> > 1 file changed, 6 insertions(+), 8 deletions(-)
...
> I wote for this at least. Definitely an improvement:
>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
I'm guessing you're planning to take this patch Jarkko?
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] security: CONFIG_LSM: LSM can depend on SECURITY
From: Paul Moore @ 2025-09-11 20:32 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel
Cc: Randy Dunlap, Kees Cook, James Morris, Serge E. Hallyn,
linux-security-module
In-Reply-To: <20250824222800.92263-1-rdunlap@infradead.org>
On Aug 24, 2025 Randy Dunlap <rdunlap@infradead.org> wrote:
>
> When CONFIG_SECURITY is not set, CONFIG_LSM (builtin_lsm_order) does
> not need to be visible and settable since builtin_lsm_order is defined in
> security.o, which is only built when CONFIG_SECURITY=y.
>
> So make CONFIG_LSM depend on CONFIG_SECURITY.
>
> Fixes: 13e735c0e953 ("LSM: Introduce CONFIG_LSM")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
> Cc: Kees Cook <kees@kernel.org>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: linux-security-module@vger.kernel.org
>
> security/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
Merged into lsm/dev, thanks!
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v3 31/34] ima,evm: move initcalls to the LSM framework
From: Paul Moore @ 2025-09-11 19:30 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: <9f1dd6d30193c82ff36b5665eadc1aec73736017.camel@linux.ibm.com>
On Mon, Sep 8, 2025 at 6:34 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Sun, 2025-09-07 at 21:05 -0400, Paul Moore wrote:
> > > 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.
>
> Thanks, Paul. I was able to apply the patches and run some regression tests.
Mimi, I know you already tagged Roberto's patch with a 'Reviewed-by'
tag, but I wanted to follow up and see if you were comfortable
converting that into an ACK, or if you wanted more time to review
Roberto's patch? No wrong answers, just trying to understand where
you are at with this patch.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCHES] simple part of ->d_name stuff
From: Linus Torvalds @ 2025-09-11 16:56 UTC (permalink / raw)
To: Al Viro
Cc: linux-fsdevel, Christian Brauner, Jan Kara, NeilBrown,
linux-security-module, David Howells, Namjae Jeon
In-Reply-To: <20250911050149.GW31600@ZenIV>
On Wed, 10 Sept 2025 at 22:01, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> So let's make sure that all functions we are passing
> &dentry->d_name are taking const struct qstr * and replace
> ->d_name with an anon union of struct qstr *__d_name and
> const struct qstr *d_name.
Ack, all these patches look ObviouslyCorrect(tm) to me, and seems a
good improvement.
Even if it won't catch the more involved cases that worry you more.
Linus
^ permalink raw reply
* Re: [PATCH 1/6] security_dentry_init_security(): constify qstr argument
From: Casey Schaufler @ 2025-09-11 14:30 UTC (permalink / raw)
To: Al Viro, linux-fsdevel
Cc: torvalds, brauner, jack, neil, linux-security-module, dhowells,
linkinjeon, Casey Schaufler
In-Reply-To: <20250911050534.3116491-1-viro@zeniv.linux.org.uk>
On 9/10/2025 10:05 PM, Al Viro wrote:
> Nothing outside of fs/dcache.c has any business modifying
> dentry names; passing &dentry->d_name as an argument should
> have that argument declared as a const pointer.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
For the Smack bit:
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> include/linux/lsm_hook_defs.h | 2 +-
> include/linux/security.h | 4 ++--
> security/security.c | 2 +-
> security/selinux/hooks.c | 2 +-
> security/smack/smack_lsm.c | 2 +-
> 5 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index fd11fffdd3c3..aa4d6ec9c98b 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -85,7 +85,7 @@ LSM_HOOK(int, -EOPNOTSUPP, dentry_init_security, struct dentry *dentry,
> int mode, const struct qstr *name, const char **xattr_name,
> struct lsm_context *cp)
> LSM_HOOK(int, 0, dentry_create_files_as, struct dentry *dentry, int mode,
> - struct qstr *name, const struct cred *old, struct cred *new)
> + const struct qstr *name, const struct cred *old, struct cred *new)
>
> #ifdef CONFIG_SECURITY_PATH
> LSM_HOOK(int, 0, path_unlink, const struct path *dir, struct dentry *dentry)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 521bcb5b9717..3f694d3ebd70 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -391,7 +391,7 @@ int security_dentry_init_security(struct dentry *dentry, int mode,
> const char **xattr_name,
> struct lsm_context *lsmcxt);
> int security_dentry_create_files_as(struct dentry *dentry, int mode,
> - struct qstr *name,
> + const struct qstr *name,
> const struct cred *old,
> struct cred *new);
> int security_path_notify(const struct path *path, u64 mask,
> @@ -871,7 +871,7 @@ static inline int security_dentry_init_security(struct dentry *dentry,
> }
>
> static inline int security_dentry_create_files_as(struct dentry *dentry,
> - int mode, struct qstr *name,
> + int mode, const struct qstr *name,
> const struct cred *old,
> struct cred *new)
> {
> diff --git a/security/security.c b/security/security.c
> index ad163f06bf7a..db2d75be87cc 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1775,7 +1775,7 @@ EXPORT_SYMBOL(security_dentry_init_security);
> * Return: Returns 0 on success, error on failure.
> */
> int security_dentry_create_files_as(struct dentry *dentry, int mode,
> - struct qstr *name,
> + const struct qstr *name,
> const struct cred *old, struct cred *new)
> {
> return call_int_hook(dentry_create_files_as, dentry, mode,
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index c95a5874bf7d..58ce49954206 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2901,7 +2901,7 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
> }
>
> static int selinux_dentry_create_files_as(struct dentry *dentry, int mode,
> - struct qstr *name,
> + const struct qstr *name,
> const struct cred *old,
> struct cred *new)
> {
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index fc340a6f0dde..5caa372ffbf3 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -4908,7 +4908,7 @@ static int smack_inode_copy_up_xattr(struct dentry *src, const char *name)
> }
>
> static int smack_dentry_create_files_as(struct dentry *dentry, int mode,
> - struct qstr *name,
> + const struct qstr *name,
> const struct cred *old,
> struct cred *new)
> {
^ permalink raw reply
* Re: [RFC PATCH] lsm,selinux: introduce LSM_ATTR_UNSHARE and wire it up for SELinux
From: Stephen Smalley @ 2025-09-11 14:00 UTC (permalink / raw)
To: selinux, linux-security-module
Cc: paul, omosnace, john.johansen, serge, casey
In-Reply-To: <CAEjxPJ4ctFLY=_AUaNso_a4XJ8AziRoebR2ExFss_s3qNnYhJA@mail.gmail.com>
On Thu, Sep 4, 2025 at 10:41 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Wed, Sep 3, 2025 at 3:28 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > In the hopes of nudging the conversation in [1] in a more focused
> > direction with a goal of getting SELinux namespaces upstreamed, this
> > RFC provides a starting point for a concrete discussion. This is RFC
> > only and has only been build-tested thus far.
> >
> > This defines a new LSM_ATTR_UNSHARE attribute for the
> > lsm_set_self_attr(2) system call and wires it up for SELinux to invoke
> > the underlying function for unsharing the SELinux namespace. As with
> > the selinuxfs interface, this immediately unshares the SELinux
> > namespace of the current process just like an unshare(2) system call
> > would do for other namespaces. I have not yet explored the
> > alternatives of deferring the unshare to the next unshare(2),
> > clone(2), or execve(2) call and would want to first confirm that doing
> > so does not introduce any issues in the kernel or make it harder to
> > integrate with existing container runtimes.
> >
> > Differences between this syscall interface and the selinuxfs interface
> > that need discussion before moving forward:
> >
> > 1. The syscall interface does not currently check any Linux capability
> > or DAC permissions, whereas the selinuxfs interface can only be set by
> > uid-0 or CAP_DAC_OVERRIDE processes. We need to decide what if any
> > capability or DAC check should apply to this syscall interface and if
> > any, add the checks to either the LSM framework code or to the SELinux
> > hook function.
> >
> > Pros: Checking a capability or DAC permissions prevents misuse of this
> > interface by unprivileged processes, particularly on systems with
> > policies that do not yet define any of the new SELinux permissions
> > introduced for controlling this operation. This is a potential concern
> > on Linux distributions that do not tightly coordinate kernel updates
> > with policy updates (or where users may choose to deploy upstream
> > kernels on their own), but not on Android.
> >
> > Cons: Checking a capability or DAC permissions requires any process
> > that uses this facility to have the corresponding capability or
> > permissions, which might otherwise be unnecessary and create
> > additional risks. This is less likely if we use a capability already
> > required by container runtimes and similar components that might
> > leverage this facility for unsharing SELinux namespaces.
> >
> > 2. The syscall interface checks a new SELinux unshare_selinuxns
> > permission in the process2 class between the task SID and itself,
> > similar to other checks for setting process attributes. This means
> > that:
> > allow domain self:process2 *; -or-
> > allow domain self:process2 ~anything-other-than-unshare_selinuxns; -or-
> > allow domain self:process2 unshare_selinuxns;
> > would allow a process to unshare its SELinux namespace.
> >
> > The selinuxfs interface checks a new unshare permission in the
> > security class between the task SID and the security initial SID,
> > likewise similar to other checks for setting selinuxfs attributes.
> > This means that:
> > allow domain security_t:security *; -or-
> > allow domain security_t:security ~anything-other-than-unshare; -or-
> > allow domain security_t:security unshare;
> > would allow a process to unshare its SELinux namespace.
> >
> > Technically, the selinuxfs interface also currently requires open and
> > write access to the selinuxfs node; hence:
> > allow domain security_t:file { open write };
> > is also required for the selinuxfs interface.
> >
> > We need to decide what we want the SELinux check(s) to be for the
> > syscall and whether it should be more like the former (process
> > attributes) or more like the latter (security policy settings). Note
> > that the permission name itself is unimportant here and only differs
> > because it seemed less evident in the process2 class that we are
> > talking about a SELinux namespace otherwise.
> >
> > Regardless, either form of allow rule can be prohibited in policies
> > via neverallow rules on systems that enforce their usage
> > (e.g. Android, not necessarily on Linux distributions).
> >
> > 3. The selinuxfs interface currently offers more functionality than I
> > have implemented here for the sycall interface, including:
> >
> > a) the ability to read the selinuxfs node to see if your namespace has
> > been unshared, which should be easily implementable via
> > lsm_get_self_attr(2). However, questions remain as to when that
> > should return 1 versus 0 (currently returns 1 whenever your namespace
> > is NOT the initial SELinux namespace, useful for the testsuite to
> > detect it is in a child, but could instead be reset to 0 by a
> > subsequent policy load to indicate completion of the setup of the
> > namespace, thus hiding from child processes that they are in a child
> > namespace once its policy has been loaded).
> >
> > b) the abilities to get and set the maximum number of SELinux
> > namespaces (via a /sys/fs/selinux/maxns node) and to get and set the
> > maximum depth for SELinux namespaces (via a /sys/fs/selinux/maxnsdepth
> > node). These could be left in selinuxfs or migrated to some other LSM
> > management APIs since they are global in scope, not per-process
> > attributes.
> >
> > Signed-off-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Gentle ping on this RFC, happy to update and re-submit whenever.
> > ---
> > include/uapi/linux/lsm.h | 1 +
> > security/selinux/hooks.c | 8 ++++++++
> > security/selinux/include/classmap.h | 4 +++-
> > 3 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
> > index 938593dfd5da..fb1b4a8aa639 100644
> > --- a/include/uapi/linux/lsm.h
> > +++ b/include/uapi/linux/lsm.h
> > @@ -83,6 +83,7 @@ struct lsm_ctx {
> > #define LSM_ATTR_KEYCREATE 103
> > #define LSM_ATTR_PREV 104
> > #define LSM_ATTR_SOCKCREATE 105
> > +#define LSM_ATTR_UNSHARE 106
> >
> > /*
> > * LSM_FLAG_XXX definitions identify special handling instructions
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index c544b3e2fd5c..11b0b3c5b74a 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -6812,6 +6812,10 @@ static int selinux_lsm_setattr(u64 attr, void *value, size_t size)
> > error = avc_has_perm(state, mysid, mysid, SECCLASS_PROCESS,
> > PROCESS__SETCURRENT, NULL);
> > break;
> > + case LSM_ATTR_UNSHARE:
> > + error = avc_has_perm(state, mysid, mysid, SECCLASS_PROCESS,
>
> Ignore the obvious typo here (should be SECCLASS_PROCESS2 above). Will
> be fixed before submission. Otherwise, the RFC remains the same.
>
> > + PROCESS2__UNSHARE_SELINUXNS, NULL);
> > + break;
> > default:
> > error = -EOPNOTSUPP;
> > break;
> > @@ -6923,6 +6927,10 @@ static int selinux_lsm_setattr(u64 attr, void *value, size_t size)
> > }
> >
> > tsec->sid = sid;
> > + } else if (attr == LSM_ATTR_UNSHARE) {
> > + error = selinux_state_create(new);
> > + if (error)
> > + goto abort_change;
> > } else {
> > error = -EINVAL;
> > goto abort_change;
> > diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> > index be52ebb6b94a..07fe316308cd 100644
> > --- a/security/selinux/include/classmap.h
> > +++ b/security/selinux/include/classmap.h
> > @@ -60,7 +60,9 @@ const struct security_class_mapping secclass_map[] = {
> > "siginh", "setrlimit", "rlimitinh", "dyntransition",
> > "setcurrent", "execmem", "execstack", "execheap",
> > "setkeycreate", "setsockcreate", "getrlimit", NULL } },
> > - { "process2", { "nnp_transition", "nosuid_transition", NULL } },
> > + { "process2",
> > + { "nnp_transition", "nosuid_transition", "unshare_selinuxns",
> > + NULL } },
> > { "system",
> > { "ipc_info", "syslog_read", "syslog_mod", "syslog_console",
> > "module_request", "module_load", "firmware_load",
> > --
> > 2.51.0
> >
^ permalink raw reply
* Re: [PATCH net-next v2 0/8] ipv4: icmp: Fix source IP derivation in presence of VRFs
From: patchwork-bot+netdevbpf @ 2025-09-11 10:40 UTC (permalink / raw)
To: Ido Schimmel
Cc: netdev, davem, kuba, pabeni, edumazet, horms, paul, dsahern,
petrm, linux-security-module
In-Reply-To: <20250908073238.119240-1-idosch@nvidia.com>
Hello:
This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Mon, 8 Sep 2025 10:32:30 +0300 you wrote:
> 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.
>
> [...]
Here is the summary with links:
- [net-next,v2,1/8] ipv4: cipso: Simplify IP options handling in cipso_v4_error()
https://git.kernel.org/netdev/net-next/c/cda276bcb9a5
- [net-next,v2,2/8] ipv4: icmp: Pass IPv4 control block structure as an argument to __icmp_send()
https://git.kernel.org/netdev/net-next/c/0d3c4a441686
- [net-next,v2,3/8] ipv4: icmp: Fix source IP derivation in presence of VRFs
https://git.kernel.org/netdev/net-next/c/4a8c416602d9
- [net-next,v2,4/8] selftests: traceroute: Return correct value on failure
https://git.kernel.org/netdev/net-next/c/c068ba9d3ded
- [net-next,v2,5/8] selftests: traceroute: Use require_command()
https://git.kernel.org/netdev/net-next/c/47efbac9b768
- [net-next,v2,6/8] selftests: traceroute: Reword comment
https://git.kernel.org/netdev/net-next/c/5c9c78224fc3
- [net-next,v2,7/8] selftests: traceroute: Test traceroute with different source IPs
https://git.kernel.org/netdev/net-next/c/2e6428100b16
- [net-next,v2,8/8] selftests: traceroute: Add VRF tests
https://git.kernel.org/netdev/net-next/c/f7240999deb4
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH 2/6] exfat_find(): constify qstr argument
From: Namjae Jeon @ 2025-09-11 6:02 UTC (permalink / raw)
To: Al Viro
Cc: linux-fsdevel, torvalds, brauner, jack, neil,
linux-security-module, dhowells
In-Reply-To: <20250911050534.3116491-2-viro@zeniv.linux.org.uk>
On Thu, Sep 11, 2025 at 2:05 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Nothing outside of fs/dcache.c has any business modifying
> dentry names; passing &dentry->d_name as an argument should
> have that argument declared as a const pointer.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Thanks!
^ permalink raw reply
* Re: [PATCHES] simple part of ->d_name stuff
From: David Howells @ 2025-09-11 5:59 UTC (permalink / raw)
To: Al Viro
Cc: dhowells, linux-fsdevel, Linus Torvalds, Christian Brauner,
Jan Kara, NeilBrown, linux-security-module, Namjae Jeon
In-Reply-To: <20250911050149.GW31600@ZenIV>
Al Viro <viro@zeniv.linux.org.uk> wrote:
> Rules for ->d_name access are rather unpleasant and so's
> verifying that they are followed. There is a relatively simple part,
> though - nobody outside of fs/dcache.c has any business modifying
> that thing.
>
> So let's make sure that all functions we are passing
> &dentry->d_name are taking const struct qstr * and replace
> ->d_name with an anon union of struct qstr *__d_name and
> const struct qstr *d_name.
>
> It is *not* enough to guarantee that another thread will
> not call __d_move() right under you - checking the requirements
> for that is the hard part. It does make it easy to verify that
> nothing else accidentally starts changing it.
>
> This stuff lives in
> git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.qstr
> Branch is -rc4-based, individual patches in followups.
>
> Please, review. If nobody objects, I'll put that in #for-next.
Reviewed-by: David Howells <dhowells@redhat.com>
^ permalink raw reply
* [PATCH 5/6] generic_ci_validate_strict_name(): constify name argument
From: Al Viro @ 2025-09-11 5:05 UTC (permalink / raw)
To: linux-fsdevel
Cc: torvalds, brauner, jack, neil, linux-security-module, dhowells,
linkinjeon
In-Reply-To: <20250911050534.3116491-1-viro@zeniv.linux.org.uk>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
include/linux/fs.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index d7ab4f96d705..6dcfc1c399ca 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3719,7 +3719,8 @@ int generic_ci_d_compare(const struct dentry *dentry, unsigned int len,
* happens when a directory is casefolded and the filesystem is strict
* about its encoding.
*/
-static inline bool generic_ci_validate_strict_name(struct inode *dir, struct qstr *name)
+static inline bool generic_ci_validate_strict_name(struct inode *dir,
+ const struct qstr *name)
{
if (!IS_CASEFOLDED(dir) || !sb_has_strict_encoding(dir->i_sb))
return true;
@@ -3734,7 +3735,8 @@ static inline bool generic_ci_validate_strict_name(struct inode *dir, struct qst
return !utf8_validate(dir->i_sb->s_encoding, name);
}
#else
-static inline bool generic_ci_validate_strict_name(struct inode *dir, struct qstr *name)
+static inline bool generic_ci_validate_strict_name(struct inode *dir,
+ const struct qstr *name)
{
return true;
}
--
2.47.2
^ permalink raw reply related
* [PATCH 2/6] exfat_find(): constify qstr argument
From: Al Viro @ 2025-09-11 5:05 UTC (permalink / raw)
To: linux-fsdevel
Cc: torvalds, brauner, jack, neil, linux-security-module, dhowells,
linkinjeon
In-Reply-To: <20250911050534.3116491-1-viro@zeniv.linux.org.uk>
Nothing outside of fs/dcache.c has any business modifying
dentry names; passing &dentry->d_name as an argument should
have that argument declared as a const pointer.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/exfat/namei.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
index f5f1c4e8a29f..c8388a2ec227 100644
--- a/fs/exfat/namei.c
+++ b/fs/exfat/namei.c
@@ -587,7 +587,7 @@ static int exfat_create(struct mnt_idmap *idmap, struct inode *dir,
}
/* lookup a file */
-static int exfat_find(struct inode *dir, struct qstr *qname,
+static int exfat_find(struct inode *dir, const struct qstr *qname,
struct exfat_dir_entry *info)
{
int ret, dentry, count;
--
2.47.2
^ permalink raw reply related
* [PATCH 6/6] make it easier to catch those who try to modify ->d_name
From: Al Viro @ 2025-09-11 5:05 UTC (permalink / raw)
To: linux-fsdevel
Cc: torvalds, brauner, jack, neil, linux-security-module, dhowells,
linkinjeon
In-Reply-To: <20250911050534.3116491-1-viro@zeniv.linux.org.uk>
Turn d_name into an anon union of const struct qstr d_name with
struct qstr __d_name. Very few places need to modify it (all
in fs/dcache.c); those are switched to use of ->__d_name.
Note that ->d_name can actually change under you unless you have
the right locking environment; this const just prohibits accidentally
doing stores without being easily spotted.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/dcache.c | 26 +++++++++++++-------------
include/linux/dcache.h | 5 ++++-
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index 60046ae23d51..b4cd5e1321b3 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1717,13 +1717,13 @@ static struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
dname = dentry->d_shortname.string;
}
- dentry->d_name.len = name->len;
- dentry->d_name.hash = name->hash;
+ dentry->__d_name.len = name->len;
+ dentry->__d_name.hash = name->hash;
memcpy(dname, name->name, name->len);
dname[name->len] = 0;
/* Make sure we always see the terminating NUL character */
- smp_store_release(&dentry->d_name.name, dname); /* ^^^ */
+ smp_store_release(&dentry->__d_name.name, dname); /* ^^^ */
dentry->d_flags = 0;
lockref_init(&dentry->d_lockref);
@@ -2743,15 +2743,15 @@ static void swap_names(struct dentry *dentry, struct dentry *target)
/*
* Both external: swap the pointers
*/
- swap(target->d_name.name, dentry->d_name.name);
+ swap(target->__d_name.name, dentry->__d_name.name);
} else {
/*
* dentry:internal, target:external. Steal target's
* storage and make target internal.
*/
- dentry->d_name.name = target->d_name.name;
+ dentry->__d_name.name = target->__d_name.name;
target->d_shortname = dentry->d_shortname;
- target->d_name.name = target->d_shortname.string;
+ target->__d_name.name = target->d_shortname.string;
}
} else {
if (unlikely(dname_external(dentry))) {
@@ -2759,9 +2759,9 @@ static void swap_names(struct dentry *dentry, struct dentry *target)
* dentry:external, target:internal. Give dentry's
* storage to target and make dentry internal
*/
- target->d_name.name = dentry->d_name.name;
+ target->__d_name.name = dentry->__d_name.name;
dentry->d_shortname = target->d_shortname;
- dentry->d_name.name = dentry->d_shortname.string;
+ dentry->__d_name.name = dentry->d_shortname.string;
} else {
/*
* Both are internal.
@@ -2771,7 +2771,7 @@ static void swap_names(struct dentry *dentry, struct dentry *target)
target->d_shortname.words[i]);
}
}
- swap(dentry->d_name.hash_len, target->d_name.hash_len);
+ swap(dentry->__d_name.hash_len, target->__d_name.hash_len);
}
static void copy_name(struct dentry *dentry, struct dentry *target)
@@ -2781,11 +2781,11 @@ static void copy_name(struct dentry *dentry, struct dentry *target)
old_name = external_name(dentry);
if (unlikely(dname_external(target))) {
atomic_inc(&external_name(target)->count);
- dentry->d_name = target->d_name;
+ dentry->__d_name = target->__d_name;
} else {
dentry->d_shortname = target->d_shortname;
- dentry->d_name.name = dentry->d_shortname.string;
- dentry->d_name.hash_len = target->d_name.hash_len;
+ dentry->__d_name.name = dentry->d_shortname.string;
+ dentry->__d_name.hash_len = target->__d_name.hash_len;
}
if (old_name && likely(atomic_dec_and_test(&old_name->count)))
kfree_rcu(old_name, head);
@@ -3133,7 +3133,7 @@ void d_mark_tmpfile(struct file *file, struct inode *inode)
!d_unlinked(dentry));
spin_lock(&dentry->d_parent->d_lock);
spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
- dentry->d_name.len = sprintf(dentry->d_shortname.string, "#%llu",
+ dentry->__d_name.len = sprintf(dentry->d_shortname.string, "#%llu",
(unsigned long long)inode->i_ino);
spin_unlock(&dentry->d_lock);
spin_unlock(&dentry->d_parent->d_lock);
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index cc3e1c1a3454..c83e02b94389 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -95,7 +95,10 @@ struct dentry {
seqcount_spinlock_t d_seq; /* per dentry seqlock */
struct hlist_bl_node d_hash; /* lookup hash list */
struct dentry *d_parent; /* parent directory */
- struct qstr d_name;
+ union {
+ struct qstr __d_name; /* for use ONLY in fs/dcache.c */
+ const struct qstr d_name;
+ };
struct inode *d_inode; /* Where the name belongs to - NULL is
* negative */
union shortname_store d_shortname;
--
2.47.2
^ permalink raw reply related
* [PATCH 4/6] afs_dir_search: constify qstr argument
From: Al Viro @ 2025-09-11 5:05 UTC (permalink / raw)
To: linux-fsdevel
Cc: torvalds, brauner, jack, neil, linux-security-module, dhowells,
linkinjeon
In-Reply-To: <20250911050534.3116491-1-viro@zeniv.linux.org.uk>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/afs/dir_search.c | 2 +-
fs/afs/internal.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/afs/dir_search.c b/fs/afs/dir_search.c
index b25bd892db4d..d2516e55b5ed 100644
--- a/fs/afs/dir_search.c
+++ b/fs/afs/dir_search.c
@@ -188,7 +188,7 @@ int afs_dir_search_bucket(struct afs_dir_iter *iter, const struct qstr *name,
/*
* Search the appropriate hash chain in the contents of an AFS directory.
*/
-int afs_dir_search(struct afs_vnode *dvnode, struct qstr *name,
+int afs_dir_search(struct afs_vnode *dvnode, const struct qstr *name,
struct afs_fid *_fid, afs_dataversion_t *_dir_version)
{
struct afs_dir_iter iter = { .dvnode = dvnode, };
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index db14882d367b..1ce5deaf6019 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -1112,7 +1112,7 @@ bool afs_dir_init_iter(struct afs_dir_iter *iter, const struct qstr *name);
union afs_xdr_dir_block *afs_dir_find_block(struct afs_dir_iter *iter, size_t block);
int afs_dir_search_bucket(struct afs_dir_iter *iter, const struct qstr *name,
struct afs_fid *_fid);
-int afs_dir_search(struct afs_vnode *dvnode, struct qstr *name,
+int afs_dir_search(struct afs_vnode *dvnode, const struct qstr *name,
struct afs_fid *_fid, afs_dataversion_t *_dir_version);
/*
--
2.47.2
^ permalink raw reply related
* [PATCH 3/6] afs_edit_dir_{add,remove}(): constify qstr argument
From: Al Viro @ 2025-09-11 5:05 UTC (permalink / raw)
To: linux-fsdevel
Cc: torvalds, brauner, jack, neil, linux-security-module, dhowells,
linkinjeon
In-Reply-To: <20250911050534.3116491-1-viro@zeniv.linux.org.uk>
Nothing outside of fs/dcache.c has any business modifying
dentry names; passing &dentry->d_name as an argument should
have that argument declared as a const pointer.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/afs/dir_edit.c | 4 ++--
fs/afs/internal.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/afs/dir_edit.c b/fs/afs/dir_edit.c
index 60a549f1d9c5..60b0f70f63a3 100644
--- a/fs/afs/dir_edit.c
+++ b/fs/afs/dir_edit.c
@@ -239,7 +239,7 @@ static void afs_edit_init_block(union afs_xdr_dir_block *meta,
* The caller must hold the inode locked.
*/
void afs_edit_dir_add(struct afs_vnode *vnode,
- struct qstr *name, struct afs_fid *new_fid,
+ const struct qstr *name, struct afs_fid *new_fid,
enum afs_edit_dir_reason why)
{
union afs_xdr_dir_block *meta, *block;
@@ -391,7 +391,7 @@ void afs_edit_dir_add(struct afs_vnode *vnode,
* The caller must hold the inode locked.
*/
void afs_edit_dir_remove(struct afs_vnode *vnode,
- struct qstr *name, enum afs_edit_dir_reason why)
+ const struct qstr *name, enum afs_edit_dir_reason why)
{
union afs_xdr_dir_block *meta, *block, *pblock;
union afs_xdr_dirent *de, *pde;
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 1124ea4000cb..db14882d367b 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -1097,9 +1097,9 @@ int afs_single_writepages(struct address_space *mapping,
/*
* dir_edit.c
*/
-extern void afs_edit_dir_add(struct afs_vnode *, struct qstr *, struct afs_fid *,
+extern void afs_edit_dir_add(struct afs_vnode *, const struct qstr *, struct afs_fid *,
enum afs_edit_dir_reason);
-extern void afs_edit_dir_remove(struct afs_vnode *, struct qstr *, enum afs_edit_dir_reason);
+extern void afs_edit_dir_remove(struct afs_vnode *, const struct qstr *, enum afs_edit_dir_reason);
void afs_edit_dir_update_dotdot(struct afs_vnode *vnode, struct afs_vnode *new_dvnode,
enum afs_edit_dir_reason why);
void afs_mkdir_init_dir(struct afs_vnode *dvnode, struct afs_vnode *parent_vnode);
--
2.47.2
^ permalink raw reply related
* [PATCH 1/6] security_dentry_init_security(): constify qstr argument
From: Al Viro @ 2025-09-11 5:05 UTC (permalink / raw)
To: linux-fsdevel
Cc: torvalds, brauner, jack, neil, linux-security-module, dhowells,
linkinjeon
In-Reply-To: <20250911050149.GW31600@ZenIV>
Nothing outside of fs/dcache.c has any business modifying
dentry names; passing &dentry->d_name as an argument should
have that argument declared as a const pointer.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
include/linux/lsm_hook_defs.h | 2 +-
include/linux/security.h | 4 ++--
security/security.c | 2 +-
security/selinux/hooks.c | 2 +-
security/smack/smack_lsm.c | 2 +-
5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index fd11fffdd3c3..aa4d6ec9c98b 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -85,7 +85,7 @@ LSM_HOOK(int, -EOPNOTSUPP, dentry_init_security, struct dentry *dentry,
int mode, const struct qstr *name, const char **xattr_name,
struct lsm_context *cp)
LSM_HOOK(int, 0, dentry_create_files_as, struct dentry *dentry, int mode,
- struct qstr *name, const struct cred *old, struct cred *new)
+ const struct qstr *name, const struct cred *old, struct cred *new)
#ifdef CONFIG_SECURITY_PATH
LSM_HOOK(int, 0, path_unlink, const struct path *dir, struct dentry *dentry)
diff --git a/include/linux/security.h b/include/linux/security.h
index 521bcb5b9717..3f694d3ebd70 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -391,7 +391,7 @@ int security_dentry_init_security(struct dentry *dentry, int mode,
const char **xattr_name,
struct lsm_context *lsmcxt);
int security_dentry_create_files_as(struct dentry *dentry, int mode,
- struct qstr *name,
+ const struct qstr *name,
const struct cred *old,
struct cred *new);
int security_path_notify(const struct path *path, u64 mask,
@@ -871,7 +871,7 @@ static inline int security_dentry_init_security(struct dentry *dentry,
}
static inline int security_dentry_create_files_as(struct dentry *dentry,
- int mode, struct qstr *name,
+ int mode, const struct qstr *name,
const struct cred *old,
struct cred *new)
{
diff --git a/security/security.c b/security/security.c
index ad163f06bf7a..db2d75be87cc 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1775,7 +1775,7 @@ EXPORT_SYMBOL(security_dentry_init_security);
* Return: Returns 0 on success, error on failure.
*/
int security_dentry_create_files_as(struct dentry *dentry, int mode,
- struct qstr *name,
+ const struct qstr *name,
const struct cred *old, struct cred *new)
{
return call_int_hook(dentry_create_files_as, dentry, mode,
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index c95a5874bf7d..58ce49954206 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2901,7 +2901,7 @@ static int selinux_dentry_init_security(struct dentry *dentry, int mode,
}
static int selinux_dentry_create_files_as(struct dentry *dentry, int mode,
- struct qstr *name,
+ const struct qstr *name,
const struct cred *old,
struct cred *new)
{
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index fc340a6f0dde..5caa372ffbf3 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4908,7 +4908,7 @@ static int smack_inode_copy_up_xattr(struct dentry *src, const char *name)
}
static int smack_dentry_create_files_as(struct dentry *dentry, int mode,
- struct qstr *name,
+ const struct qstr *name,
const struct cred *old,
struct cred *new)
{
--
2.47.2
^ permalink raw reply related
* [PATCHES] simple part of ->d_name stuff
From: Al Viro @ 2025-09-11 5:01 UTC (permalink / raw)
To: linux-fsdevel
Cc: Linus Torvalds, Christian Brauner, Jan Kara, NeilBrown,
linux-security-module, David Howells, Namjae Jeon
Rules for ->d_name access are rather unpleasant and so's
verifying that they are followed. There is a relatively simple part,
though - nobody outside of fs/dcache.c has any business modifying
that thing.
So let's make sure that all functions we are passing
&dentry->d_name are taking const struct qstr * and replace
->d_name with an anon union of struct qstr *__d_name and
const struct qstr *d_name.
It is *not* enough to guarantee that another thread will
not call __d_move() right under you - checking the requirements
for that is the hard part. It does make it easy to verify that
nothing else accidentally starts changing it.
This stuff lives in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.qstr
Branch is -rc4-based, individual patches in followups.
Please, review. If nobody objects, I'll put that in #for-next.
Shortlog:
security_dentry_init_security(): constify qstr argument
exfat_find(): constify qstr argument
afs_edit_dir_{add,remove}(): constify qstr argument
afs_dir_search: constify qstr argument
generic_ci_validate_strict_name(): constify name argument
make it easier to catch those who try to modify ->d_name
Diffstat:
fs/afs/dir_edit.c | 4 ++--
fs/afs/dir_search.c | 2 +-
fs/afs/internal.h | 6 +++---
fs/dcache.c | 26 +++++++++++++-------------
fs/exfat/namei.c | 2 +-
include/linux/dcache.h | 5 ++++-
include/linux/fs.h | 6 ++++--
include/linux/lsm_hook_defs.h | 2 +-
include/linux/security.h | 4 ++--
security/security.c | 2 +-
security/selinux/hooks.c | 2 +-
security/smack/smack_lsm.c | 2 +-
12 files changed, 34 insertions(+), 29 deletions(-)
^ permalink raw reply
* Re: [PATCH] Audit: Fix skb leak when audit rate limit is exceeded
From: Paul Moore @ 2025-09-10 23:54 UTC (permalink / raw)
To: Gerald Yang, Casey Schaufler, linux-security-module, audit; +Cc: gerald.yang.tw
In-Reply-To: <20250909131056.3395574-1-gerald.yang@canonical.com>
On Sep 9, 2025 Gerald Yang <gerald.yang@canonical.com> wrote:
>
> When configuring a small audit rate limit in
> /etc/audit/rules.d/audit.rules:
> -a always,exit -F arch=b64 -S openat -S truncate -S ftruncate
> -F exit=-EACCES -F auid>=1000 -F auid!=4294967295 -k access -r 100
>
> And then repeatedly triggering permission denied as a normal user:
> while :; do cat /proc/1/environ; done
>
> We can see the messages in kernel log:
> [ 2531.862184] audit: rate limit exceeded
>
> The unreclaimable slab objects start to leak quickly. With kmemleak
> enabled, many call traces appear like:
> unreferenced object 0xffff99144b13f600 (size 232):
> comm "cat", pid 1100, jiffies 4294739144
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace (crc 8540ec4f):
> kmemleak_alloc+0x4a/0x90
> kmem_cache_alloc_node+0x2ea/0x390
> __alloc_skb+0x174/0x1b0
> audit_log_start+0x198/0x3d0
> audit_log_proctitle+0x32/0x160
> audit_log_exit+0x6c6/0x780
> __audit_syscall_exit+0xee/0x140
> syscall_exit_work+0x12b/0x150
> syscall_exit_to_user_mode_prepare+0x39/0x80
> syscall_exit_to_user_mode+0x11/0x260
> do_syscall_64+0x8c/0x180
> entry_SYSCALL_64_after_hwframe+0x78/0x80
>
> This shows that the skb allocated in audit_log_start() and queued
> onto skb_list is never freed.
>
> In audit_log_end(), each skb is dequeued from skb_list and passed
> to __audit_log_end(). However, when the audit rate limit is exceeded,
> __audit_log_end() simply prints "rate limit exceeded" and returns
> without processing the skb. Since the skb is already removed from
> skb_list, audit_buffer_free() cannot free it later, leading to a
> memory leak.
>
> Fix this by freeing the skb when the rate limit is exceeded.
>
> Signed-off-by: Gerald Yang <gerald.yang@canonical.com>
> ---
> kernel/audit.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Thanks Gerald, this looks good to me, merged into audit/dev with a fixes
tag and a slight tweak to the subject line.
For those of you wondering about -stable, existing kernels, including
Linus' current tree are just fine; this patch fixes a regression found
in commit eb59d494eebd ("audit: add record for multiple task security
contexts") which is only present in the audit/dev branch.
--
paul-moore.com
^ permalink raw reply
* [PATCH] pid: use ns_capable_noaudit() when determining net sysctl permissions
From: Christian Göttsche @ 2025-09-10 19:26 UTC (permalink / raw)
To: linux-kernel
Cc: Christian Göttsche, Christian Brauner, linux-security-module,
selinux
From: Christian Göttsche <cgzones@googlemail.com>
The capability check should not be audited since it is only being used
to determine the inode permissions. A failed check does not indicate a
violation of security policy but, when an LSM is enabled, a denial audit
message was being generated.
The denial audit message can either lead to the capability being
unnecessarily allowed in a security policy, or being silenced potentially
masking a legitimate capability check at a later point in time.
Similar to commit d6169b0206db ("net: Use ns_capable_noaudit() when
determining net sysctl permissions")
Fixes: 7863dcc72d0f ("pid: allow pid_max to be set per pid namespace")
CC: Christian Brauner <brauner@kernel.org>
CC: linux-security-module@vger.kernel.org
CC: selinux@vger.kernel.org
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
kernel/pid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/pid.c b/kernel/pid.c
index c45a28c16cd2..d94ce0250501 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -680,7 +680,7 @@ static int pid_table_root_permissions(struct ctl_table_header *head,
container_of(head->set, struct pid_namespace, set);
int mode = table->mode;
- if (ns_capable(pidns->user_ns, CAP_SYS_ADMIN) ||
+ if (ns_capable_noaudit(pidns->user_ns, CAP_SYS_ADMIN) ||
uid_eq(current_euid(), make_kuid(pidns->user_ns, 0)))
mode = (mode & S_IRWXU) >> 6;
else if (in_egroup_p(make_kgid(pidns->user_ns, 0)))
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] ima: don't clear IMA_DIGSIG flag when setting non-IMA xattr
From: Mimi Zohar @ 2025-09-10 12:21 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: <wwcnoevou44eoe3ner4oegtdsdg46tlvwidu3ynobs7huac7ae@ljivg5ksohxv>
On Wed, 2025-09-10 at 09:36 +0800, Coiby Xu wrote:
> On Mon, Sep 08, 2025 at 04:58:05PM -0400, Mimi Zohar wrote:
> > On Mon, 2025-09-08 at 10:53 -0400, Mimi Zohar wrote:
> > > 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.
> >
> > My mistake. An appraise policy indeed needs to be defined for the file
> > signature to be replaced with a file hash.
>
> Thanks for the clarification! rpm-plugin-ima does try to install IMA
> signature as shown from the following strace output,
Agreed. I was referring to the SELinux label, which would be installed for new
files, but not necessarily re-installed on existing files. The test program
simplified testing. Thank you.
Mimi
^ permalink raw reply
* Re: [PATCH v2] ima: don't clear IMA_DIGSIG flag when setting non-IMA xattr
From: Mimi Zohar @ 2025-09-10 12:21 UTC (permalink / raw)
To: Coiby Xu, linux-integrity
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM,
open list
In-Reply-To: <20250908105825.1573222-1-coxu@redhat.com>
On Mon, 2025-09-08 at 18:58 +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.
The problem description should be generic.
-> and then writes some other security xattr for the file.
Start a new paragraph here for the example.
> 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.
Start a new paragraph here, adding a sentence describing the solution to the
problem. For example,
Prevent replacing the IMA file signature with a file hash, by preventing the
IMA_DIGSIG flag from being reset.
>
> 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. The updated patch looks good. Have you looked at the other
calls to ima_reset_appraise_flags() to make sure they don't need to be adjusted?
thanks,
Mimi
> ---
> 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);
^ permalink raw reply
* Re: [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature
From: Mimi Zohar @ 2025-09-10 11:15 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: <r3himk4z2aiyqsjstlpnda4wafeo7i4oum3n2dbvnasmtep5ex@zqodcpjmyx5b>
On Wed, 2025-09-10 at 09:20 +0800, Coiby Xu wrote:
> On Tue, Sep 09, 2025 at 11:31:20AM -0400, Mimi Zohar wrote:
> > On Tue, 2025-09-09 at 12:19 +0800, Coiby Xu wrote:
> > > When both IMA and EVM fix modes are enabled, accessing a file with IMA
> > > signature won't cause security.evm to be fixed. But this doesn't happen
> > > to a file with correct IMA hash already set because accessing it will
> > > cause setting security.ima again which triggers fixing security.evm
> > > thanks to security_inode_post_setxattr->evm_update_evmxattr.
> > >
> > > Let's use the same mechanism to fix security.evm for a file with IMA
> > > signature.
> > >
> > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> >
> > Agreed, re-writing the file signature stored as security.ima would force
> > security.evm to be updated.
> >
> > Unfortunately, I'm missing something. ima_appraise_measurement() first verifies
> > the existing security.evm xattr, before verifying the security.ima xattr. If
> > the EVM HMAC fails to verify, it immediately exits ima_appraise_measurement().
> > security.ima in this case is never verified.
> >
> > This patch seems to address the case where the existing security.evm is valid,
> > but the file signature stored in security.ima is invalid. (To get to the new
> > code, the "status" flag is not INTEGRITY_PASS.) Re-writing the same invalid
> > file signature would solve an invalid security.evm, but not an invalid IMA file
> > signature. What am I missing?
>
> Hi, Mimi,
>
> Thanks for raising the question! This patch is to address the case where
> IMA signature is already added but security.evm doesn't yet exist. So
> EVM HMAC fails to verify but there is no exiting
> ima_appraise_measurement immediately.
>
> And you are right that re-writing an invalid IMA file won't fix an
> invalid IMA file signature. And even when IMA signature is valid, the
> verification may fail because the key is missing from .ima keyring. This
> happens because we need to turn off secure boot to enable fix mode. As a
> result, CA keys won't be loaded into .machine keyring.
> Btw, if I'm not
> mistaken, current IMA code assumes we are not supposed to fix IMA file
> signature.
Right, unlike file hashes, new or the same file signature can be written, but
cannot be "fixed" in the literal sense, as that would require calculating the
file hash and signing it with a private key.
This patch triggers "fixing" the EVM HMAC by re-writing the existing IMA file
signature. I assume the same result could be achieved by simply re-installing
the file signature. In both cases, the EVM HMAC key needs to exist and be
loaded.
>
> >
> > > ---
> > > security/integrity/ima/ima_appraise.c | 27 +++++++++++++++++++++------
> > > 1 file changed, 21 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> > > index f435eff4667f..18c3907c5e44 100644
> > > --- a/security/integrity/ima/ima_appraise.c
> > > +++ b/security/integrity/ima/ima_appraise.c
> > > @@ -595,12 +595,27 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
> > > integrity_audit_msg(audit_msgno, inode, filename,
> > > op, cause, rc, 0);
> > > } else if (status != INTEGRITY_PASS) {
> > > - /* Fix mode, but don't replace file signatures. */
> > > - if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig &&
> > > - (!xattr_value ||
> > > - xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
> > > - if (!ima_fix_xattr(dentry, iint))
> > > - status = INTEGRITY_PASS;
> > > + /*
> > > + * Fix mode, but don't replace file signatures.
> > > + *
> > > + * When EVM fix mode is also enabled, security.evm will be
> > > + * fixed automatically when security.ima is set because of
> > > + * security_inode_post_setxattr->evm_update_evmxattr.
> > > + */
> > > + if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig) {
> > > + if (!xattr_value ||
> > > + xattr_value->type != EVM_IMA_XATTR_DIGSIG) {
> > > + if (ima_fix_xattr(dentry, iint))
> > > + status = INTEGRITY_PASS;
> > > + } else if (xattr_value->type == EVM_IMA_XATTR_DIGSIG &&
> > > + evm_revalidate_status(XATTR_NAME_IMA)) {
> > > + if (!__vfs_setxattr_noperm(&nop_mnt_idmap,
> > > + dentry,
> > > + XATTR_NAME_IMA,
> > > + xattr_value,
> > > + xattr_len, 0))
> > > + status = INTEGRITY_PASS;
> > > + }
> > > }
> > >
> > > /*
> > >
> > > base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0
> >
>
^ permalink raw reply
* Re: [PATCH] ima: don't clear IMA_DIGSIG flag when setting non-IMA xattr
From: Coiby Xu @ 2025-09-10 1:36 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: <9fb8781bfb9c9ae9dd0a1413e23cae20dcd7356a.camel@linux.ibm.com>
On Mon, Sep 08, 2025 at 04:58:05PM -0400, Mimi Zohar wrote:
>On Mon, 2025-09-08 at 10:53 -0400, Mimi Zohar wrote:
>> 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.
>
>My mistake. An appraise policy indeed needs to be defined for the file
>signature to be replaced with a file hash.
Thanks for the clarification! rpm-plugin-ima does try to install IMA
signature as shown from the following strace output,
# strace rpm --reinstall ip*.rpm
openat(11, "lnstat;68aee3f4", O_WRONLY|O_CREAT|O_EXCL, 0200) = 12
dup(12) = 13
write(13, "\177ELF\2\1\1\0\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\0'\0\0\0\0\0\0"..., 19256) = 19256
close(13) = 0
getuid() = 0
fchown(12, 0, 0) = 0
fchmod(12, 0755) = 0
getuid() = 0
utimensat(12, NULL, [{tv_sec=1734480000, tv_nsec=0} /* 2024-12-17T19:00:00-0500 */, {tv_sec=1734480000, tv_nsec=0} /* 2024-12-17T19:00:00-0500 */], 0) = 0
fsetxattr(12, "security.ima", "\3\2\4\3232\4I\0f0d\0020O\231\341q\323Q\322\235\341\7\323\224\205\2104\24\241\331#"..., 111, 0) = 0
fsetxattr(12, "security.selinux", "system_u:object_r:bin_t:s0", 27, 0) = 0
close(12) = 0
But after rpm-plugin-selinux sets security.selinux, the IMA
signature get cleared and is replaced with IMA hash.
>
>>
>> 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>".
Since rpm-plugin-selinux will write security.selinux for a newly
installed file, so this issue can be easily reproduced. If you want to
reproduce this issue by yourself, here are the steps to reproduce this
issue on Fedora,
1. Turn off secure boot and boot the kernel with
"ima_appraise=fix evm=fix ima_policy=appraise_tcb"
2. dnf install rpm-plugin-ima -y
3. dnf reinstall iproute -y
4. Run "getfattr -m - -d -e hex /usr/sbin/ip" to check if /usr/sbin/ip has IMA signature set
And my attached C reproducer is to extract the essence of what
rpm-plugin-ima does so it can be a minimal reproducer and also to
illustrate what the problem is.
>>
>> thanks,
>>
>> Mimi
>>
>
--
Best regards,
Coiby
^ permalink raw reply
* Re: [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature
From: Coiby Xu @ 2025-09-10 1:20 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: <5aeecf1aa6eff8ae0ea0a9e95d5df79aee338b32.camel@linux.ibm.com>
On Tue, Sep 09, 2025 at 11:31:20AM -0400, Mimi Zohar wrote:
>On Tue, 2025-09-09 at 12:19 +0800, Coiby Xu wrote:
>> When both IMA and EVM fix modes are enabled, accessing a file with IMA
>> signature won't cause security.evm to be fixed. But this doesn't happen
>> to a file with correct IMA hash already set because accessing it will
>> cause setting security.ima again which triggers fixing security.evm
>> thanks to security_inode_post_setxattr->evm_update_evmxattr.
>>
>> Let's use the same mechanism to fix security.evm for a file with IMA
>> signature.
>>
>> Signed-off-by: Coiby Xu <coxu@redhat.com>
>
>Agreed, re-writing the file signature stored as security.ima would force
>security.evm to be updated.
>
>Unfortunately, I'm missing something. ima_appraise_measurement() first verifies
>the existing security.evm xattr, before verifying the security.ima xattr. If
>the EVM HMAC fails to verify, it immediately exits ima_appraise_measurement().
>security.ima in this case is never verified.
>
>This patch seems to address the case where the existing security.evm is valid,
>but the file signature stored in security.ima is invalid. (To get to the new
>code, the "status" flag is not INTEGRITY_PASS.) Re-writing the same invalid
>file signature would solve an invalid security.evm, but not an invalid IMA file
>signature. What am I missing?
Hi, Mimi,
Thanks for raising the question! This patch is to address the case where
IMA signature is already added but security.evm doesn't yet exist. So
EVM HMAC fails to verify but there is no exiting
ima_appraise_measurement immediately.
And you are right that re-writing an invalid IMA file won't fix an
invalid IMA file signature. And even when IMA signature is valid, the
verification may fail because the key is missing from .ima keyring. This
happens because we need to turn off secure boot to enable fix mode. As a
result, CA keys won't be loaded into .machine keyring. Btw, if I'm not
mistaken, current IMA code assumes we are not supposed to fix IMA file
signature.
>
>thanks,
>
>Mimi
>
>> ---
>> security/integrity/ima/ima_appraise.c | 27 +++++++++++++++++++++------
>> 1 file changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
>> index f435eff4667f..18c3907c5e44 100644
>> --- a/security/integrity/ima/ima_appraise.c
>> +++ b/security/integrity/ima/ima_appraise.c
>> @@ -595,12 +595,27 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
>> integrity_audit_msg(audit_msgno, inode, filename,
>> op, cause, rc, 0);
>> } else if (status != INTEGRITY_PASS) {
>> - /* Fix mode, but don't replace file signatures. */
>> - if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig &&
>> - (!xattr_value ||
>> - xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
>> - if (!ima_fix_xattr(dentry, iint))
>> - status = INTEGRITY_PASS;
>> + /*
>> + * Fix mode, but don't replace file signatures.
>> + *
>> + * When EVM fix mode is also enabled, security.evm will be
>> + * fixed automatically when security.ima is set because of
>> + * security_inode_post_setxattr->evm_update_evmxattr.
>> + */
>> + if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig) {
>> + if (!xattr_value ||
>> + xattr_value->type != EVM_IMA_XATTR_DIGSIG) {
>> + if (ima_fix_xattr(dentry, iint))
>> + status = INTEGRITY_PASS;
>> + } else if (xattr_value->type == EVM_IMA_XATTR_DIGSIG &&
>> + evm_revalidate_status(XATTR_NAME_IMA)) {
>> + if (!__vfs_setxattr_noperm(&nop_mnt_idmap,
>> + dentry,
>> + XATTR_NAME_IMA,
>> + xattr_value,
>> + xattr_len, 0))
>> + status = INTEGRITY_PASS;
>> + }
>> }
>>
>> /*
>>
>> base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0
>
--
Best regards,
Coiby
^ permalink raw reply
* Re: [PATCH v2] memfd,selinux: call security_inode_init_security_anon
From: Paul Moore @ 2025-09-09 21:32 UTC (permalink / raw)
To: Stephen Smalley
Cc: Thiébaud Weksteen, 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: <CAHC9VhR1pEFYzSFaqqWsU8C6vDaH_E8uZZ5g=KyK6TJvA7a8MQ@mail.gmail.com>
On Tue, Sep 9, 2025 at 5:10 PM Paul Moore <paul@paul-moore.com> wrote:
>
> Considering that we are at -rc5 right now, we only have a few more
> days left in the current dev cycle, I'm going to merge this now (with
> a subject line tweak and some unnecessary vertical whitespace
> removed), and I'll put together a quick little patch to do the
> WARN_ON()/EACCES conversion which you'll see on list shortly ...
The patch can be found at the link below.
https://lore.kernel.org/selinux/20250909213020.343501-2-paul@paul-moore.com/
--
paul-moore.com
^ 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