* Re: [RFC PATCH v1 1/2] fs: Add O_DENY_WRITE
From: Mickaël Salaün @ 2025-08-24 11:03 UTC (permalink / raw)
To: Jann Horn
Cc: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn,
Andy Lutomirski, Arnd Bergmann, Christian Heimes, Dmitry Vyukov,
Elliott Hughes, Fan Wu, Florian Weimer, Jeff Xu, Jonathan Corbet,
Jordan R Abrahams, Lakshmi Ramasubramanian, Luca Boccassi,
Matt Bobrowski, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Robert Waite, Roberto Sassu, Scott Shell, Steve Dower,
Steve Grubb, kernel-hardening, linux-api, linux-fsdevel,
linux-integrity, linux-kernel, linux-security-module,
Andy Lutomirski, Jeff Xu
In-Reply-To: <CAG48ez1XjUdcFztc_pF2qcoLi7xvfpJ224Ypc=FoGi-Px-qyZw@mail.gmail.com>
On Fri, Aug 22, 2025 at 09:45:32PM +0200, Jann Horn wrote:
> On Fri, Aug 22, 2025 at 7:08 PM Mickaël Salaün <mic@digikod.net> wrote:
> > Add a new O_DENY_WRITE flag usable at open time and on opened file (e.g.
> > passed file descriptors). This changes the state of the opened file by
> > making it read-only until it is closed. The main use case is for script
> > interpreters to get the guarantee that script' content cannot be altered
> > while being read and interpreted. This is useful for generic distros
> > that may not have a write-xor-execute policy. See commit a5874fde3c08
> > ("exec: Add a new AT_EXECVE_CHECK flag to execveat(2)")
> >
> > Both execve(2) and the IOCTL to enable fsverity can already set this
> > property on files with deny_write_access(). This new O_DENY_WRITE make
>
> The kernel actually tried to get rid of this behavior on execve() in
> commit 2a010c41285345da60cece35575b4e0af7e7bf44.; but sadly that had
> to be reverted in commit 3b832035387ff508fdcf0fba66701afc78f79e3d
> because it broke userspace assumptions.
Oh, good to know.
>
> > it widely available. This is similar to what other OSs may provide
> > e.g., opening a file with only FILE_SHARE_READ on Windows.
>
> We used to have the analogous mmap() flag MAP_DENYWRITE, and that was
> removed for security reasons; as
> https://man7.org/linux/man-pages/man2/mmap.2.html says:
>
> | MAP_DENYWRITE
> | This flag is ignored. (Long ago—Linux 2.0 and earlier—it
> | signaled that attempts to write to the underlying file
> | should fail with ETXTBSY. But this was a source of denial-
> | of-service attacks.)"
>
> It seems to me that the same issue applies to your patch - it would
> allow unprivileged processes to essentially lock files such that other
> processes can't write to them anymore. This might allow unprivileged
> users to prevent root from updating config files or stuff like that if
> they're updated in-place.
Yes, I agree, but since it is the case for executed files I though it
was worth starting a discussion on this topic. This new flag could be
restricted to executable files, but we should avoid system-wide locks
like this. I'm not sure how Windows handle these issues though.
Anyway, we should rely on the access control policy to control write and
execute access in a consistent way (e.g. write-xor-execute). Thanks for
the references and the background!
^ permalink raw reply
* Re: LSM namespacing API
From: John Johansen @ 2025-08-23 23:00 UTC (permalink / raw)
To: Dr. Greg
Cc: Casey Schaufler, Paul Moore, Micka??l Sala??n,
linux-security-module, selinux, Stephen Smalley, Maxime B??lair
In-Reply-To: <20250823174101.GA22123@wind.enjellic.com>
On 8/23/25 10:41, Dr. Greg wrote:
> On Fri, Aug 22, 2025 at 12:59:29PM -0700, John Johansen wrote:
>
> Good morning, I hope the weekend is going well for everyone.
>
>> On 8/22/25 07:47, Casey Schaufler wrote:
>>> On 8/21/2025 7:14 PM, Paul Moore wrote:
>>>> On Thu, Aug 21, 2025 at 6:00???AM Micka??l Sala??n <mic@digikod.net>
>>>> wrote:
>>>>> On Tue, Aug 19, 2025 at 02:40:52PM -0400, Paul Moore wrote:
>>>>>> On Tue, Aug 19, 2025 at 1:11???PM Casey Schaufler
>>>>>> <casey@schaufler-ca.com> wrote:
>>>>>>> The advantage of a clone flag is that the operation is atomic with
>>>>>>> the other namespace flag based behaviors. Having a two step process
>>>>>>>
>>>>>>> clone(); lsm_set_self_attr(); - or -
>>>>>>> lsm_set_self_attr(); clone();
>>>>>>>
>>>>>>> is going to lead to cases where neither order really works correctly.
>>>>>> I was envisioning something that works similarly to LSM_ATTR_EXEC
>>>>>> where the unshare isn't immediate, but rather happens at a future
>>>>>> event. With LSM_ATTR_EXEC it happens at the next exec*(), with
>>>>>> LSM_ATTR_UNSHARE I imagine it would happen at the next clone*().
>>>>> The next unshare(2) would make more sense to me.
>>>> That's definitely something to discuss. I've been fairly loose on
>>>> that in the discussion thus far, but as things are starting to settle
>>>> on the lsm_set_self_attr(2) approach as one API, we should start to
>>>> clarify that.
>>>>
>>>>> This deferred operation could be requested with a flag in
>>>>> lsm_config_system_policy(2) instead:
>>>>> https://lore.kernel.org/r/20250709080220.110947-1-maxime.belair@canonical.com
>>>> I want to keep the policy syscall work separate from the LSM namespace
>>>> discussion as we don't want to require a policy load operation to
>>>> create a new LSM namespace. I think it's probably okay if the policy
>>>> syscall work were to be namespace aware so that an orchestrator could
>>>> load a LSM policy into a LSM namespace other than it's own, but that
>>>> is still not overly dependent on what we are discussing here (yes,
>>>> maybe it is a little, but only just so).
>>>
>>> Policy load and namespace manipulation *must* be kept separate. Smack
>>> requires the ability to "load policy" at any time. Smack allows a process
>>> to add "policy" to further restrict its own access, and does not require
>>> a namespace change. There has been an implementation of namespaces for
>>> Smack, but the developers disappeared quietly and sadly no one picked it
>>> up. Introducing a requirement that LSMs support namespaces in order to
>>> load policy beyond system initialization is a non-starter.
>
>> yes the ability to load policy must be exist separately, however
>> policy load could be made namespace aware so that a parent could
>> inject policy into a child.
>
> Policy or model load, specific to the subordinate namespace, will be
> a necessity.
>
> As Casey noted, some LSM namespaces will require configuration and
> management calls well after the namespace has started. Other LSM's
> will want the configuration to be completed before the namespace
> starts, with any further configurations to the namespace blocked.
>
> There is a very valid security rationale for isolating the capability
> for namespace separation from the capability that allows the
> configuration of a security model. It would be an entirely realistic
> security objective for a namespace to block further separation
> attempts, while still allowing for management operations to be
> conducted in the context of the subordinate namespace.
>
> Hence the rationale for splitting CAP_MAC_ADMIN from whatever name the
> bike shedding process around the new capability naming process
> produces.
>
>> There is also an open question as to whether we need to allow, but
>> not require, some kind of policy manipulation/injection with the
>> creation of the LSM namespace so that the there is an atomic
>> transition with entering the namespace. Is there a case where policy
>> really needs to be present atomically with the creation of the
>> namespace? If so we need to further break it down to
>>
>> 1. is it sufficient for the LSM to do it, without container manager
>> guidance? An inherit of policy, or already present policy that can be
>> injected. Then we don't need policy load inject to be considered at
>> the point of clone/unshare.
>>
>> 2. do we need to let the container manager hint/load policy.
>
> Policy load needs to be atomic with respect to namespace separation.
> In other words, the policy needs to be in place when execution within
> the context of the new security namespace begins.
>
no, it _may_ need to be depending on the model/policy being used, and
an LSM is in the best place to make that decision and do it for its own
policy as long as the infrastructure supports it.
> A resource orchestrator will need the ability to load the new policy
> that will be enforced into the context of the new namespace.
No an LSM is fully capable of doing this and honestly in a better
position to do so for its own policy than an external orchestrator.
Where coordination orchestration is need is at the infrastructure layer
(LSM), to ensure once everything is decided by inidivual LSMs that
what the security context is atomically setup correctly.
So in that sense the LSM infrastructure is an orchestrator, but only
in the loosest sense.
>
> In the case of some model/integrity based LSM's, the security events
> related to the policy load need to occur in the context of the parent
> LSM namespace.
>
yes, it very much depends on the model. I would argue if the LSM needs
this.
1. the policy at the exec/fork/clone/unshare point already needs to
be loaded.
2. the LSMs policy needs a way to initiate the transition. Eg. in
the selinux case, the transition is setting up a new layer in
mediation that will be bounded by the previous layers. There isn't
a transition from one policy to another, but adding a new layer
on top of.
> See the writings of Werner Karl Heisenberg for the reasoning behind
> that... :-)
>
>> So far I think the inherit/policy directed injection works for
>> apparmor, and selinux. Container managers generally speaking have to
>> additional setup after the container is created before running the
>> work load, which means a separate load phase should be fine.
>>
>> However I can see an argument for having policy in place when
>> clone/unshare exit. Admittedly atm its largely around flexibility, and
>> nebulous ill defined use cases. Just because something works for
>> apparmor, selinux, and I think smack, doesn't mean it would work for
>> all use cases.
>>
>> But we also should add flexibility for flexibility just because we can
>> see there might be some future utility for some future use case. It
>> would certainly make the interface uglier, and more complicated, and I
>> would hate to have to carry that without a concrete use case.
>>
>> I think unless there is a solid use case for making clone/unshare
>> policy aware we don't worry about it for now. A new interface can be
>> add in the future if the capability is really needed.
>
> We will respond more directly to the issue of clone, unshare and
> external process entry, in the other thread where you initiated a
> discussion of these issues. We believe there is a strong argument to
> be made that LSM namespace separation is a poor fit for the classic
> fork/unshare model of the other resource namespaces.
>
the other resource namespaces being able to move independent of the
security namespace, or at least mediation by the security namespace is
a complete disaster and should not have ever been allowed.
> Among other issues, a direct separation model places the complexity of
> policy verification and loading in userspace. As was noted above,
> accounting for the security events related to the policy verification
> and load process, in the orchestrator process, will be a requirement
> for some integrity and functional models.
>
There are different levels of verification. It makes sense to do some
of it in the individual LSM, some of it in userspace, and potentially
some at another level in another LSM. Unfortunately Linux has forced the
concept of containers to be a user level construct, and this forces
certain verifications around containers to be in userspace.
AppArmor does a policy verification checking that policy meet all the
bounding constraints etc. Is very different than the verification check
that IMA may doing check that this policy is blessed and allowed to be
loaded. AppArmor could support some IMA verification but is very much
designed to be like landlock in that unprivileged userspace _may_ have
privilege to load policy into the kernel. You may not want to allow
that on some systems, but you certainly do on others. The system level
signature check that IMA does isn't appropriate for unprivileged
user policy. But the apparmor verification check is.
and Yes something like IMA that is doing a system level integrity is going
to need a post policy load callback to do verification. This again doesn't
need an orchestrator, but just support in the infrastructure, and a
callback that individual LSMs can trigger. See the work Paul is doing
to rework the LSM init or how IMA is doing a verification of selinux
policy.
Of course you have to trust the LSMs to trigger the callback, but its
opersource and the code can be checked. If you can't trust the individual
LSMs you have a much bigger problem because you just can't trust a
monolithic kernel and you are going need a trust zone/hyper visor above
the kernel to do any form of integrity check you can trust.
> Have a good weekend.
>
> As always,
> Dr. Greg
>
> The Quixote Project - Flailing at the Travails of Cybersecurity
> https://github.com/Quixote-Project
>
^ permalink raw reply
* Re: LSM namespacing API
From: Dr. Greg @ 2025-08-23 17:41 UTC (permalink / raw)
To: John Johansen
Cc: Casey Schaufler, Paul Moore, Micka??l Sala??n,
linux-security-module, selinux, Stephen Smalley, Maxime B??lair
In-Reply-To: <47fb07e3-5047-49dc-b5fa-83b4f82f9dce@canonical.com>
On Fri, Aug 22, 2025 at 12:59:29PM -0700, John Johansen wrote:
Good morning, I hope the weekend is going well for everyone.
> On 8/22/25 07:47, Casey Schaufler wrote:
> >On 8/21/2025 7:14 PM, Paul Moore wrote:
> >>On Thu, Aug 21, 2025 at 6:00???AM Micka??l Sala??n <mic@digikod.net>
> >>wrote:
> >>>On Tue, Aug 19, 2025 at 02:40:52PM -0400, Paul Moore wrote:
> >>>>On Tue, Aug 19, 2025 at 1:11???PM Casey Schaufler
> >>>><casey@schaufler-ca.com> wrote:
> >>>>>The advantage of a clone flag is that the operation is atomic with
> >>>>>the other namespace flag based behaviors. Having a two step process
> >>>>>
> >>>>> clone(); lsm_set_self_attr(); - or -
> >>>>> lsm_set_self_attr(); clone();
> >>>>>
> >>>>>is going to lead to cases where neither order really works correctly.
> >>>>I was envisioning something that works similarly to LSM_ATTR_EXEC
> >>>>where the unshare isn't immediate, but rather happens at a future
> >>>>event. With LSM_ATTR_EXEC it happens at the next exec*(), with
> >>>>LSM_ATTR_UNSHARE I imagine it would happen at the next clone*().
> >>>The next unshare(2) would make more sense to me.
> >>That's definitely something to discuss. I've been fairly loose on
> >>that in the discussion thus far, but as things are starting to settle
> >>on the lsm_set_self_attr(2) approach as one API, we should start to
> >>clarify that.
> >>
> >>>This deferred operation could be requested with a flag in
> >>>lsm_config_system_policy(2) instead:
> >>>https://lore.kernel.org/r/20250709080220.110947-1-maxime.belair@canonical.com
> >>I want to keep the policy syscall work separate from the LSM namespace
> >>discussion as we don't want to require a policy load operation to
> >>create a new LSM namespace. I think it's probably okay if the policy
> >>syscall work were to be namespace aware so that an orchestrator could
> >>load a LSM policy into a LSM namespace other than it's own, but that
> >>is still not overly dependent on what we are discussing here (yes,
> >>maybe it is a little, but only just so).
> >
> >Policy load and namespace manipulation *must* be kept separate. Smack
> >requires the ability to "load policy" at any time. Smack allows a process
> >to add "policy" to further restrict its own access, and does not require
> >a namespace change. There has been an implementation of namespaces for
> >Smack, but the developers disappeared quietly and sadly no one picked it
> >up. Introducing a requirement that LSMs support namespaces in order to
> >load policy beyond system initialization is a non-starter.
> yes the ability to load policy must be exist separately, however
> policy load could be made namespace aware so that a parent could
> inject policy into a child.
Policy or model load, specific to the subordinate namespace, will be
a necessity.
As Casey noted, some LSM namespaces will require configuration and
management calls well after the namespace has started. Other LSM's
will want the configuration to be completed before the namespace
starts, with any further configurations to the namespace blocked.
There is a very valid security rationale for isolating the capability
for namespace separation from the capability that allows the
configuration of a security model. It would be an entirely realistic
security objective for a namespace to block further separation
attempts, while still allowing for management operations to be
conducted in the context of the subordinate namespace.
Hence the rationale for splitting CAP_MAC_ADMIN from whatever name the
bike shedding process around the new capability naming process
produces.
> There is also an open question as to whether we need to allow, but
> not require, some kind of policy manipulation/injection with the
> creation of the LSM namespace so that the there is an atomic
> transition with entering the namespace. Is there a case where policy
> really needs to be present atomically with the creation of the
> namespace? If so we need to further break it down to
>
> 1. is it sufficient for the LSM to do it, without container manager
> guidance? An inherit of policy, or already present policy that can be
> injected. Then we don't need policy load inject to be considered at
> the point of clone/unshare.
>
> 2. do we need to let the container manager hint/load policy.
Policy load needs to be atomic with respect to namespace separation.
In other words, the policy needs to be in place when execution within
the context of the new security namespace begins.
A resource orchestrator will need the ability to load the new policy
that will be enforced into the context of the new namespace.
In the case of some model/integrity based LSM's, the security events
related to the policy load need to occur in the context of the parent
LSM namespace.
See the writings of Werner Karl Heisenberg for the reasoning behind
that... :-)
> So far I think the inherit/policy directed injection works for
> apparmor, and selinux. Container managers generally speaking have to
> additional setup after the container is created before running the
> work load, which means a separate load phase should be fine.
>
> However I can see an argument for having policy in place when
> clone/unshare exit. Admittedly atm its largely around flexibility, and
> nebulous ill defined use cases. Just because something works for
> apparmor, selinux, and I think smack, doesn't mean it would work for
> all use cases.
>
> But we also should add flexibility for flexibility just because we can
> see there might be some future utility for some future use case. It
> would certainly make the interface uglier, and more complicated, and I
> would hate to have to carry that without a concrete use case.
>
> I think unless there is a solid use case for making clone/unshare
> policy aware we don't worry about it for now. A new interface can be
> add in the future if the capability is really needed.
We will respond more directly to the issue of clone, unshare and
external process entry, in the other thread where you initiated a
discussion of these issues. We believe there is a strong argument to
be made that LSM namespace separation is a poor fit for the classic
fork/unshare model of the other resource namespaces.
Among other issues, a direct separation model places the complexity of
policy verification and loading in userspace. As was noted above,
accounting for the security events related to the policy verification
and load process, in the orchestrator process, will be a requirement
for some integrity and functional models.
Have a good weekend.
As always,
Dr. Greg
The Quixote Project - Flailing at the Travails of Cybersecurity
https://github.com/Quixote-Project
^ permalink raw reply
* Re: [PATCH v3 31/34] ima,evm: move initcalls to the LSM framework
From: Paul Moore @ 2025-08-22 20:45 UTC (permalink / raw)
To: Mimi Zohar, 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: <20250814225159.275901-67-paul@paul-moore.com>
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.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
> security/integrity/Makefile | 2 +-
> security/integrity/evm/evm_main.c | 6 ++---
> security/integrity/iint.c | 4 +--
> security/integrity/ima/ima_main.c | 6 ++---
> security/integrity/initcalls.c | 41 +++++++++++++++++++++++++++++++
> security/integrity/initcalls.h | 28 +++++++++++++++++++++
> 6 files changed, 78 insertions(+), 9 deletions(-)
> create mode 100644 security/integrity/initcalls.c
> create mode 100644 security/integrity/initcalls.h
Mimi, Roberto, I believe I've incorporated all of your feedback thus
far, does this patch look okay to you? If so, can I get an ACK from
one or both of you?
> diff --git a/security/integrity/Makefile b/security/integrity/Makefile
> index 92b63039c654..6ea330ea88b1 100644
> --- a/security/integrity/Makefile
> +++ b/security/integrity/Makefile
> @@ -5,7 +5,7 @@
>
> obj-$(CONFIG_INTEGRITY) += integrity.o
>
> -integrity-y := iint.o
> +integrity-y := iint.o initcalls.o
> integrity-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o
> integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
> integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index db8e324ed4e6..823573bcaa27 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -25,6 +25,7 @@
> #include <crypto/hash.h>
> #include <crypto/hash_info.h>
> #include <crypto/utils.h>
> +#include "../initcalls.h"
> #include "evm.h"
>
> int evm_initialized;
> @@ -1112,7 +1113,7 @@ void __init evm_load_x509(void)
> }
> #endif
>
> -static int __init init_evm(void)
> +int __init init_evm(void)
> {
> int error;
> struct list_head *pos, *q;
> @@ -1179,6 +1180,5 @@ DEFINE_LSM(evm) = {
> .init = init_evm_lsm,
> .order = LSM_ORDER_LAST,
> .blobs = &evm_blob_sizes,
> + .initcall_late = integrity_late_init,
> };
> -
> -late_initcall(init_evm);
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index 068ac6c2ae1e..a4b88d67ff43 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -11,6 +11,7 @@
> */
> #include <linux/security.h>
> #include "integrity.h"
> +#include "initcalls.h"
>
> struct dentry *integrity_dir;
>
> @@ -42,7 +43,7 @@ void __init integrity_load_keys(void)
> evm_load_x509();
> }
>
> -static int __init integrity_fs_init(void)
> +int __init integrity_fs_init(void)
> {
> integrity_dir = securityfs_create_dir("integrity", NULL);
> if (IS_ERR(integrity_dir)) {
> @@ -58,4 +59,3 @@ static int __init integrity_fs_init(void)
> return 0;
> }
>
> -late_initcall(integrity_fs_init)
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index eade8e1e3cb1..06ae59cd77f4 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -28,6 +28,7 @@
> #include <linux/iversion.h>
> #include <linux/evm.h>
> #include <linux/crash_dump.h>
> +#include "../initcalls.h"
>
> #include "ima.h"
>
> @@ -1202,7 +1203,7 @@ static int ima_kernel_module_request(char *kmod_name)
>
> #endif /* CONFIG_INTEGRITY_ASYMMETRIC_KEYS */
>
> -static int __init init_ima(void)
> +int __init init_ima(void)
> {
> int error;
>
> @@ -1283,6 +1284,5 @@ DEFINE_LSM(ima) = {
> .init = init_ima_lsm,
> .order = LSM_ORDER_LAST,
> .blobs = &ima_blob_sizes,
> + .initcall_late = integrity_late_init,
> };
> -
> -late_initcall(init_ima); /* Start IMA after the TPM is available */
> diff --git a/security/integrity/initcalls.c b/security/integrity/initcalls.c
> new file mode 100644
> index 000000000000..6afa411068f2
> --- /dev/null
> +++ b/security/integrity/initcalls.c
> @@ -0,0 +1,41 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * IMA/EVM initcalls
> + *
> + */
> +
> +#include <linux/init.h>
> +
> +#include "initcalls.h"
> +
> +/**
> + * integrity_late_init - late_initcalls for IMA/EVM
> + *
> + * This helper function wraps all of the late_initcalls for both IMA and EVM.
> + * It can be called multiple times, e.g. once from IMA and once from EVM,
> + * without problem as it maintains an internal static state variable which
> + * ensures that any setup/initialization is only done once.
> + */
> +int __init integrity_late_init(void)
> +{
> + int rc = 0, rc_tmp;
> + static bool setup = false;
> +
> + if (setup)
> + return 0;
> + setup = true;
> +
> + rc_tmp = integrity_fs_init();
> + if (!rc && rc_tmp)
> + rc = rc_tmp;
> +
> + rc_tmp = init_ima();
> + if (!rc && rc_tmp)
> + rc = rc_tmp;
> +
> + rc_tmp = init_evm();
> + if (!rc && rc_tmp)
> + rc = rc_tmp;
> +
> + return rc;
> +}
> diff --git a/security/integrity/initcalls.h b/security/integrity/initcalls.h
> new file mode 100644
> index 000000000000..b56e9c576505
> --- /dev/null
> +++ b/security/integrity/initcalls.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _INTEGRITY_INITCALLS_H
> +#define _INTEGRITY_INITCALLS_H
> +
> +int integrity_fs_init(void);
> +
> +#ifdef CONFIG_IMA
> +int init_ima(void);
> +#else
> +static inline int init_ima(void)
> +{
> + return 0;
> +}
> +#endif
> +
> +#ifdef CONFIG_EVM
> +int init_evm(void);
> +#else
> +static inline int init_evm(void)
> +{
> + return 0;
> +}
> +#endif
> +
> +int integrity_late_init(void);
> +
> +#endif
> --
> 2.50.1
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: add the associated Rust helper to the CREDENTIALS section
From: Paul Moore @ 2025-08-22 20:36 UTC (permalink / raw)
To: linux-security-module, rust-for-linux
In-Reply-To: <20250819195841.238246-2-paul@paul-moore.com>
On Tue, Aug 19, 2025 at 3:58 PM Paul Moore <paul@paul-moore.com> wrote:
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
Merged into lsm/dev.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d61f7246e5bf..0ee0098f2df8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6484,6 +6484,7 @@ S: Supported
> T: git https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git
> F: include/linux/cred.h
> F: kernel/cred.c
> +F: rust/kernel/cred.rs
> F: Documentation/security/credentials.rst
>
> INTEL CRPS COMMON REDUNDANT PSU DRIVER
> --
> 2.50.1
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: add the associated Rust helper to the LSM section
From: Paul Moore @ 2025-08-22 20:35 UTC (permalink / raw)
To: linux-security-module; +Cc: Benno Lossin
In-Reply-To: <20250814215952.238316-2-paul@paul-moore.com>
On Thu, Aug 14, 2025 at 5:59 PM Paul Moore <paul@paul-moore.com> wrote:
>
> Suggested-by: Benno Lossin <lossin@kernel.org>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
Merged into lsm/dev.
--
paul-moore.com
^ permalink raw reply
* Re: LSM namespacing API
From: John Johansen @ 2025-08-22 19:59 UTC (permalink / raw)
To: Casey Schaufler, Paul Moore, Mickaël Salaün
Cc: linux-security-module, selinux, Stephen Smalley,
Maxime Bélair
In-Reply-To: <5612ec76-9257-402b-ac98-bdc8a8287a60@schaufler-ca.com>
On 8/22/25 07:47, Casey Schaufler wrote:
> On 8/21/2025 7:14 PM, Paul Moore wrote:
>> On Thu, Aug 21, 2025 at 6:00 AM Mickaël Salaün <mic@digikod.net> wrote:
>>> On Tue, Aug 19, 2025 at 02:40:52PM -0400, Paul Moore wrote:
>>>> On Tue, Aug 19, 2025 at 1:11 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>> The advantage of a clone flag is that the operation is atomic with
>>>>> the other namespace flag based behaviors. Having a two step process
>>>>>
>>>>> clone(); lsm_set_self_attr(); - or -
>>>>> lsm_set_self_attr(); clone();
>>>>>
>>>>> is going to lead to cases where neither order really works correctly.
>>>> I was envisioning something that works similarly to LSM_ATTR_EXEC
>>>> where the unshare isn't immediate, but rather happens at a future
>>>> event. With LSM_ATTR_EXEC it happens at the next exec*(), with
>>>> LSM_ATTR_UNSHARE I imagine it would happen at the next clone*().
>>> The next unshare(2) would make more sense to me.
>> That's definitely something to discuss. I've been fairly loose on
>> that in the discussion thus far, but as things are starting to settle
>> on the lsm_set_self_attr(2) approach as one API, we should start to
>> clarify that.
>>
>>> This deferred operation could be requested with a flag in
>>> lsm_config_system_policy(2) instead:
>>> https://lore.kernel.org/r/20250709080220.110947-1-maxime.belair@canonical.com
>> I want to keep the policy syscall work separate from the LSM namespace
>> discussion as we don't want to require a policy load operation to
>> create a new LSM namespace. I think it's probably okay if the policy
>> syscall work were to be namespace aware so that an orchestrator could
>> load a LSM policy into a LSM namespace other than it's own, but that
>> is still not overly dependent on what we are discussing here (yes,
>> maybe it is a little, but only just so).
>
> Policy load and namespace manipulation *must* be kept separate. Smack
> requires the ability to "load policy" at any time. Smack allows a process
> to add "policy" to further restrict its own access, and does not require
> a namespace change. There has been an implementation of namespaces for
> Smack, but the developers disappeared quietly and sadly no one picked it
> up. Introducing a requirement that LSMs support namespaces in order to
> load policy beyond system initialization is a non-starter.
>
yes the ability to load policy must be exist separately, however
policy load could be made namespace aware so that a parent could
inject policy into a child.
There is also an open question as to whether we need to allow, but not
require, some kind of policy manipulation/injection with the creation
of the LSM namespace so that the there is an atomic transition with
entering the namespace. Is there a case where policy really needs to
be present atomically with the creation of the namespace? If so we
need to further break it down to
1. is it sufficient for the LSM to do it, without container manager
guidance? An inherit of policy, or already present policy that can be
injected. Then we don't need policy load inject to be considered at
the point of clone/unshare.
2. do we need to let the container manager hint/load policy.
So far I think the inherit/policy directed injection works for
apparmor, and selinux. Container managers generally speaking have to
additional setup after the container is created before running the
work load, which means a separate load phase should be fine.
However I can see an argument for having policy in place when
clone/unshare exit. Admittedly atm its largely around flexibility, and
nebulous ill defined use cases. Just because something works for
apparmor, selinux, and I think smack, doesn't mean it would work for
all use cases.
But we also should add flexibility for flexibility just because we can
see there might be some future utility for some future use case. It
would certainly make the interface uglier, and more complicated, and I
would hate to have to carry that without a concrete use case.
I think unless there is a solid use case for making clone/unshare
policy aware we don't worry about it for now. A new interface can be
add in the future if the capability is really needed.
^ permalink raw reply
* Re: [RFC PATCH v1 1/2] fs: Add O_DENY_WRITE
From: Jann Horn @ 2025-08-22 19:45 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn,
Andy Lutomirski, Arnd Bergmann, Christian Heimes, Dmitry Vyukov,
Elliott Hughes, Fan Wu, Florian Weimer, Jeff Xu, Jonathan Corbet,
Jordan R Abrahams, Lakshmi Ramasubramanian, Luca Boccassi,
Matt Bobrowski, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Robert Waite, Roberto Sassu, Scott Shell, Steve Dower,
Steve Grubb, kernel-hardening, linux-api, linux-fsdevel,
linux-integrity, linux-kernel, linux-security-module,
Andy Lutomirski, Jeff Xu
In-Reply-To: <20250822170800.2116980-2-mic@digikod.net>
On Fri, Aug 22, 2025 at 7:08 PM Mickaël Salaün <mic@digikod.net> wrote:
> Add a new O_DENY_WRITE flag usable at open time and on opened file (e.g.
> passed file descriptors). This changes the state of the opened file by
> making it read-only until it is closed. The main use case is for script
> interpreters to get the guarantee that script' content cannot be altered
> while being read and interpreted. This is useful for generic distros
> that may not have a write-xor-execute policy. See commit a5874fde3c08
> ("exec: Add a new AT_EXECVE_CHECK flag to execveat(2)")
>
> Both execve(2) and the IOCTL to enable fsverity can already set this
> property on files with deny_write_access(). This new O_DENY_WRITE make
The kernel actually tried to get rid of this behavior on execve() in
commit 2a010c41285345da60cece35575b4e0af7e7bf44.; but sadly that had
to be reverted in commit 3b832035387ff508fdcf0fba66701afc78f79e3d
because it broke userspace assumptions.
> it widely available. This is similar to what other OSs may provide
> e.g., opening a file with only FILE_SHARE_READ on Windows.
We used to have the analogous mmap() flag MAP_DENYWRITE, and that was
removed for security reasons; as
https://man7.org/linux/man-pages/man2/mmap.2.html says:
| MAP_DENYWRITE
| This flag is ignored. (Long ago—Linux 2.0 and earlier—it
| signaled that attempts to write to the underlying file
| should fail with ETXTBSY. But this was a source of denial-
| of-service attacks.)"
It seems to me that the same issue applies to your patch - it would
allow unprivileged processes to essentially lock files such that other
processes can't write to them anymore. This might allow unprivileged
users to prevent root from updating config files or stuff like that if
they're updated in-place.
^ permalink raw reply
* [RFC PATCH v1 2/2] selftests/exec: Add O_DENY_WRITE tests
From: Mickaël Salaün @ 2025-08-22 17:08 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, Andy Lutomirski, Arnd Bergmann,
Christian Heimes, Dmitry Vyukov, Elliott Hughes, Fan Wu,
Florian Weimer, Jann Horn, Jeff Xu, Jonathan Corbet,
Jordan R Abrahams, Lakshmi Ramasubramanian, Luca Boccassi,
Matt Bobrowski, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Robert Waite, Roberto Sassu, Scott Shell, Steve Dower,
Steve Grubb, kernel-hardening, linux-api, linux-fsdevel,
linux-integrity, linux-kernel, linux-security-module,
Andy Lutomirski, Jeff Xu
In-Reply-To: <20250822170800.2116980-1-mic@digikod.net>
Add 6 test suites to check O_DENY_WRITE used through open(2) and
fcntl(2). Check that it fills its purpose, that it only applies to
regular files, and that setting this flag several times is not an issue.
The O_DENY_WRITE flag is useful in conjunction with AT_EXECVE_CHECK for
systems that don't enforce a write-xor-execute policy. Extend related
tests to also use them as examples.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jeff Xu <jeffxu@chromium.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Robert Waite <rowait@microsoft.com>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20250822170800.2116980-3-mic@digikod.net
---
tools/testing/selftests/exec/check-exec.c | 219 ++++++++++++++++++++++
1 file changed, 219 insertions(+)
diff --git a/tools/testing/selftests/exec/check-exec.c b/tools/testing/selftests/exec/check-exec.c
index 55bce47e56b7..9db1d7b9aa97 100644
--- a/tools/testing/selftests/exec/check-exec.c
+++ b/tools/testing/selftests/exec/check-exec.c
@@ -30,6 +30,10 @@
#define _ASM_GENERIC_FCNTL_H
#include <linux/fcntl.h>
+#ifndef O_DENY_WRITE
+#define O_DENY_WRITE 040000000
+#endif
+
#include "../kselftest_harness.h"
static int sys_execveat(int dirfd, const char *pathname, char *const argv[],
@@ -319,6 +323,221 @@ TEST_F(access, non_regular_files)
test_exec_fd(_metadata, self->pipefd, EACCES);
}
+TEST_F(access, deny_write_check_open)
+{
+ int fd_deny, fd_read, fd_write;
+
+ fd_deny = open(reg_file_path, O_DENY_WRITE | O_RDONLY | O_CLOEXEC);
+ ASSERT_LE(0, fd_deny);
+
+ /* Concurrent reads are always allowed. */
+ fd_read = open(reg_file_path, O_RDONLY | O_CLOEXEC);
+ EXPECT_LE(0, fd_read);
+ EXPECT_EQ(0, close(fd_read));
+
+ /* Concurrent writes are denied. */
+ fd_write = open(reg_file_path, O_WRONLY | O_CLOEXEC);
+ EXPECT_EQ(-1, fd_write);
+ EXPECT_EQ(ETXTBSY, errno);
+
+ /* Drops O_DENY_WRITE. */
+ EXPECT_EQ(0, close(fd_deny));
+
+ /* The restriction is now gone. */
+ fd_write = open(reg_file_path, O_WRONLY | O_CLOEXEC);
+ EXPECT_LE(0, fd_write);
+ EXPECT_EQ(0, close(fd_write));
+}
+
+TEST_F(access, deny_write_check_open_and_fcntl)
+{
+ int fd_deny, fd_read, fd_write, flags;
+
+ fd_deny = open(reg_file_path, O_DENY_WRITE | O_RDONLY | O_CLOEXEC);
+ ASSERT_LE(0, fd_deny);
+
+ /* Sets O_DENY_WRITE a "second" time. */
+ flags = fcntl(fd_deny, F_GETFL);
+ ASSERT_NE(-1, flags);
+ EXPECT_EQ(0, fcntl(fd_deny, F_SETFL, flags | O_DENY_WRITE));
+
+ /* Concurrent reads are always allowed. */
+ fd_read = open(reg_file_path, O_RDONLY | O_CLOEXEC);
+ EXPECT_LE(0, fd_read);
+ EXPECT_EQ(0, close(fd_read));
+
+ /* Concurrent writes are denied. */
+ fd_write = open(reg_file_path, O_WRONLY | O_CLOEXEC);
+ EXPECT_EQ(-1, fd_write);
+ EXPECT_EQ(ETXTBSY, errno);
+
+ /* Drops O_DENY_WRITE. */
+ EXPECT_EQ(0, close(fd_deny));
+
+ /* The restriction is now gone. */
+ fd_write = open(reg_file_path, O_WRONLY | O_CLOEXEC);
+ EXPECT_LE(0, fd_write);
+ EXPECT_EQ(0, close(fd_write));
+}
+
+TEST_F(access, deny_write_check_fcntl)
+{
+ int fd_deny, fd_read, fd_write, flags;
+
+ fd_deny = open(reg_file_path, O_RDONLY | O_CLOEXEC);
+ ASSERT_LE(0, fd_deny);
+
+ /* Sets O_DENY_WRITE a first time. */
+ flags = fcntl(fd_deny, F_GETFL);
+ ASSERT_NE(-1, flags);
+ EXPECT_EQ(0, fcntl(fd_deny, F_SETFL, flags | O_DENY_WRITE));
+
+ /* Sets O_DENY_WRITE a "second" time. */
+ EXPECT_EQ(0, fcntl(fd_deny, F_SETFL, flags | O_DENY_WRITE));
+
+ /* Concurrent reads are always allowed. */
+ fd_read = open(reg_file_path, O_RDONLY | O_CLOEXEC);
+ EXPECT_LE(0, fd_read);
+ EXPECT_EQ(0, close(fd_read));
+
+ /* Concurrent writes are denied. */
+ fd_write = open(reg_file_path, O_WRONLY | O_CLOEXEC);
+ EXPECT_EQ(-1, fd_write);
+ EXPECT_EQ(ETXTBSY, errno);
+
+ /* Drops O_DENY_WRITE. */
+ EXPECT_EQ(0, fcntl(fd_deny, F_SETFL, flags & ~O_DENY_WRITE));
+
+ /* The restriction is now gone. */
+ fd_write = open(reg_file_path, O_WRONLY | O_CLOEXEC);
+ EXPECT_LE(0, fd_write);
+ EXPECT_EQ(0, close(fd_write));
+
+ EXPECT_EQ(0, close(fd_deny));
+}
+
+static void test_deny_write_open(struct __test_metadata *_metadata,
+ const char *const path, int flags,
+ const int err_code)
+{
+ int fd;
+
+ flags |= O_CLOEXEC;
+
+ /* Do not block on pipes. */
+ if (path == fifo_path)
+ flags |= O_NONBLOCK;
+
+ fd = open(path, flags | O_RDONLY);
+ if (err_code) {
+ ASSERT_EQ(-1, fd)
+ {
+ TH_LOG("Successfully opened %s", path);
+ }
+ EXPECT_EQ(errno, err_code)
+ {
+ TH_LOG("Wrong error code for %s: %s", path,
+ strerror(errno));
+ }
+ } else {
+ ASSERT_LE(0, fd)
+ {
+ TH_LOG("Failed to open %s: %s", path, strerror(errno));
+ }
+ EXPECT_EQ(0, close(fd));
+ }
+}
+
+TEST_F(access, deny_write_type_open)
+{
+ test_deny_write_open(_metadata, reg_file_path, O_DENY_WRITE, 0);
+ test_deny_write_open(_metadata, dir_path, O_DENY_WRITE, EINVAL);
+ test_deny_write_open(_metadata, block_dev_path, O_DENY_WRITE, EINVAL);
+ test_deny_write_open(_metadata, char_dev_path, O_DENY_WRITE, EINVAL);
+ test_deny_write_open(_metadata, fifo_path, O_DENY_WRITE, EINVAL);
+}
+
+static void test_deny_write_fcntl(struct __test_metadata *_metadata,
+ const char *const path, int setfl,
+ const int err_code)
+{
+ int fd, ret;
+ int getfl, flags = O_CLOEXEC;
+
+ /* Do not block on pipes. */
+ if (path == fifo_path)
+ flags |= O_NONBLOCK;
+
+ fd = open(path, flags | O_RDONLY);
+ ASSERT_LE(0, fd)
+ {
+ TH_LOG("Failed to open %s: %s", path, strerror(errno));
+ }
+ getfl = fcntl(fd, F_GETFL);
+ ASSERT_NE(-1, getfl);
+ ret = fcntl(fd, F_SETFL, getfl | setfl);
+ if (err_code) {
+ ASSERT_EQ(-1, ret)
+ {
+ TH_LOG("Successfully updated flags for %s", path);
+ }
+ EXPECT_EQ(errno, err_code)
+ {
+ TH_LOG("Wrong error code for %s: %s", path,
+ strerror(errno));
+ }
+ } else {
+ ASSERT_LE(0, ret)
+ {
+ TH_LOG("Failed to update flags for %s: %s", path,
+ strerror(errno));
+ }
+ EXPECT_EQ(0, close(fd));
+ }
+}
+
+TEST_F(access, deny_write_type_fcntl)
+{
+ int flags;
+
+ test_deny_write_fcntl(_metadata, reg_file_path, O_DENY_WRITE, 0);
+ test_deny_write_fcntl(_metadata, dir_path, O_DENY_WRITE, EINVAL);
+ test_deny_write_fcntl(_metadata, block_dev_path, O_DENY_WRITE, EINVAL);
+ test_deny_write_fcntl(_metadata, char_dev_path, O_DENY_WRITE, EINVAL);
+ test_deny_write_fcntl(_metadata, fifo_path, O_DENY_WRITE, EINVAL);
+
+ flags = fcntl(self->socket_fds[0], F_GETFL);
+ ASSERT_NE(-1, flags);
+ EXPECT_EQ(-1,
+ fcntl(self->socket_fds[0], F_SETFL, flags | O_DENY_WRITE));
+ EXPECT_EQ(EINVAL, errno);
+
+ flags = fcntl(self->pipefd, F_GETFL);
+ ASSERT_NE(-1, flags);
+ EXPECT_EQ(-1, fcntl(self->pipefd, F_SETFL, flags | O_DENY_WRITE));
+ EXPECT_EQ(EINVAL, errno);
+}
+
+TEST_F(access, allow_write_type_fcntl)
+{
+ int flags;
+
+ test_deny_write_fcntl(_metadata, reg_file_path, 0, 0);
+ test_deny_write_fcntl(_metadata, dir_path, 0, 0);
+ test_deny_write_fcntl(_metadata, block_dev_path, 0, 0);
+ test_deny_write_fcntl(_metadata, char_dev_path, 0, 0);
+ test_deny_write_fcntl(_metadata, fifo_path, 0, 0);
+
+ flags = fcntl(self->socket_fds[0], F_GETFL);
+ ASSERT_NE(-1, flags);
+ EXPECT_EQ(0,
+ fcntl(self->socket_fds[0], F_SETFL, flags & ~O_DENY_WRITE));
+
+ flags = fcntl(self->pipefd, F_GETFL);
+ ASSERT_NE(-1, flags);
+ EXPECT_EQ(0, fcntl(self->pipefd, F_SETFL, flags & ~O_DENY_WRITE));
+}
+
/* clang-format off */
FIXTURE(secbits) {};
/* clang-format on */
--
2.50.1
^ permalink raw reply related
* [RFC PATCH v1 1/2] fs: Add O_DENY_WRITE
From: Mickaël Salaün @ 2025-08-22 17:07 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, Andy Lutomirski, Arnd Bergmann,
Christian Heimes, Dmitry Vyukov, Elliott Hughes, Fan Wu,
Florian Weimer, Jann Horn, Jeff Xu, Jonathan Corbet,
Jordan R Abrahams, Lakshmi Ramasubramanian, Luca Boccassi,
Matt Bobrowski, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Robert Waite, Roberto Sassu, Scott Shell, Steve Dower,
Steve Grubb, kernel-hardening, linux-api, linux-fsdevel,
linux-integrity, linux-kernel, linux-security-module,
Andy Lutomirski, Jeff Xu
In-Reply-To: <20250822170800.2116980-1-mic@digikod.net>
Add a new O_DENY_WRITE flag usable at open time and on opened file (e.g.
passed file descriptors). This changes the state of the opened file by
making it read-only until it is closed. The main use case is for script
interpreters to get the guarantee that script' content cannot be altered
while being read and interpreted. This is useful for generic distros
that may not have a write-xor-execute policy. See commit a5874fde3c08
("exec: Add a new AT_EXECVE_CHECK flag to execveat(2)")
Both execve(2) and the IOCTL to enable fsverity can already set this
property on files with deny_write_access(). This new O_DENY_WRITE make
it widely available. This is similar to what other OSs may provide
e.g., opening a file with only FILE_SHARE_READ on Windows.
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Jeff Xu <jeffxu@chromium.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge Hallyn <serge@hallyn.com>
Reported-by: Robert Waite <rowait@microsoft.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20250822170800.2116980-2-mic@digikod.net
---
fs/fcntl.c | 26 ++++++++++++++++++++++++--
fs/file_table.c | 2 ++
fs/namei.c | 6 ++++++
include/linux/fcntl.h | 2 +-
include/uapi/asm-generic/fcntl.h | 4 ++++
5 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 5598e4d57422..0c80c0fbc706 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -34,7 +34,8 @@
#include "internal.h"
-#define SETFL_MASK (O_APPEND | O_NONBLOCK | O_NDELAY | O_DIRECT | O_NOATIME)
+#define SETFL_MASK (O_APPEND | O_NONBLOCK | O_NDELAY | O_DIRECT | O_NOATIME | \
+ O_DENY_WRITE)
static int setfl(int fd, struct file * filp, unsigned int arg)
{
@@ -80,8 +81,29 @@ static int setfl(int fd, struct file * filp, unsigned int arg)
error = 0;
}
spin_lock(&filp->f_lock);
+
+ if (arg & O_DENY_WRITE) {
+ /* Only regular files. */
+ if (!S_ISREG(inode->i_mode)) {
+ error = -EINVAL;
+ goto unlock;
+ }
+
+ /* Only sets once. */
+ if (!(filp->f_flags & O_DENY_WRITE)) {
+ error = exe_file_deny_write_access(filp);
+ if (error)
+ goto unlock;
+ }
+ } else {
+ if (filp->f_flags & O_DENY_WRITE)
+ exe_file_allow_write_access(filp);
+ }
+
filp->f_flags = (arg & SETFL_MASK) | (filp->f_flags & ~SETFL_MASK);
filp->f_iocb_flags = iocb_flags(filp);
+
+unlock:
spin_unlock(&filp->f_lock);
out:
@@ -1158,7 +1180,7 @@ static int __init fcntl_init(void)
* Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
* is defined as O_NONBLOCK on some platforms and not on others.
*/
- BUILD_BUG_ON(20 - 1 /* for O_RDONLY being 0 */ !=
+ BUILD_BUG_ON(21 - 1 /* for O_RDONLY being 0 */ !=
HWEIGHT32(
(VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) |
__FMODE_EXEC));
diff --git a/fs/file_table.c b/fs/file_table.c
index 81c72576e548..6ba896b6a53f 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -460,6 +460,8 @@ static void __fput(struct file *file)
locks_remove_file(file);
security_file_release(file);
+ if (unlikely(file->f_flags & O_DENY_WRITE))
+ exe_file_allow_write_access(file);
if (unlikely(file->f_flags & FASYNC)) {
if (file->f_op->fasync)
file->f_op->fasync(-1, file, 0);
diff --git a/fs/namei.c b/fs/namei.c
index cd43ff89fbaa..366530bf937d 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3885,6 +3885,12 @@ static int do_open(struct nameidata *nd,
error = may_open(idmap, &nd->path, acc_mode, open_flag);
if (!error && !(file->f_mode & FMODE_OPENED))
error = vfs_open(&nd->path, file);
+ if (!error && (open_flag & O_DENY_WRITE)) {
+ if (S_ISREG(file_inode(file)->i_mode))
+ error = exe_file_deny_write_access(file);
+ else
+ error = -EINVAL;
+ }
if (!error)
error = security_file_post_open(file, op->acc_mode);
if (!error && do_truncate)
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index a332e79b3207..dad14101686f 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -10,7 +10,7 @@
(O_RDONLY | O_WRONLY | O_RDWR | O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC | \
O_APPEND | O_NDELAY | O_NONBLOCK | __O_SYNC | O_DSYNC | \
FASYNC | O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
- O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
+ O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE | O_DENY_WRITE)
/* List of all valid flags for the how->resolve argument: */
#define VALID_RESOLVE_FLAGS \
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 613475285643..facd9136f5af 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -91,6 +91,10 @@
/* a horrid kludge trying to make sure that this will fail on old kernels */
#define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)
+#ifndef O_DENY_WRITE
+#define O_DENY_WRITE 040000000
+#endif
+
#ifndef O_NDELAY
#define O_NDELAY O_NONBLOCK
#endif
--
2.50.1
^ permalink raw reply related
* [RFC PATCH v1 0/2] Add O_DENY_WRITE (complement AT_EXECVE_CHECK)
From: Mickaël Salaün @ 2025-08-22 17:07 UTC (permalink / raw)
To: Al Viro, Christian Brauner, Kees Cook, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, Andy Lutomirski, Arnd Bergmann,
Christian Heimes, Dmitry Vyukov, Elliott Hughes, Fan Wu,
Florian Weimer, Jann Horn, Jeff Xu, Jonathan Corbet,
Jordan R Abrahams, Lakshmi Ramasubramanian, Luca Boccassi,
Matt Bobrowski, Miklos Szeredi, Mimi Zohar, Nicolas Bouchinet,
Robert Waite, Roberto Sassu, Scott Shell, Steve Dower,
Steve Grubb, kernel-hardening, linux-api, linux-fsdevel,
linux-integrity, linux-kernel, linux-security-module
Hi,
Script interpreters can check if a file would be allowed to be executed
by the kernel using the new AT_EXECVE_CHECK flag. This approach works
well on systems with write-xor-execute policies, where scripts cannot
be modified by malicious processes. However, this protection may not be
available on more generic distributions.
The key difference between `./script.sh` and `sh script.sh` (when using
AT_EXECVE_CHECK) is that execve(2) prevents the script from being opened
for writing while it's being executed. To achieve parity, the kernel
should provide a mechanism for script interpreters to deny write access
during script interpretation. While interpreters can copy script content
into a buffer, a race condition remains possible after AT_EXECVE_CHECK.
This patch series introduces a new O_DENY_WRITE flag for use with
open*(2) and fcntl(2). Both interfaces are necessary since script
interpreters may receive either a file path or file descriptor. For
backward compatibility, open(2) with O_DENY_WRITE will not fail on
unsupported systems, while users requiring explicit support guarantees
can use openat2(2).
The check_exec.rst documentation and related examples do not mention this new
feature yet.
Regards,
Mickaël Salaün (2):
fs: Add O_DENY_WRITE
selftests/exec: Add O_DENY_WRITE tests
fs/fcntl.c | 26 ++-
fs/file_table.c | 2 +
fs/namei.c | 6 +
include/linux/fcntl.h | 2 +-
include/uapi/asm-generic/fcntl.h | 4 +
tools/testing/selftests/exec/check-exec.c | 219 ++++++++++++++++++++++
6 files changed, 256 insertions(+), 3 deletions(-)
base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
--
2.50.1
^ permalink raw reply
* Re: [PATCH] selftests: centralise maybe-unused definition in kselftest.h
From: Mickaël Salaün @ 2025-08-22 14:51 UTC (permalink / raw)
To: Bala-Vignesh-Reddy
Cc: akpm, shuah, gnoack, david, lorenzo.stoakes, Liam.Howlett, vbabka,
rppt, surenb, mhocko, davem, edumazet, kuba, pabeni, horms,
ming.lei, skhan, linux-kselftest, linux-kernel,
linux-security-module, linux-mm, netdev, linux-fsdevel,
linux-block
In-Reply-To: <20250821101159.2238-1-reddybalavignesh9979@gmail.com>
On Thu, Aug 21, 2025 at 03:41:59PM +0530, Bala-Vignesh-Reddy wrote:
> Several selftests subdirectories duplicated the define __maybe_unused,
> leading to redundant code. Moved to kselftest.h header and removed
> other definition.
>
> This addresses the duplication noted in the proc-pid-vm warning fix
>
> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
> Link:https://lore.kernel.org/lkml/20250820143954.33d95635e504e94df01930d0@linux-foundation.org/
>
> Signed-off-by: Bala-Vignesh-Reddy <reddybalavignesh9979@gmail.com>
Looks good for Landlock:
Acked-by: Mickaël Salaün <mic@digikod.net>
> ---
> tools/testing/selftests/kselftest.h | 4 ++++
> tools/testing/selftests/landlock/audit.h | 6 ++----
> tools/testing/selftests/landlock/common.h | 4 ----
> tools/testing/selftests/mm/pkey-helpers.h | 3 ---
> tools/testing/selftests/net/psock_lib.h | 4 ----
> tools/testing/selftests/perf_events/watermark_signal.c | 2 --
> tools/testing/selftests/proc/proc-pid-vm.c | 4 ----
> tools/testing/selftests/ublk/utils.h | 2 --
> 8 files changed, 6 insertions(+), 23 deletions(-)
>
> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
> index c3b6d2604b1e..661d31c4b558 100644
> --- a/tools/testing/selftests/kselftest.h
> +++ b/tools/testing/selftests/kselftest.h
> @@ -92,6 +92,10 @@
> #endif
> #define __printf(a, b) __attribute__((format(printf, a, b)))
>
> +#ifndef __maybe_unused
> +#define __maybe_unused __attribute__((__unused__))
> +#endif
> +
> /* counters */
> struct ksft_count {
> unsigned int ksft_pass;
> diff --git a/tools/testing/selftests/landlock/audit.h b/tools/testing/selftests/landlock/audit.h
> index b16986aa6442..02fd1393947a 100644
> --- a/tools/testing/selftests/landlock/audit.h
> +++ b/tools/testing/selftests/landlock/audit.h
> @@ -20,14 +20,12 @@
> #include <sys/time.h>
> #include <unistd.h>
>
> +#include "../kselftest.h"
> +
> #ifndef ARRAY_SIZE
> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> #endif
>
> -#ifndef __maybe_unused
> -#define __maybe_unused __attribute__((__unused__))
> -#endif
> -
> #define REGEX_LANDLOCK_PREFIX "^audit([0-9.:]\\+): domain=\\([0-9a-f]\\+\\)"
>
> struct audit_filter {
> diff --git a/tools/testing/selftests/landlock/common.h b/tools/testing/selftests/landlock/common.h
> index 88a3c78f5d98..9acecae36f51 100644
> --- a/tools/testing/selftests/landlock/common.h
> +++ b/tools/testing/selftests/landlock/common.h
> @@ -22,10 +22,6 @@
>
> #define TMP_DIR "tmp"
>
> -#ifndef __maybe_unused
> -#define __maybe_unused __attribute__((__unused__))
> -#endif
> -
We could explicitly include kselftest.h in this file, but it's already
included by kselftest_harness.h, so that's OK.
> /* TEST_F_FORK() should not be used for new tests. */
> #define TEST_F_FORK(fixture_name, test_name) TEST_F(fixture_name, test_name)
>
> diff --git a/tools/testing/selftests/mm/pkey-helpers.h b/tools/testing/selftests/mm/pkey-helpers.h
> index ea404f80e6cb..fa15f006fa68 100644
> --- a/tools/testing/selftests/mm/pkey-helpers.h
> +++ b/tools/testing/selftests/mm/pkey-helpers.h
> @@ -84,9 +84,6 @@ extern void abort_hooks(void);
> #ifndef noinline
> # define noinline __attribute__((noinline))
> #endif
> -#ifndef __maybe_unused
> -# define __maybe_unused __attribute__((__unused__))
> -#endif
>
> int sys_pkey_alloc(unsigned long flags, unsigned long init_val);
> int sys_pkey_free(unsigned long pkey);
> diff --git a/tools/testing/selftests/net/psock_lib.h b/tools/testing/selftests/net/psock_lib.h
> index 6e4fef560873..067265b0a554 100644
> --- a/tools/testing/selftests/net/psock_lib.h
> +++ b/tools/testing/selftests/net/psock_lib.h
> @@ -22,10 +22,6 @@
>
> #define PORT_BASE 8000
>
> -#ifndef __maybe_unused
> -# define __maybe_unused __attribute__ ((__unused__))
> -#endif
> -
> static __maybe_unused void pair_udp_setfilter(int fd)
> {
> /* the filter below checks for all of the following conditions that
> diff --git a/tools/testing/selftests/perf_events/watermark_signal.c b/tools/testing/selftests/perf_events/watermark_signal.c
> index e03fe1b9bba2..b3a72f0ac522 100644
> --- a/tools/testing/selftests/perf_events/watermark_signal.c
> +++ b/tools/testing/selftests/perf_events/watermark_signal.c
> @@ -17,8 +17,6 @@
>
> #include "../kselftest_harness.h"
>
> -#define __maybe_unused __attribute__((__unused__))
> -
> static int sigio_count;
>
> static void handle_sigio(int signum __maybe_unused,
> diff --git a/tools/testing/selftests/proc/proc-pid-vm.c b/tools/testing/selftests/proc/proc-pid-vm.c
> index 978cbcb3eb11..2a72d37ad008 100644
> --- a/tools/testing/selftests/proc/proc-pid-vm.c
> +++ b/tools/testing/selftests/proc/proc-pid-vm.c
> @@ -47,10 +47,6 @@
> #include <sys/resource.h>
> #include <linux/fs.h>
>
> -#ifndef __maybe_unused
> -#define __maybe_unused __attribute__((__unused__))
> -#endif
> -
> #include "../kselftest.h"
>
> static inline long sys_execveat(int dirfd, const char *pathname, char **argv, char **envp, int flags)
> diff --git a/tools/testing/selftests/ublk/utils.h b/tools/testing/selftests/ublk/utils.h
> index 36545d1567f1..a852e0b7153e 100644
> --- a/tools/testing/selftests/ublk/utils.h
> +++ b/tools/testing/selftests/ublk/utils.h
> @@ -2,8 +2,6 @@
> #ifndef KUBLK_UTILS_H
> #define KUBLK_UTILS_H
>
> -#define __maybe_unused __attribute__((unused))
> -
> #ifndef min
> #define min(a, b) ((a) < (b) ? (a) : (b))
> #endif
> --
> 2.43.0
>
>
^ permalink raw reply
* Re: LSM namespacing API
From: Casey Schaufler @ 2025-08-22 14:47 UTC (permalink / raw)
To: Paul Moore, Mickaël Salaün
Cc: linux-security-module, selinux, John Johansen, Stephen Smalley,
Maxime Bélair, Casey Schaufler
In-Reply-To: <CAHC9VhSS1K0Zsq_ULP4sK9Okwthd+CO3vUdVPAf+F8FKfZsVqQ@mail.gmail.com>
On 8/21/2025 7:14 PM, Paul Moore wrote:
> On Thu, Aug 21, 2025 at 6:00 AM Mickaël Salaün <mic@digikod.net> wrote:
>> On Tue, Aug 19, 2025 at 02:40:52PM -0400, Paul Moore wrote:
>>> On Tue, Aug 19, 2025 at 1:11 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>> The advantage of a clone flag is that the operation is atomic with
>>>> the other namespace flag based behaviors. Having a two step process
>>>>
>>>> clone(); lsm_set_self_attr(); - or -
>>>> lsm_set_self_attr(); clone();
>>>>
>>>> is going to lead to cases where neither order really works correctly.
>>> I was envisioning something that works similarly to LSM_ATTR_EXEC
>>> where the unshare isn't immediate, but rather happens at a future
>>> event. With LSM_ATTR_EXEC it happens at the next exec*(), with
>>> LSM_ATTR_UNSHARE I imagine it would happen at the next clone*().
>> The next unshare(2) would make more sense to me.
> That's definitely something to discuss. I've been fairly loose on
> that in the discussion thus far, but as things are starting to settle
> on the lsm_set_self_attr(2) approach as one API, we should start to
> clarify that.
>
>> This deferred operation could be requested with a flag in
>> lsm_config_system_policy(2) instead:
>> https://lore.kernel.org/r/20250709080220.110947-1-maxime.belair@canonical.com
> I want to keep the policy syscall work separate from the LSM namespace
> discussion as we don't want to require a policy load operation to
> create a new LSM namespace. I think it's probably okay if the policy
> syscall work were to be namespace aware so that an orchestrator could
> load a LSM policy into a LSM namespace other than it's own, but that
> is still not overly dependent on what we are discussing here (yes,
> maybe it is a little, but only just so).
Policy load and namespace manipulation *must* be kept separate. Smack
requires the ability to "load policy" at any time. Smack allows a process
to add "policy" to further restrict its own access, and does not require
a namespace change. There has been an implementation of namespaces for
Smack, but the developers disappeared quietly and sadly no one picked it
up. Introducing a requirement that LSMs support namespaces in order to
load policy beyond system initialization is a non-starter.
^ permalink raw reply
* Re: LSM namespacing API
From: John Johansen @ 2025-08-22 14:30 UTC (permalink / raw)
To: Paul Moore
Cc: Casey Schaufler, linux-security-module, selinux, Stephen Smalley
In-Reply-To: <CAHC9VhTE3cf80U0BRJLhYcWj9vc6+S_8fotrEHo7eRfFkLnveA@mail.gmail.com>
On 8/21/25 18:57, Paul Moore wrote:
> On Thu, Aug 21, 2025 at 3:23 AM John Johansen
> <john.johansen@canonical.com> wrote:
>> On 8/19/25 11:40, Paul Moore wrote:
>>> On Tue, Aug 19, 2025 at 1:11 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>>>
>>>> The advantage of a clone flag is that the operation is atomic with
>>>> the other namespace flag based behaviors. Having a two step process
>>>>
>>>> clone(); lsm_set_self_attr(); - or -
>>>> lsm_set_self_attr(); clone();
>>>>
>>>> is going to lead to cases where neither order really works correctly.
>>>
>>> I was envisioning something that works similarly to LSM_ATTR_EXEC
>>> where the unshare isn't immediate, but rather happens at a future
>>> event. With LSM_ATTR_EXEC it happens at the next exec*(), with
>>> LSM_ATTR_UNSHARE I imagine it would happen at the next clone*().
>>
>> I do think something like this is needed to deal well with the two
>> step process. Without it is fairly easy to get into situations
>> where you either need more permissions, than strictly necessary,
>> because of steps in between or as Casey says things just don't work
>> correctly.
>
> I think we're starting to all coalesce on this basic idea now, at
> least for creating new LSM namespace sets, that's good. As the only
> LSM that really has a namespace currently, would AppArmor be able to
> work within the lsm_set_self_attr(2) approach, or would you need
> something a bit different? If so, can you give us a basic idea of
> what AA would need to work?
>
>> There will need to be an additional call that allows entering a
>> namespace separately from clone/unshare, but that covers a different
>> use case.
>
> In this particular case I've been thinking of not allowing the same
> level of arbitrary LSM namespace composability, but rather limiting
> the caller to the set of LSM namespaces already configured for a given
> process, using the procfs/setns(2) mechanism. Does that work for your
> use case(s), or do you need more flexibility?
>
yes it should work, I think the LSM/security namespaces need to move
together. In fact I want even less arbitrary composability as I think
switching LSM namespaces should be able to force system namespace
changes as well.
Their are all kinds of potential security corner cases you have to
worry about when trying to move them independently.
^ permalink raw reply
* Re: LSM namespacing API
From: Paul Moore @ 2025-08-22 2:14 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Casey Schaufler, linux-security-module, selinux, John Johansen,
Stephen Smalley, Maxime Bélair
In-Reply-To: <20250820.xo0hee4Zeeyu@digikod.net>
On Thu, Aug 21, 2025 at 6:00 AM Mickaël Salaün <mic@digikod.net> wrote:
> On Tue, Aug 19, 2025 at 02:40:52PM -0400, Paul Moore wrote:
> > On Tue, Aug 19, 2025 at 1:11 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> > >
> > > The advantage of a clone flag is that the operation is atomic with
> > > the other namespace flag based behaviors. Having a two step process
> > >
> > > clone(); lsm_set_self_attr(); - or -
> > > lsm_set_self_attr(); clone();
> > >
> > > is going to lead to cases where neither order really works correctly.
> >
> > I was envisioning something that works similarly to LSM_ATTR_EXEC
> > where the unshare isn't immediate, but rather happens at a future
> > event. With LSM_ATTR_EXEC it happens at the next exec*(), with
> > LSM_ATTR_UNSHARE I imagine it would happen at the next clone*().
>
> The next unshare(2) would make more sense to me.
That's definitely something to discuss. I've been fairly loose on
that in the discussion thus far, but as things are starting to settle
on the lsm_set_self_attr(2) approach as one API, we should start to
clarify that.
> This deferred operation could be requested with a flag in
> lsm_config_system_policy(2) instead:
> https://lore.kernel.org/r/20250709080220.110947-1-maxime.belair@canonical.com
I want to keep the policy syscall work separate from the LSM namespace
discussion as we don't want to require a policy load operation to
create a new LSM namespace. I think it's probably okay if the policy
syscall work were to be namespace aware so that an orchestrator could
load a LSM policy into a LSM namespace other than it's own, but that
is still not overly dependent on what we are discussing here (yes,
maybe it is a little, but only just so).
--
paul-moore.com
^ permalink raw reply
* Re: LSM namespacing API
From: Paul Moore @ 2025-08-22 2:09 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Stephen Smalley, linux-security-module, selinux, John Johansen,
Maxime Bélair
In-Reply-To: <20250821.Ree4Liedeita@digikod.net>
On Thu, Aug 21, 2025 at 5:56 AM Mickaël Salaün <mic@digikod.net> wrote:
> On Wed, Aug 20, 2025 at 04:47:15PM -0400, Paul Moore wrote:
> > On Wed, Aug 20, 2025 at 10:44 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > On Tue, Aug 19, 2025 at 02:51:00PM -0400, Paul Moore wrote:
> > > > On Tue, Aug 19, 2025 at 1:47 PM Stephen Smalley
> > > > <stephen.smalley.work@gmail.com> wrote:
> >
> > ...
> >
> > > > Since we have an existing LSM namespace combination, with processes
> > > > running inside of it, it might be sufficient to simply support moving
> > > > into an existing LSM namespace set with setns(2) using only a pidfd
> > > > and a new CLONE_LSMNS flag (or similar, upstream might want this as
> > > > CLONE_NEWLSM). This would simply set the LSM namespace set for the
> > >
> > > Bike shedding but, I would prefer CLONE_NEWSEC or something without LSM
> > > because the goal is not to add a new LSM but a new "security" namespace.
> >
> > I disagree with your statement about the goal. In fact I would argue
> > that one of the goals is to explicitly *not* create a generic
> > "security" namespace. Defining a single, LSM-wide namespace, is
> > already an almost impossible task, extending it to become a generic
> > "security" namespace seems maddening.
>
> I didn't suggest a generic "security" namespace that would include
> non-LSM access checks, just using the name "security" instead of "LSM",
> but never mind.
>
> > > > setns(2) caller to match that of the target pidfd. We still wouldn't
> > > > want to support CLONE_LSMNS/CLONE_NEWLSM for clone*().
> > >
> > > Why making clone*() support this flag would be an issue?
> >
> > With the understanding that I'm not going to support a single LSM-wide
> > namespace (see my previous comments), we would need multiple flags for
>
> I'm confused about the goal of this thread... When I read namespace I
> think about the user space interface that enables to tie a set of
> processes to ambient kernel objects. I'm not suggesting to force all
> LSM to handle namespaces, but to have a unified user space interface
> (i.e. namespace flag, file descriptor...) that can be used by user space
> to request a new "context" that may or may not be used by running LSMs.
The goal of this thread is to hopefully define a set of APIs that
allow userspace to create new LSM namespace sets, and join existing
LSM namespace sets. We're not necessarily focused on any individual
LSM namespace concepts, beyond ensuring that the API provides enough
flexibility for the different concepts to be implemented.
> > clone*(), one for each LSM that wanted to implement a namespace.
>
> My understanding of this proposal was to create a LSM-wide namespace,
> and one of the reason was to avoid one namespace per LSM.
As I stated in my original email, perhaps not clearly enough, and
several times in the past, I have no interest in supporting a single
LSM-wide namespace at this point in time. Any LSM namespaces must be
done at the individual LSM layer, although I am supportive of an API
at the LSM framework layer to both help facilitate the individual LSM
namespaces and provide a better userspace interface.
--
paul-moore.com
^ permalink raw reply
* Re: LSM namespacing API
From: Paul Moore @ 2025-08-22 1:59 UTC (permalink / raw)
To: John Johansen; +Cc: Stephen Smalley, linux-security-module, selinux
In-Reply-To: <67e72960-c985-48e1-aaeb-a4286cc8508f@canonical.com>
On Thu, Aug 21, 2025 at 3:46 AM John Johansen
<john.johansen@canonical.com> wrote:
> On 8/19/25 10:47, Stephen Smalley wrote:
...
> > This is handled for other Linux namespaces by opening a pseudo file
> > under /proc/pid/ns and invoking setns(2), so not sure how we want to
> > do it.
>
> That is a possible interface, not one that I like, so I would like to
> explore other options first.
Fair enough, suggestions are definitely welcome :)
--
paul-moore.com
^ permalink raw reply
* Re: LSM namespacing API
From: Paul Moore @ 2025-08-22 1:57 UTC (permalink / raw)
To: John Johansen
Cc: Casey Schaufler, linux-security-module, selinux, Stephen Smalley
In-Reply-To: <7d4d60d6-fe50-4646-9f9b-0cb524aef3fb@canonical.com>
On Thu, Aug 21, 2025 at 3:23 AM John Johansen
<john.johansen@canonical.com> wrote:
> On 8/19/25 11:40, Paul Moore wrote:
> > On Tue, Aug 19, 2025 at 1:11 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >>
> >> The advantage of a clone flag is that the operation is atomic with
> >> the other namespace flag based behaviors. Having a two step process
> >>
> >> clone(); lsm_set_self_attr(); - or -
> >> lsm_set_self_attr(); clone();
> >>
> >> is going to lead to cases where neither order really works correctly.
> >
> > I was envisioning something that works similarly to LSM_ATTR_EXEC
> > where the unshare isn't immediate, but rather happens at a future
> > event. With LSM_ATTR_EXEC it happens at the next exec*(), with
> > LSM_ATTR_UNSHARE I imagine it would happen at the next clone*().
>
> I do think something like this is needed to deal well with the two
> step process. Without it is fairly easy to get into situations
> where you either need more permissions, than strictly necessary,
> because of steps in between or as Casey says things just don't work
> correctly.
I think we're starting to all coalesce on this basic idea now, at
least for creating new LSM namespace sets, that's good. As the only
LSM that really has a namespace currently, would AppArmor be able to
work within the lsm_set_self_attr(2) approach, or would you need
something a bit different? If so, can you give us a basic idea of
what AA would need to work?
> There will need to be an additional call that allows entering a
> namespace separately from clone/unshare, but that covers a different
> use case.
In this particular case I've been thinking of not allowing the same
level of arbitrary LSM namespace composability, but rather limiting
the caller to the set of LSM namespaces already configured for a given
process, using the procfs/setns(2) mechanism. Does that work for your
use case(s), or do you need more flexibility?
--
paul-moore.com
^ permalink raw reply
* Re: LSM namespacing API
From: Paul Moore @ 2025-08-22 1:50 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Stephen Smalley, linux-security-module, selinux, John Johansen
In-Reply-To: <aKaMVPbPrgUc7mtv@mail.hallyn.com>
On Wed, Aug 20, 2025 at 11:02 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> On Wed, Aug 20, 2025 at 10:35:42PM -0400, Paul Moore wrote:
> > On Wed, Aug 20, 2025 at 10:05 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> > > On Tue, Aug 19, 2025 at 02:51:00PM -0400, Paul Moore wrote:
> > > > On Tue, Aug 19, 2025 at 1:47 PM Stephen Smalley
> > > > <stephen.smalley.work@gmail.com> wrote:
> >
> > ...
> >
> > > > > Serge pointed out that we also will need an API to attach to an
> > > > > existing SELinux namespace, which I captured here:
> > > > > https://github.com/stephensmalley/selinuxns/issues/19
> > > > > This is handled for other Linux namespaces by opening a pseudo file
> > > > > under /proc/pid/ns and invoking setns(2), so not sure how we want to
> > > > > do it.
> > > >
> > > > One option would be to have a the LSM framework return a LSM namespace
> > > > "handle" for a given LSM using lsm_get_self_attr(2) and then do a
> > > > setns(2)-esque operation using lsm_set_self_attr(2) with that
> > > > "handle". We would need to figure out what would constitute a
> > > > "handle" but let's just mark that as TBD for now with this approach (I
> > > > think better options are available).
> > >
> > > The use case which would be complicated (not blocked) by this, is
> > >
> > > * a runtime creates a process p1
> > > * p1 unshares its lsm namespace
> > > * runtime forks a debug/admin process p2
> > > * p2 wants to enter p1's namespace
> > >
> > > Of course the runtime could work around it by, before relinquishing
> > > control of p1 to a new executable, returning the lsm_get_self_attr()
> > > data to over a pipe.
> > >
> > > Note I don't think we should support setting another task's namespace,
> > > only getting its namespace ID.
> > >
> > > > Since we have an existing LSM namespace combination, with processes
> > > > running inside of it, it might be sufficient to simply support moving
> > > > into an existing LSM namespace set with setns(2) using only a pidfd
> > > > and a new CLONE_LSMNS flag (or similar, upstream might want this as
> > > > CLONE_NEWLSM). This would simply set the LSM namespace set for the
> > > > setns(2) caller to match that of the target pidfd. We still wouldn't
> > > > want to support CLONE_LSMNS/CLONE_NEWLSM for clone*().
> > >
> > > A part of me is telling (another part of) me that being able to setns
> > > to a subset of the lsms could lead to privilege escapes through
> > > weird policy configurations for the various LSMs. In which case,
> > > an all-or-nothing LSM setns might actually be preferable.
> >
> > Sorry I probably wasn't as clear as I should have been, but my idea
> > with using the existing procfs/setns(2) approach with a single
> > CLONE_NEWLSM (name pending sufficient bikeshedding) was that the
> > process being setns()'d would simply end up in the exact copy of the
> > target process' LSM namespace configuration, it shouldn't be a new
>
> Oh, I think I was being unclear - I thought the first option, using
> lsm_set_self_attr(), would allow choosing a subset of LSMs to setns to.
> In contrast, the pure setns with a single flag is less flexible, but
> possibly safer. So I typed there the result of my train of thought,
> which is that your second suggestion is probably preferable.
I think we've probably both been a bit off :) Let me try again ...
I'm proposing the lsm_set_self_attr(2) approach as a way for a process
to setup an arbitrary set of LSM namespaces to take effect on an
upcoming clone() or exec() (we can discuss that detail). I didn't
originally envision this as a way to potentially join existing LSM
namespaces, but rather a way to create new LSM namespaces when a new
process is created/exec'd.
The procfs/setns(2) approach would be in addition to the
lsm_set_self_attr(2) mechanism, and would allow a process to enter a
previously configured LSM namespace set when a CLONE_LSMNS (or
similar) flag was passed to setns(2).
Both mechanisms are very much up for debate in my mind, and doing
either or both, is possible as far as I'm concerned.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] selftests: centralise maybe-unused definition in kselftest.h
From: Ming Lei @ 2025-08-22 1:21 UTC (permalink / raw)
To: Bala-Vignesh-Reddy
Cc: akpm, shuah, mic, gnoack, david, lorenzo.stoakes, Liam.Howlett,
vbabka, rppt, surenb, mhocko, davem, edumazet, kuba, pabeni,
horms, skhan, linux-kselftest, linux-kernel,
linux-security-module, linux-mm, netdev, linux-fsdevel,
linux-block
In-Reply-To: <20250821101159.2238-1-reddybalavignesh9979@gmail.com>
On Thu, Aug 21, 2025 at 03:41:59PM +0530, Bala-Vignesh-Reddy wrote:
> Several selftests subdirectories duplicated the define __maybe_unused,
> leading to redundant code. Moved to kselftest.h header and removed
> other definition.
>
> This addresses the duplication noted in the proc-pid-vm warning fix
>
> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
> Link:https://lore.kernel.org/lkml/20250820143954.33d95635e504e94df01930d0@linux-foundation.org/
>
> Signed-off-by: Bala-Vignesh-Reddy <reddybalavignesh9979@gmail.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Thanks,
Ming
^ permalink raw reply
* [PATCH v17] exec: Fix dead-lock in de_thread with ptrace_attach
From: Bernd Edlinger @ 2025-08-21 17:34 UTC (permalink / raw)
To: Alexander Viro, Alexey Dobriyan, Oleg Nesterov, Kees Cook,
Andy Lutomirski, Will Drewry, Christian Brauner, Andrew Morton,
Michal Hocko, Serge Hallyn, James Morris, Randy Dunlap,
Suren Baghdasaryan, Yafang Shao, Helge Deller, Eric W. Biederman,
Adrian Reber, Thomas Gleixner, Jens Axboe, Alexei Starovoitov,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest, linux-mm, linux-security-module, tiozhang,
Luis Chamberlain, Paulo Alcantara (SUSE), Sergey Senozhatsky,
Frederic Weisbecker, YueHaibing, Paul Moore, Aleksa Sarai,
Stefan Roesch, Chao Yu, xu xin, Jeff Layton, Jan Kara,
David Hildenbrand, Dave Chinner, Shuah Khan, Alexey Dobriyan,
Jens Axboe, Paul Moore, Elena Reshetova, David Windsor,
Mateusz Guzik, YueHaibing, Ard Biesheuvel,
Joel Fernandes (Google), Matthew Wilcox (Oracle),
Hans Liljestrand, tiozhang, Penglei Jiang, Lorenzo Stoakes,
Adrian Ratiu, Ingo Molnar, Peter Zijlstra (Intel),
Cyrill Gorcunov, Eric Dumazet
In-Reply-To: <GV2PPF74270EBEEEDE0B9742310DE91E9A7E431A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
This introduces signal->exec_bprm, which is used to
fix the case when at least one of the sibling threads
is traced, and therefore the trace process may dead-lock
in ptrace_attach, but de_thread will need to wait for the
tracer to continue execution.
The problem happens when a tracer tries to ptrace_attach
to a multi-threaded process, that does an execve in one of
the threads at the same time, without doing that in a forked
sub-process. That means: There is a race condition, when one
or more of the threads are already ptraced, but the thread
that invoked the execve is not yet traced. Now in this
case the execve locks the cred_guard_mutex and waits for
de_thread to complete. But that waits for the traced
sibling threads to exit, and those have to wait for the
tracer to receive the exit signal, but the tracer cannot
call wait right now, because it is waiting for the ptrace
call to complete, and this never does not happen.
The traced process and the tracer are now in a deadlock
situation, and can only be killed by a fatal signal.
The solution is to detect this situation and allow
ptrace_attach to continue by temporarily releasing the
cred_guard_mutex, while de_thread() is still waiting for
traced zombies to be eventually released by the tracer.
In the case of the thread group leader we only have to wait
for the thread to become a zombie, which may also need
co-operation from the tracer due to PTRACE_O_TRACEEXIT.
When a tracer wants to ptrace_attach a task that already
is in execve, we simply retry the ptrace_may_access
check while temporarily installing the new credentials
and dumpability which are about to be used after execve
completes. If the ptrace_attach happens on a thread that
is a sibling-thread of the thread doing execve, it is
sufficient to check against the old credentials, as this
thread will be waited for, before the new credentials are
installed.
Other threads die quickly since the cred_guard_mutex is
released, but a deadly signal is already pending. In case
the mutex_lock_killable misses the signal, the non-zero
current->signal->exec_bprm makes sure they release the
mutex immediately and return with -ERESTARTNOINTR.
This means there is no API change, unlike the previous
version of this patch which was discussed here:
https://lore.kernel.org/lkml/b6537ae6-31b1-5c50-f32b-8b8332ace882@hotmail.de/
See tools/testing/selftests/ptrace/vmaccess.c
for a test case that gets fixed by this change.
Note that since the test case was originally designed to
test the ptrace_attach returning an error in this situation,
the test expectation needed to be adjusted, to allow the
API to succeed at the first attempt.
Signed-off-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
---
fs/exec.c | 69 ++++++++---
fs/proc/base.c | 6 +
include/linux/cred.h | 1 +
include/linux/sched/signal.h | 18 +++
kernel/cred.c | 30 ++++-
kernel/ptrace.c | 32 +++++
kernel/seccomp.c | 12 +-
tools/testing/selftests/ptrace/vmaccess.c | 135 ++++++++++++++++++++--
8 files changed, 266 insertions(+), 37 deletions(-)
v10: Changes to previous version, make the PTRACE_ATTACH
return -EAGAIN, instead of execve return -ERESTARTSYS.
Added some lessions learned to the description.
v11: Check old and new credentials in PTRACE_ATTACH again without
changing the API.
Note: I got actually one response from an automatic checker to the v11 patch,
https://lore.kernel.org/lkml/202107121344.wu68hEPF-lkp@intel.com/
which is complaining about:
>> >> kernel/ptrace.c:425:26: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cred const *old_cred @@ got struct cred const [noderef] __rcu *real_cred @@
417 struct linux_binprm *bprm = task->signal->exec_bprm;
418 const struct cred *old_cred;
419 struct mm_struct *old_mm;
420
421 retval = down_write_killable(&task->signal->exec_update_lock);
422 if (retval)
423 goto unlock_creds;
424 task_lock(task);
> 425 old_cred = task->real_cred;
v12: Essentially identical to v11.
- Fixed a minor merge conflict in linux v5.17, and fixed the
above mentioned nit by adding __rcu to the declaration.
- re-tested the patch with all linux versions from v5.11 to v6.6
v10 was an alternative approach which did imply an API change.
But I would prefer to avoid such an API change.
The difficult part is getting the right dumpability flags assigned
before de_thread starts, hope you like this version.
If not, the v10 is of course also acceptable.
v13: Fixed duplicated Return section in function header of
is_dumpability_changed which was reported by the kernel test robot
v14: rebased to v6.7, refreshed and retested.
And added a more detailed description of the actual bug.
v15: rebased to v6.8-rc1, addressed some review comments.
Split the test case vmaccess into vmaccess1 and vmaccess2
to improve overall test coverage.
v16: rebased to 6.17-rc2, fixed some minor merge conflicts.
v17: avoid use of task->in_execve in ptrace_attach.
Thanks
Bernd.
diff --git a/fs/exec.c b/fs/exec.c
index 2a1e5e4042a1..31c6ceaa5f69 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -905,11 +905,13 @@ static int exec_mmap(struct mm_struct *mm)
return 0;
}
-static int de_thread(struct task_struct *tsk)
+static int de_thread(struct task_struct *tsk, struct linux_binprm *bprm)
{
struct signal_struct *sig = tsk->signal;
struct sighand_struct *oldsighand = tsk->sighand;
spinlock_t *lock = &oldsighand->siglock;
+ struct task_struct *t;
+ bool unsafe_execve_in_progress = false;
if (thread_group_empty(tsk))
goto no_thread_group;
@@ -932,6 +934,19 @@ static int de_thread(struct task_struct *tsk)
if (!thread_group_leader(tsk))
sig->notify_count--;
+ for_other_threads(tsk, t) {
+ if (unlikely(t->ptrace)
+ && (t != tsk->group_leader || !t->exit_state))
+ unsafe_execve_in_progress = true;
+ }
+
+ if (unlikely(unsafe_execve_in_progress)) {
+ spin_unlock_irq(lock);
+ sig->exec_bprm = bprm;
+ mutex_unlock(&sig->cred_guard_mutex);
+ spin_lock_irq(lock);
+ }
+
while (sig->notify_count) {
__set_current_state(TASK_KILLABLE);
spin_unlock_irq(lock);
@@ -1021,6 +1036,11 @@ static int de_thread(struct task_struct *tsk)
release_task(leader);
}
+ if (unlikely(unsafe_execve_in_progress)) {
+ mutex_lock(&sig->cred_guard_mutex);
+ sig->exec_bprm = NULL;
+ }
+
sig->group_exec_task = NULL;
sig->notify_count = 0;
@@ -1032,6 +1052,11 @@ static int de_thread(struct task_struct *tsk)
return 0;
killed:
+ if (unlikely(unsafe_execve_in_progress)) {
+ mutex_lock(&sig->cred_guard_mutex);
+ sig->exec_bprm = NULL;
+ }
+
/* protects against exit_notify() and __exit_signal() */
read_lock(&tasklist_lock);
sig->group_exec_task = NULL;
@@ -1114,13 +1139,31 @@ int begin_new_exec(struct linux_binprm * bprm)
*/
trace_sched_prepare_exec(current, bprm);
+ /* If the binary is not readable then enforce mm->dumpable=0 */
+ would_dump(bprm, bprm->file);
+ if (bprm->have_execfd)
+ would_dump(bprm, bprm->executable);
+
+ /*
+ * Figure out dumpability. Note that this checking only of current
+ * is wrong, but userspace depends on it. This should be testing
+ * bprm->secureexec instead.
+ */
+ if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
+ is_dumpability_changed(current_cred(), bprm->cred) ||
+ !(uid_eq(current_euid(), current_uid()) &&
+ gid_eq(current_egid(), current_gid())))
+ set_dumpable(bprm->mm, suid_dumpable);
+ else
+ set_dumpable(bprm->mm, SUID_DUMP_USER);
+
/*
* Ensure all future errors are fatal.
*/
bprm->point_of_no_return = true;
/* Make this the only thread in the thread group */
- retval = de_thread(me);
+ retval = de_thread(me, bprm);
if (retval)
goto out;
/* see the comment in check_unsafe_exec() */
@@ -1144,11 +1187,6 @@ int begin_new_exec(struct linux_binprm * bprm)
if (retval)
goto out;
- /* If the binary is not readable then enforce mm->dumpable=0 */
- would_dump(bprm, bprm->file);
- if (bprm->have_execfd)
- would_dump(bprm, bprm->executable);
-
/*
* Release all of the old mmap stuff
*/
@@ -1210,18 +1248,6 @@ int begin_new_exec(struct linux_binprm * bprm)
me->sas_ss_sp = me->sas_ss_size = 0;
- /*
- * Figure out dumpability. Note that this checking only of current
- * is wrong, but userspace depends on it. This should be testing
- * bprm->secureexec instead.
- */
- if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP ||
- !(uid_eq(current_euid(), current_uid()) &&
- gid_eq(current_egid(), current_gid())))
- set_dumpable(current->mm, suid_dumpable);
- else
- set_dumpable(current->mm, SUID_DUMP_USER);
-
perf_event_exec();
/*
@@ -1361,6 +1387,11 @@ static int prepare_bprm_creds(struct linux_binprm *bprm)
if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
return -ERESTARTNOINTR;
+ if (unlikely(current->signal->exec_bprm)) {
+ mutex_unlock(¤t->signal->cred_guard_mutex);
+ return -ERESTARTNOINTR;
+ }
+
bprm->cred = prepare_exec_creds();
if (likely(bprm->cred))
return 0;
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 62d35631ba8c..e5bcf812cee0 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2838,6 +2838,12 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
if (rv < 0)
goto out_free;
+ if (unlikely(current->signal->exec_bprm)) {
+ mutex_unlock(¤t->signal->cred_guard_mutex);
+ rv = -ERESTARTNOINTR;
+ goto out_free;
+ }
+
rv = security_setprocattr(PROC_I(inode)->op.lsmid,
file->f_path.dentry->d_name.name, page,
count);
diff --git a/include/linux/cred.h b/include/linux/cred.h
index a102a10f833f..fb0361911489 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -153,6 +153,7 @@ extern const struct cred *get_task_cred(struct task_struct *);
extern struct cred *cred_alloc_blank(void);
extern struct cred *prepare_creds(void);
extern struct cred *prepare_exec_creds(void);
+extern bool is_dumpability_changed(const struct cred *, const struct cred *);
extern int commit_creds(struct cred *);
extern void abort_creds(struct cred *);
extern struct cred *prepare_kernel_cred(struct task_struct *);
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 1ef1edbaaf79..3c47d8b55863 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -237,9 +237,27 @@ struct signal_struct {
struct mm_struct *oom_mm; /* recorded mm when the thread group got
* killed by the oom killer */
+ struct linux_binprm *exec_bprm; /* Used to check ptrace_may_access
+ * against new credentials while
+ * de_thread is waiting for other
+ * traced threads to terminate.
+ * Set while de_thread is executing.
+ * The cred_guard_mutex is released
+ * after de_thread() has called
+ * zap_other_threads(), therefore
+ * a fatal signal is guaranteed to be
+ * already pending in the unlikely
+ * event, that
+ * current->signal->exec_bprm happens
+ * to be non-zero after the
+ * cred_guard_mutex was acquired.
+ */
+
struct mutex cred_guard_mutex; /* guard against foreign influences on
* credential calculations
* (notably. ptrace)
+ * Held while execve runs, except when
+ * a sibling thread is being traced.
* Deprecated do not use in new code.
* Use exec_update_lock instead.
*/
diff --git a/kernel/cred.c b/kernel/cred.c
index 9676965c0981..0b2822c762df 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -375,6 +375,30 @@ static bool cred_cap_issubset(const struct cred *set, const struct cred *subset)
return false;
}
+/**
+ * is_dumpability_changed - Will changing creds affect dumpability?
+ * @old: The old credentials.
+ * @new: The new credentials.
+ *
+ * If the @new credentials have no elevated privileges compared to the
+ * @old credentials, the task may remain dumpable. Otherwise we have
+ * to mark the task as undumpable to avoid information leaks from higher
+ * to lower privilege domains.
+ *
+ * Return: True if the task will become undumpable.
+ */
+bool is_dumpability_changed(const struct cred *old, const struct cred *new)
+{
+ if (!uid_eq(old->euid, new->euid) ||
+ !gid_eq(old->egid, new->egid) ||
+ !uid_eq(old->fsuid, new->fsuid) ||
+ !gid_eq(old->fsgid, new->fsgid) ||
+ !cred_cap_issubset(old, new))
+ return true;
+
+ return false;
+}
+
/**
* commit_creds - Install new credentials upon the current task
* @new: The credentials to be assigned
@@ -403,11 +427,7 @@ int commit_creds(struct cred *new)
get_cred(new); /* we will require a ref for the subj creds too */
/* dumpability changes */
- if (!uid_eq(old->euid, new->euid) ||
- !gid_eq(old->egid, new->egid) ||
- !uid_eq(old->fsuid, new->fsuid) ||
- !gid_eq(old->fsgid, new->fsgid) ||
- !cred_cap_issubset(old, new)) {
+ if (is_dumpability_changed(old, new)) {
if (task->mm)
set_dumpable(task->mm, suid_dumpable);
task->pdeath_signal = 0;
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 75a84efad40f..230298817dbf 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -20,6 +20,7 @@
#include <linux/pagemap.h>
#include <linux/ptrace.h>
#include <linux/security.h>
+#include <linux/binfmts.h>
#include <linux/signal.h>
#include <linux/uio.h>
#include <linux/audit.h>
@@ -453,6 +454,28 @@ static int ptrace_attach(struct task_struct *task, long request,
return retval;
}
+ if (unlikely(task == task->signal->group_exec_task)) {
+ retval = down_write_killable(&task->signal->exec_update_lock);
+ if (retval)
+ return retval;
+
+ scoped_guard (task_lock, task) {
+ struct linux_binprm *bprm = task->signal->exec_bprm;
+ const struct cred __rcu *old_cred = task->real_cred;
+ struct mm_struct *old_mm = task->mm;
+
+ rcu_assign_pointer(task->real_cred, bprm->cred);
+ task->mm = bprm->mm;
+ retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
+ rcu_assign_pointer(task->real_cred, old_cred);
+ task->mm = old_mm;
+ }
+
+ up_write(&task->signal->exec_update_lock);
+ if (retval)
+ return retval;
+ }
+
scoped_guard (write_lock_irq, &tasklist_lock) {
if (unlikely(task->exit_state))
return -EPERM;
@@ -488,6 +511,14 @@ static int ptrace_traceme(void)
{
int ret = -EPERM;
+ if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
+ return -ERESTARTNOINTR;
+
+ if (unlikely(current->signal->exec_bprm)) {
+ mutex_unlock(¤t->signal->cred_guard_mutex);
+ return -ERESTARTNOINTR;
+ }
+
write_lock_irq(&tasklist_lock);
/* Are we already being traced? */
if (!current->ptrace) {
@@ -503,6 +534,7 @@ static int ptrace_traceme(void)
}
}
write_unlock_irq(&tasklist_lock);
+ mutex_unlock(¤t->signal->cred_guard_mutex);
return ret;
}
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 41aa761c7738..d61fc275235a 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -1994,9 +1994,15 @@ static long seccomp_set_mode_filter(unsigned int flags,
* Make sure we cannot change seccomp or nnp state via TSYNC
* while another thread is in the middle of calling exec.
*/
- if (flags & SECCOMP_FILTER_FLAG_TSYNC &&
- mutex_lock_killable(¤t->signal->cred_guard_mutex))
- goto out_put_fd;
+ if (flags & SECCOMP_FILTER_FLAG_TSYNC) {
+ if (mutex_lock_killable(¤t->signal->cred_guard_mutex))
+ goto out_put_fd;
+
+ if (unlikely(current->signal->exec_bprm)) {
+ mutex_unlock(¤t->signal->cred_guard_mutex);
+ goto out_put_fd;
+ }
+ }
spin_lock_irq(¤t->sighand->siglock);
diff --git a/tools/testing/selftests/ptrace/vmaccess.c b/tools/testing/selftests/ptrace/vmaccess.c
index 4db327b44586..5d4a65eb5a8d 100644
--- a/tools/testing/selftests/ptrace/vmaccess.c
+++ b/tools/testing/selftests/ptrace/vmaccess.c
@@ -14,6 +14,7 @@
#include <signal.h>
#include <unistd.h>
#include <sys/ptrace.h>
+#include <sys/syscall.h>
static void *thread(void *arg)
{
@@ -23,7 +24,7 @@ static void *thread(void *arg)
TEST(vmaccess)
{
- int f, pid = fork();
+ int s, f, pid = fork();
char mm[64];
if (!pid) {
@@ -31,19 +32,42 @@ TEST(vmaccess)
pthread_create(&pt, NULL, thread, NULL);
pthread_join(pt, NULL);
- execlp("true", "true", NULL);
+ execlp("false", "false", NULL);
+ return;
}
sleep(1);
sprintf(mm, "/proc/%d/mem", pid);
+ /* deadlock did happen here */
f = open(mm, O_RDONLY);
ASSERT_GE(f, 0);
close(f);
- f = kill(pid, SIGCONT);
- ASSERT_EQ(f, 0);
+ f = waitpid(-1, &s, WNOHANG);
+ ASSERT_NE(f, -1);
+ ASSERT_NE(f, 0);
+ ASSERT_NE(f, pid);
+ ASSERT_EQ(WIFEXITED(s), 1);
+ ASSERT_EQ(WEXITSTATUS(s), 0);
+ f = waitpid(-1, &s, 0);
+ ASSERT_EQ(f, pid);
+ ASSERT_EQ(WIFEXITED(s), 1);
+ ASSERT_EQ(WEXITSTATUS(s), 1);
+ f = waitpid(-1, NULL, 0);
+ ASSERT_EQ(f, -1);
+ ASSERT_EQ(errno, ECHILD);
}
-TEST(attach)
+/*
+ * Same test as previous, except that
+ * we try to ptrace the group leader,
+ * which is about to call execve,
+ * when the other thread is already ptraced.
+ * This exercises the code in de_thread
+ * where it is waiting inside the
+ * while (sig->notify_count) {
+ * loop.
+ */
+TEST(attach1)
{
int s, k, pid = fork();
@@ -52,19 +76,76 @@ TEST(attach)
pthread_create(&pt, NULL, thread, NULL);
pthread_join(pt, NULL);
- execlp("sleep", "sleep", "2", NULL);
+ execlp("false", "false", NULL);
+ return;
}
sleep(1);
+ /* deadlock may happen here */
k = ptrace(PTRACE_ATTACH, pid, 0L, 0L);
- ASSERT_EQ(errno, EAGAIN);
- ASSERT_EQ(k, -1);
+ ASSERT_EQ(k, 0);
k = waitpid(-1, &s, WNOHANG);
ASSERT_NE(k, -1);
ASSERT_NE(k, 0);
ASSERT_NE(k, pid);
ASSERT_EQ(WIFEXITED(s), 1);
ASSERT_EQ(WEXITSTATUS(s), 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGTRAP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFEXITED(s), 1);
+ ASSERT_EQ(WEXITSTATUS(s), 1);
+ k = waitpid(-1, NULL, 0);
+ ASSERT_EQ(k, -1);
+ ASSERT_EQ(errno, ECHILD);
+}
+
+/*
+ * Same test as previous, except that
+ * the group leader is ptraced first,
+ * but this time with PTRACE_O_TRACEEXIT,
+ * and the thread that does execve is
+ * not yet ptraced. This exercises the
+ * code block in de_thread where the
+ * if (!thread_group_leader(tsk)) {
+ * is executed and enters a wait state.
+ */
+static long thread2_tid;
+static void *thread2(void *arg)
+{
+ thread2_tid = syscall(__NR_gettid);
+ sleep(2);
+ execlp("false", "false", NULL);
+ return NULL;
+}
+
+TEST(attach2)
+{
+ int s, k, pid = fork();
+
+ if (!pid) {
+ pthread_t pt;
+
+ pthread_create(&pt, NULL, thread2, NULL);
+ pthread_join(pt, NULL);
+ return;
+ }
+
sleep(1);
k = ptrace(PTRACE_ATTACH, pid, 0L, 0L);
ASSERT_EQ(k, 0);
@@ -72,12 +153,46 @@ TEST(attach)
ASSERT_EQ(k, pid);
ASSERT_EQ(WIFSTOPPED(s), 1);
ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
- k = ptrace(PTRACE_DETACH, pid, 0L, 0L);
+ k = ptrace(PTRACE_SETOPTIONS, pid, 0L, PTRACE_O_TRACEEXIT);
+ ASSERT_EQ(k, 0);
+ thread2_tid = ptrace(PTRACE_PEEKDATA, pid, &thread2_tid, 0L);
+ ASSERT_NE(thread2_tid, -1);
+ ASSERT_NE(thread2_tid, 0);
+ ASSERT_NE(thread2_tid, pid);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ sleep(2);
+ /* deadlock may happen here */
+ k = ptrace(PTRACE_ATTACH, thread2_tid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGTRAP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGTRAP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
+ ASSERT_EQ(k, 0);
+ k = waitpid(-1, &s, 0);
+ ASSERT_EQ(k, pid);
+ ASSERT_EQ(WIFSTOPPED(s), 1);
+ ASSERT_EQ(WSTOPSIG(s), SIGSTOP);
+ k = waitpid(-1, &s, WNOHANG);
+ ASSERT_EQ(k, 0);
+ k = ptrace(PTRACE_CONT, pid, 0L, 0L);
ASSERT_EQ(k, 0);
k = waitpid(-1, &s, 0);
ASSERT_EQ(k, pid);
ASSERT_EQ(WIFEXITED(s), 1);
- ASSERT_EQ(WEXITSTATUS(s), 0);
+ ASSERT_EQ(WEXITSTATUS(s), 1);
k = waitpid(-1, NULL, 0);
ASSERT_EQ(k, -1);
ASSERT_EQ(errno, ECHILD);
--
2.39.5
^ permalink raw reply related
* Re: [PATCH] selftests: centralise maybe-unused definition in kselftest.h
From: SeongJae Park @ 2025-08-21 16:56 UTC (permalink / raw)
To: Bala-Vignesh-Reddy
Cc: SeongJae Park, akpm, shuah, mic, gnoack, david, lorenzo.stoakes,
Liam.Howlett, vbabka, rppt, surenb, mhocko, davem, edumazet, kuba,
pabeni, horms, ming.lei, skhan, linux-kselftest, linux-kernel,
linux-security-module, linux-mm, netdev, linux-fsdevel,
linux-block
In-Reply-To: <20250821101159.2238-1-reddybalavignesh9979@gmail.com>
On Thu, 21 Aug 2025 15:41:59 +0530 Bala-Vignesh-Reddy <reddybalavignesh9979@gmail.com> wrote:
> Several selftests subdirectories duplicated the define __maybe_unused,
> leading to redundant code. Moved to kselftest.h header and removed
> other definition.
>
> This addresses the duplication noted in the proc-pid-vm warning fix
>
> Suggested-by: Andrew Morton <akpm@linux-foundation.org>
> Link:https://lore.kernel.org/lkml/20250820143954.33d95635e504e94df01930d0@linux-foundation.org/
>
> Signed-off-by: Bala-Vignesh-Reddy <reddybalavignesh9979@gmail.com>
Acked-by: SeongJae Park <sj@kernel.org>
Thanks,
SJ
[...]
^ permalink raw reply
* Re: LSM namespacing API
From: John Johansen @ 2025-08-21 14:57 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Stephen Smalley, Paul Moore, linux-security-module, selinux
In-Reply-To: <aKcskclwVVe1X4kP@mail.hallyn.com>
On 8/21/25 07:26, Serge E. Hallyn wrote:
> On Thu, Aug 21, 2025 at 12:46:10AM -0700, John Johansen wrote:
>> On 8/19/25 10:47, Stephen Smalley wrote:
>>> On Tue, Aug 19, 2025 at 10:56 AM Paul Moore <paul@paul-moore.com> wrote:
>>>>
>>>> Hello all,
>>>>
>>>> As most of you are likely aware, Stephen Smalley has been working on
>>>> adding namespace support to SELinux, and the work has now progressed
>>>> to the point where a serious discussion on the API is warranted. For
>>>> those of you are unfamiliar with the details or Stephen's patchset, or
>>>> simply need a refresher, he has some excellent documentation in his
>>>> work-in-progress repo:
>>>>
>>>> * https://github.com/stephensmalley/selinuxns
>>>>
>>>> Stephen also gave a (pre-recorded) presentation at LSS-NA this year
>>>> about SELinux namespacing, you can watch the presentation here:
>>>>
>>>> * https://www.youtube.com/watch?v=AwzGCOwxLoM
>>>>
>>>> In the past you've heard me state, rather firmly at times, that I
>>>> believe namespacing at the LSM framework layer to be a mistake,
>>>> although if there is something that can be done to help facilitate the
>>>> namespacing of individual LSMs at the framework layer, I would be
>>>> supportive of that. I think that a single LSM namespace API, similar
>>>> to our recently added LSM syscalls, may be such a thing, so I'd like
>>>> us to have a discussion to see if we all agree on that, and if so,
>>>> what such an API might look like.
>>>>
>>>> At LSS-NA this year, John Johansen and I had a brief discussion where
>>>> he suggested a single LSM wide clone*(2) flag that individual LSM's
>>>> could opt into via callbacks. John is directly CC'd on this mail, so
>>>> I'll let him expand on this idea.
>>>>
>>>> While I agree with John that a fs based API is problematic (see all of
>>>> our discussions around the LSM syscalls), I'm concerned that a single
>>>> clone*(2) flag will significantly limit our flexibility around how
>>>> individual LSMs are namespaced, something I don't want to see happen.
>>>> This makes me wonder about the potential for expanding
>>>> lsm_set_self_attr(2) to support a new LSM attribute that would support
>>>> a namespace "unshare" operation, e.g. LSM_ATTR_UNSHARE. This would
>>>> provide a single LSM framework API for an unshare operation while also
>>>> providing a mechanism to pass LSM specific via the lsm_ctx struct if
>>>> needed. Just as we do with the other LSM_ATTR_* flags today,
>>>> individual LSMs can opt-in to the API fairly easily by providing a
>>>> setselfattr() LSM callback.
>>>>
>>>> Thoughts?
>>>
>>> I think we want to be able to unshare a specific security module
>>> namespace without unsharing the others, i.e. just SELinux or just
>>> AppArmor.
>>
>> yes which is part of the problem with the single flag. That choice
>> would be entirely at the policy level, without any input from userspace.
>
> AIUI Paul's suggestion is the user can pre-set the details of which
> lsms to unshare and how with the lsm_set_self_attr(), and then a
> single CLONE_LSM effects that.
>
yes, I was specifically addressing the conversation I had with Paul at
LSS that Paul brought up. That is
At LSS-NA this year, John Johansen and I had a brief discussion where
he suggested a single LSM wide clone*(2) flag that individual LSM's
could opt into via callbacks.
the idea there isn't all that different than what Paul proposed. You
could have a single flag, if you can provide ancillary information. But
a single flag on its own isn't sufficient.
You can do a subset with a single flag and only policy directing things,
but that would cut container managers out of the decision. Without a
universal container identifier that really limits what you can do. In
another email I likend it to the MCS label approach to the container
where you have a single security policy for the container and each
container gets to be a unique instance of that policy. Its not a perfect
analogy as with namespace policy can be loaded into the namespace making
it unique. I don't think the approach is right because not all namespaces
implement a loadable policy, and even when they do I think we can do a
better job if the container manager is allowed to provide additional
context with the namespacing request.
^ permalink raw reply
* Re: [PATCH] selftests: centralise maybe-unused definition in kselftest.h
From: Wei Yang @ 2025-08-21 14:46 UTC (permalink / raw)
To: Bala-Vignesh-Reddy
Cc: akpm, shuah, mic, gnoack, david, lorenzo.stoakes, Liam.Howlett,
vbabka, rppt, surenb, mhocko, davem, edumazet, kuba, pabeni,
horms, ming.lei, skhan, linux-kselftest, linux-kernel,
linux-security-module, linux-mm, netdev, linux-fsdevel,
linux-block
In-Reply-To: <20250821101159.2238-1-reddybalavignesh9979@gmail.com>
On Thu, Aug 21, 2025 at 03:41:59PM +0530, Bala-Vignesh-Reddy wrote:
>Several selftests subdirectories duplicated the define __maybe_unused,
>leading to redundant code. Moved to kselftest.h header and removed
>other definition.
>
>This addresses the duplication noted in the proc-pid-vm warning fix
>
>Suggested-by: Andrew Morton <akpm@linux-foundation.org>
>Link:https://lore.kernel.org/lkml/20250820143954.33d95635e504e94df01930d0@linux-foundation.org/
>
>Signed-off-by: Bala-Vignesh-Reddy <reddybalavignesh9979@gmail.com>
Looks reasonable.
Reviewed-by: Wei Yang <richard.weiyang@gmail.com>
--
Wei Yang
Help you, Help me
^ permalink raw reply
* Re: LSM namespacing API
From: John Johansen @ 2025-08-21 14:44 UTC (permalink / raw)
To: Dr. Greg, Paul Moore; +Cc: linux-security-module, selinux, Stephen Smalley
In-Reply-To: <20250821112011.GA31850@wind.enjellic.com>
On 8/21/25 04:20, Dr. Greg wrote:
> On Tue, Aug 19, 2025 at 10:56:27AM -0400, Paul Moore wrote:
>
>> Hello all,
>
> Good morning, I hope the day is going well for everyone.
>
>> As most of you are likely aware, Stephen Smalley has been working on
>> adding namespace support to SELinux, and the work has now progressed
>> to the point where a serious discussion on the API is warranted. For
>> those of you are unfamiliar with the details or Stephen's patchset, or
>> simply need a refresher, he has some excellent documentation in his
>> work-in-progress repo:
>>
>> * https://github.com/stephensmalley/selinuxns
>>
>> Stephen also gave a (pre-recorded) presentation at LSS-NA this year
>> about SELinux namespacing, you can watch the presentation here:
>>
>> * https://www.youtube.com/watch?v=AwzGCOwxLoM
>>
>> In the past you've heard me state, rather firmly at times, that I
>> believe namespacing at the LSM framework layer to be a mistake,
>> although if there is something that can be done to help facilitate the
>> namespacing of individual LSMs at the framework layer, I would be
>> supportive of that. I think that a single LSM namespace API, similar
>> to our recently added LSM syscalls, may be such a thing, so I'd like
>> us to have a discussion to see if we all agree on that, and if so,
>> what such an API might look like.
>>
>> At LSS-NA this year, John Johansen and I had a brief discussion where
>> he suggested a single LSM wide clone*(2) flag that individual LSM's
>> could opt into via callbacks. John is directly CC'd on this mail, so
>> I'll let him expand on this idea.
>>
>> While I agree with John that a fs based API is problematic (see all of
>> our discussions around the LSM syscalls), I'm concerned that a single
>> clone*(2) flag will significantly limit our flexibility around how
>> individual LSMs are namespaced, something I don't want to see happen.
>> This makes me wonder about the potential for expanding
>> lsm_set_self_attr(2) to support a new LSM attribute that would support
>> a namespace "unshare" operation, e.g. LSM_ATTR_UNSHARE. This would
>> provide a single LSM framework API for an unshare operation while also
>> providing a mechanism to pass LSM specific via the lsm_ctx struct if
>> needed. Just as we do with the other LSM_ATTR_* flags today,
>> individual LSMs can opt-in to the API fairly easily by providing a
>> setselfattr() LSM callback.
>>
>> Thoughts?
>
> There has been an adage that traces back to the writings of George
> Santayana in 1905 that seems relevant:
>
> "Those who cannot remember the past are condemned to repeat it."
>
> To that end, some input from more than a decade of our work on this
> issue. Some of our reflections below are relevant to issues being
> covered in downstream components of this thread, particularly by John
> in the last few hours.
>
> We have had code on the table for three years with respect to the
> problem of generic namespacing of security policy/model/architecture,
> whatever one chooses to call it.
>
> For everyone's reference, here are the URL's to the patch series:
>
> V1:
> https://lore.kernel.org/linux-security-module/20230204050954.11583-1-greg@enjellic.com/T/#t
>
> V2:
> https://lore.kernel.org/linux-security-module/20230710102319.19716-1-greg@enjellic.com/T/#t
>
> V3:
> https://lore.kernel.org/linux-security-module/20240401105015.27614-1-greg@enjellic.com/T/#t
>
> V4:
> https://lore.kernel.org/linux-security-module/20240826103728.3378-1-greg@enjellic.com/T/#t
>
> We started this work about 13-15 years ago. We initially described
> our work and the need for it, 10 years ago almost to this day. See
> our 2015 paper at the Linux Security Summit in Seattle.
>
> James Morris and Casey were in the first row, Stephen and a co-worker
> from the NSA were in the second row, to the speakers left.
>
> If one spends some time looking under the hood, TSEM is in large part
> about providing a generic framework for running multiple, independent
> and orthogonal security frameworks/policies/architectures, whatever
> one chooses to call these entities.
>
> The reason that we argue that TSEM is a generic framework, is that in
> our internal work, we have ported the major LSM's, including the IMA
> infrastructure, to run in isolated namespaces as plugins for TSEM's
> notion of Trusted Modeling Agents (TMA's). We also have ongoing work
> that enables Kubernetes to dispatch workloads, using whatever LSM
> based security policy that container developers desire for their
> workloads.
>
> Suffice it to say, we have howed a lot of ground on the issues
> surrounding this, including issues surrounding production deployment
> of this type of technology.
>
> In our initial implementation, circa 2015, we adopted the approach of
> using a CLONE_* flag and wired the implementation of security
> namespaces into the rest of the namespace infrastructure.
>
> During COVID, we re-architected the entire implementation and moved to
> using a control file in the pseudo-filesystem that TSEM implements, we
> have never looked back on this decision.
>
> TSEM security workloads are a poster child for security namespaces
> that require a number of different setup parameters. A command verb
> syntax with key=value pairs, written to a pseudo-file, has proven
> itself to be the most flexible approach when setting up security
> workloads.
>
> With respect to namespace transition, we trigger the transition of a
> process to a new namespace (unsharing) when the process issues the
> request via the control file. This has proven to be, at once, the
> most straight forward and least security prone approach.
>
> The other major, and thorny issue, is the notion of another process
> 'entering' a security namespace. There are a ton of open issues to be
> considered with this, the approach that we took that has worked well
> to date, is the notion of a 'trust orchestrator' that has
> responsibility for controlling the namespace. Any manipulations or
> control of the namespace are conducted through the orchestrator
> process.
>
> If anyone chooses to look at our implementation, you will find that we
> 'bless' the orchestrator process, at the time of namespace creation,
> with access to the security namespace context control structure for
> the namespace being created. The orchestrator is the only entity that
> can access the security state of the namespace, other than processes
> within the namespace itself.
>
> This significantly narrows the scope of vulnerability with respect to
> who or what can manipulate a security namespace. There are a number
> of thorny issues, that we have not seen anyone mention, that surface
> with respect to allowing entry into a security namespace by an
> arbitrary process. Believe me when I say we have found a number of
> them by accident and incident.
>
indeed, this has to be tightly controlled. Much more so than just
creating a namespace. And its not just the "security/LSM" namespace
but the entire context around it. That is whether or not you can
step into say the mount namespace separate from the security/LSM
namespace it was created with.
Each and everyone of those opens potential attack surface. Even if you
if it turns out to be safe, you have to carefully evaluate each
potential combination.
> So big picture.
>
> Over a decade of experience with these issues, suggests that Paul's
> premise that most of these issues are best left to specific LSM's that
> elect to implement namespacing, is correct.
>
> The challenge is that this situation ends up being all or nothing.
>
> The actual amount of code involved in unsharing a namespace is so
> trivial, in comparison to the work involved with setting up and
> maintaining state information for a security namespace context, that
> it seems to make little sense to implement this support at the level
> of the LSM infrastructure itself.
>
actually I think that is pretty much the goal, just a minimal thin
layer that provides the hooks and maybe an LSM blob object for the
individual LSMs to do their thing.
Instead of each LSM implementing their own interface there is a common
one for container orchastrators to use to make the request.
> If the decision is made to provide generic namespace support, other
> than a request to create a namespace, it will rapidly become a
> slippery slope with respect to the amount of infrastructure needed to
> address the complexities associated with every security model being
> different from every other.
>
yep, this is really just about a thin common API, and minimal
infrastructure around the existing system namespacing calls (clone,
ushare, setns).
> The caveat to this is if our notion of a 'trust orchestrator' would be
> deemed to have merit. In that case, an LSM based namespace separation
> architecture would provide a common point for the orchestrator to be
> 'blessed' with access to control of a namespace.
>
A trust orchestrator isn't necessarily needed. Each LSM can manage its
own trust within its policy. A trust orchestrator becomes more necessary
when you are trying to namespacing without the LSMs themselves
participating in the decision around namespacing. Which admittedly has
largely been the current situation.
> The other open issue is whether or not a separate capability should be
> implemented that allows the creation of a new security namespace. If
> one paws through our TSEM submissions, one will see that we proposed
> such a capability bit.
>
Its not needed if individual LSMs are making decisions around namespacing
based on policy. In fact in that case it can even be harmful. Per LSM
policy would be finer grained, where a capability becomes this shared
flag that lacks context. Examples abound in the kernel where we have
a cap check without context and then a more context based security
check.
Where the capability might be useful is wehn LSMs aren't dealing with
the namespacing request directly.
> Casey noted, rather emphatically, that no new capabilities were going
> to be implemented in Linux, particularly for what was described as a
> 'toy' project. He indicated that CAP_MAC_ADMIN was the canonical
> capability that should be used for manipulating LSM's.
>
I disagree with the reuse of CAP_MAC_ADMIN, if there is going to be
a capability around this it should be distinct from MAC_ADMIN and
MAC_OVERRIDE, as it very much has different semantics.
> We will be very interested in seeing how a discussion around this
> evolves, as 'escaping' from an existing security context to a new one
> is an extremely critical operation from a security perspective, if one
yes. I might have mentioned just how much I dislike setns().
> stands back and looks at the issue objectively. If the concept of a
> 'security orchestrator' is embraced, it would make perfect sense for
> the orchestrator to drop CAP_SEC_NS, or whatever it would be called,
> and retain CAP_MAC_ADMIN in order to manage the namespace.
>
> So lots of issues to consider; thorny, political and otherwise, on
> multiple fronts.
>
>> paul-moore.com
>
> Have a good day.
>
> As always,
> Dr. Greg
>
> The Quixote Project - Flailing at the Travails of Cybersecurity
> https://github.com/Quixote-Project
>
^ 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