* Re: [PATCH RFC] security: add LSM blob and hooks for namespaces
From: Paul Moore @ 2026-02-16 18:53 UTC (permalink / raw)
To: Christian Brauner; +Cc: James Morris, linux-security-module, linux-kernel
In-Reply-To: <20260216-work-security-namespace-v1-1-075c28758e1f@kernel.org>
On February 16, 2026 2:52:34 PM Christian Brauner <brauner@kernel.org> wrote:
> All namespace types now share the same ns_common infrastructure. Extend
> this to include a security blob so LSMs can start managing namespaces
> uniformly without having to add one-off hooks or security fields to
> every individual namespace type.
>
> Add a ns_security pointer to ns_common and the corresponding lbs_ns
> blob size to lsm_blob_sizes. Allocation and freeing hooks are called
> from the common __ns_common_init() and __ns_common_free() paths so
> every namespace type gets covered in one go. All information about the
> namespace type and the appropriate casting helpers to get at the
> containing namespace are available via ns_common making it
> straightforward for LSMs to differentiate when they need to.
>
> A namespace_install hook is called from validate_ns() during setns(2)
> giving LSMs a chance to enforce policy on namespace transitions.
>
> Individual namespace types can still have their own specialized security
> hooks when needed. This is just the common baseline that makes it easy
> to track and manage namespaces from the security side without requiring
> every namespace type to reinvent the wheel.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
> include/linux/lsm_hook_defs.h | 3 ++
> include/linux/lsm_hooks.h | 1 +
> include/linux/ns/ns_common_types.h | 3 ++
> include/linux/security.h | 20 ++++++++++
> kernel/nscommon.c | 12 ++++++
> kernel/nsproxy.c | 8 +++-
> security/lsm_init.c | 2 +
> security/security.c | 76 ++++++++++++++++++++++++++++++++++++++
> 8 files changed, 124 insertions(+), 1 deletion(-)
I still have limited network access for a few more days, but a couple of
quick comments in no particular order ...
Generally speaking we don't add things to the LSM interface without a user,
and I can't think of a good reason why we would want to do things
differently here. This means that when you propose something like this you
should also propose an addition to one of the in-tree LSMs to make use of
it. While the guidance doc linked below (also linked in the LSM MAINTAINERS
entry) doesn't have any guidance for the LSM blobs as they are generally a
byproduct of the hooks, if you are looking for some general info I think
the bits on adding a new LSM hook would be very close to what we would
expect for blob additions.
https://github.com/LinuxSecurityModule/kernel/blob/main/README.md
Getting to the specifics of namespace related APIs, we've had a lot of
discussions about namespacing and my current opinion is that we need to
sort out if we want a userspace API at the LSM framework layer, or if we
want to do that at the individual LSM layer; there is a lot of nuance there
and while one option may seem like an obvious choice, we need some more
discussion and I need a chance to get caught up on the threads. Once we
have an API decision then we can start sorting out the implementation
details like the LSM blobs.
--
paul-moore.com
>
^ permalink raw reply
* Re: [PATCH v1 1/2] landlock: Fully release unused TSYNC work entries
From: Günther Noack @ 2026-02-16 19:33 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Jann Horn
In-Reply-To: <20260216.iep2jei5Dees@digikod.net>
On Mon, Feb 16, 2026 at 06:43:25PM +0100, Mickaël Salaün wrote:
> On Mon, Feb 16, 2026 at 04:25:53PM +0100, Günther Noack wrote:
> > On Mon, Feb 16, 2026 at 03:26:38PM +0100, Mickaël Salaün wrote:
> > > for (i = 0; i < s->size; i++) {
> > > - if (!s->works[i]->task)
> > > + if (WARN_ON_ONCE(!s->works[i]->task))
> >
> > Is this a condition we should warn on? It is very unlikely, but it
> > can technically happen that a thread exits at the same time as TSYNC
> > and happens to hit that narrow race condition window. As long as it
> > happens only sporadically, I don't think there is anything wrong (and
> > in particular, it's not actionable for the user - I don't think there
> > is a way to fix it if that warning appears?)
>
> WARN() should definitely not be called if the condition can legitimately
> be true.
>
> "task" is only set by tsync_works_provide(), so only by the caller
> thread. How could "task" be NULL (within the works->size range)?
Ah, you are right. This could have become NULL before, but now it
can't become NULL any more. Please ignore my remark.
> > > continue;
> > >
> > > put_task_struct(s->works[i]->task);
> > > @@ -389,6 +389,15 @@ static bool schedule_task_work(struct tsync_works *works,
> > > */
> > > put_task_struct(ctx->task);
> > > ctx->task = NULL;
> > > + ctx->shared_ctx = NULL;
> > > +
> > > + /*
> > > + * Cancel the tsync_works_provide() change to recycle the reserved
> > > + * memory for the next thread, if any. This also ensures that
> > > + * cancel_tsync_works() and tsync_works_release() do not see any
> > > + * NULL task pointers.
> > > + */
> > > + works->size--;
> >
> > Looks good.
> >
> > [Optional code arrangement remarks:
> >
> > I would recommend to put that logic in a helper function
> > "tsync_works_return(struct tsync_works *s, struct tsync_work *)", to
> > be in line with the existing implementation where the manipulation of
> > struct tsync_works is encapsulated in the "tsync_*" helper functions.
> >
> > The scope of that function would be to do the inverse of
> > "tsync_works_provide()" -- putting the task_struct, decreasing
> > works->size, and then, to be safe, also clearing the contents of the
> > tsync_work struct (although that is strictly speaking not required if
> > we decrease the size, I think).
>
> Should we move the atomic_inc() to tsync_works_provide() and the
> atomic_dec() to this new helper?
No, I would keep the atomic_inc() and atomic_dec() calls in the
functions where they are now.
The atomic counters belong logically to the synchronization scheme
between the different threads, and I think it's clearer if we keep
that synchronization code outside of the struct task_works
abstraction.
I see the struct tsync_works and its operations (functions starting
with "tsync_works_") as logically belonging together in an
OO/encapsulation sense, and I think it's useful to have a clear
boundary of responsibilities. These functions are only in the
business of managing the direct values stored in the "struct
tsync_works", and in the business of allocating the memory for that
data structure and incrementing refcounts to the struct task_struct.
(The latter is mostly useful to have in tsync_works_provide() because
the inverse put_task_struct() is useful to have in
tsync_works_release(), and then it is symmetric.)
> > The only unusual thing about the tsync_works_return() function would
> > be that it is only OK to return the very last tsync_work struct which
> > was returned from tsync_works_provide().
>
> What about renaming tsync_works_provide() to tsync_works_push() and this
> new one to tsync_works_pop()?
I think I would find that naming slightly confusing: When a function
is called "push", I would normally expect to pass a value to it, but
we're getting one from it. And when a method is called "pop" I would
expect to get a value from it. But the inverse is true here. With
the names "provide" and "return" it feel that the directionality of
argument passing would be clearer.
> > It's an improvement either way though; If you want to prioritize
> > fixing this and don't want to extract the extra function now, we can
> > also look into it in a follow-up. From a functional standpoint, I
> > think your code works as well.
>
> It's a small refactoring, so better to do it now.
Sounds good. 👍
–Günther
^ permalink raw reply
* Re: [PATCH v1 1/2] landlock: Fully release unused TSYNC work entries
From: Mickaël Salaün @ 2026-02-16 19:57 UTC (permalink / raw)
To: Günther Noack; +Cc: Günther Noack, linux-security-module, Jann Horn
In-Reply-To: <20260216.b2c8aaab9a80@gnoack.org>
On Mon, Feb 16, 2026 at 08:33:05PM +0100, Günther Noack wrote:
> On Mon, Feb 16, 2026 at 06:43:25PM +0100, Mickaël Salaün wrote:
> > On Mon, Feb 16, 2026 at 04:25:53PM +0100, Günther Noack wrote:
> > > On Mon, Feb 16, 2026 at 03:26:38PM +0100, Mickaël Salaün wrote:
> > > > for (i = 0; i < s->size; i++) {
> > > > - if (!s->works[i]->task)
> > > > + if (WARN_ON_ONCE(!s->works[i]->task))
> > >
> > > Is this a condition we should warn on? It is very unlikely, but it
> > > can technically happen that a thread exits at the same time as TSYNC
> > > and happens to hit that narrow race condition window. As long as it
> > > happens only sporadically, I don't think there is anything wrong (and
> > > in particular, it's not actionable for the user - I don't think there
> > > is a way to fix it if that warning appears?)
> >
> > WARN() should definitely not be called if the condition can legitimately
> > be true.
> >
> > "task" is only set by tsync_works_provide(), so only by the caller
> > thread. How could "task" be NULL (within the works->size range)?
>
> Ah, you are right. This could have become NULL before, but now it
> can't become NULL any more. Please ignore my remark.
>
>
> > > > continue;
> > > >
> > > > put_task_struct(s->works[i]->task);
> > > > @@ -389,6 +389,15 @@ static bool schedule_task_work(struct tsync_works *works,
> > > > */
> > > > put_task_struct(ctx->task);
> > > > ctx->task = NULL;
> > > > + ctx->shared_ctx = NULL;
> > > > +
> > > > + /*
> > > > + * Cancel the tsync_works_provide() change to recycle the reserved
> > > > + * memory for the next thread, if any. This also ensures that
> > > > + * cancel_tsync_works() and tsync_works_release() do not see any
> > > > + * NULL task pointers.
> > > > + */
> > > > + works->size--;
> > >
> > > Looks good.
> > >
> > > [Optional code arrangement remarks:
> > >
> > > I would recommend to put that logic in a helper function
> > > "tsync_works_return(struct tsync_works *s, struct tsync_work *)", to
> > > be in line with the existing implementation where the manipulation of
> > > struct tsync_works is encapsulated in the "tsync_*" helper functions.
> > >
> > > The scope of that function would be to do the inverse of
> > > "tsync_works_provide()" -- putting the task_struct, decreasing
> > > works->size, and then, to be safe, also clearing the contents of the
> > > tsync_work struct (although that is strictly speaking not required if
> > > we decrease the size, I think).
> >
> > Should we move the atomic_inc() to tsync_works_provide() and the
> > atomic_dec() to this new helper?
>
> No, I would keep the atomic_inc() and atomic_dec() calls in the
> functions where they are now.
>
> The atomic counters belong logically to the synchronization scheme
> between the different threads, and I think it's clearer if we keep
> that synchronization code outside of the struct task_works
> abstraction.
>
> I see the struct tsync_works and its operations (functions starting
> with "tsync_works_") as logically belonging together in an
> OO/encapsulation sense, and I think it's useful to have a clear
> boundary of responsibilities. These functions are only in the
> business of managing the direct values stored in the "struct
> tsync_works", and in the business of allocating the memory for that
> data structure and incrementing refcounts to the struct task_struct.
> (The latter is mostly useful to have in tsync_works_provide() because
> the inverse put_task_struct() is useful to have in
> tsync_works_release(), and then it is symmetric.)
This makes sense.
>
>
> > > The only unusual thing about the tsync_works_return() function would
> > > be that it is only OK to return the very last tsync_work struct which
> > > was returned from tsync_works_provide().
> >
> > What about renaming tsync_works_provide() to tsync_works_push() and this
> > new one to tsync_works_pop()?
>
> I think I would find that naming slightly confusing: When a function
> is called "push", I would normally expect to pass a value to it, but
> we're getting one from it.
Well, it pushes the thread and returns the wrapped object.
> And when a method is called "pop" I would
> expect to get a value from it. But the inverse is true here.
Fair
> With
> the names "provide" and "return" it feel that the directionality of
> argument passing would be clearer.
I don't understand the logic with "return": this tsync_works_return()
would not return anything.
What about something like tsync_works_shrink()?
^ permalink raw reply
* Re: [PATCH v1 1/2] landlock: Fully release unused TSYNC work entries
From: Mickaël Salaün @ 2026-02-16 20:10 UTC (permalink / raw)
To: Günther Noack; +Cc: Günther Noack, linux-security-module, Jann Horn
In-Reply-To: <20260216.deiM1cuphohZ@digikod.net>
On Mon, Feb 16, 2026 at 08:57:34PM +0100, Mickaël Salaün wrote:
> On Mon, Feb 16, 2026 at 08:33:05PM +0100, Günther Noack wrote:
> > On Mon, Feb 16, 2026 at 06:43:25PM +0100, Mickaël Salaün wrote:
> > > On Mon, Feb 16, 2026 at 04:25:53PM +0100, Günther Noack wrote:
> > > > On Mon, Feb 16, 2026 at 03:26:38PM +0100, Mickaël Salaün wrote:
> > > > > @@ -389,6 +389,15 @@ static bool schedule_task_work(struct tsync_works *works,
> > > > > */
> > > > > put_task_struct(ctx->task);
> > > > > ctx->task = NULL;
> > > > > + ctx->shared_ctx = NULL;
> > > > > +
> > > > > + /*
> > > > > + * Cancel the tsync_works_provide() change to recycle the reserved
> > > > > + * memory for the next thread, if any. This also ensures that
> > > > > + * cancel_tsync_works() and tsync_works_release() do not see any
> > > > > + * NULL task pointers.
> > > > > + */
> > > > > + works->size--;
> > > >
> > > > Looks good.
> > > >
> > > > [Optional code arrangement remarks:
> > > >
> > > > I would recommend to put that logic in a helper function
> > > > "tsync_works_return(struct tsync_works *s, struct tsync_work *)", to
> > > > be in line with the existing implementation where the manipulation of
> > > > struct tsync_works is encapsulated in the "tsync_*" helper functions.
> > > >
> > > > The scope of that function would be to do the inverse of
> > > > "tsync_works_provide()" -- putting the task_struct, decreasing
> > > > works->size, and then, to be safe, also clearing the contents of the
> > > > tsync_work struct (although that is strictly speaking not required if
> > > > we decrease the size, I think).
> > >
> > > Should we move the atomic_inc() to tsync_works_provide() and the
> > > atomic_dec() to this new helper?
> >
> > No, I would keep the atomic_inc() and atomic_dec() calls in the
> > functions where they are now.
> >
> > The atomic counters belong logically to the synchronization scheme
> > between the different threads, and I think it's clearer if we keep
> > that synchronization code outside of the struct task_works
> > abstraction.
> >
> > I see the struct tsync_works and its operations (functions starting
> > with "tsync_works_") as logically belonging together in an
> > OO/encapsulation sense, and I think it's useful to have a clear
> > boundary of responsibilities. These functions are only in the
> > business of managing the direct values stored in the "struct
> > tsync_works", and in the business of allocating the memory for that
> > data structure and incrementing refcounts to the struct task_struct.
> > (The latter is mostly useful to have in tsync_works_provide() because
> > the inverse put_task_struct() is useful to have in
> > tsync_works_release(), and then it is symmetric.)
>
> This makes sense.
>
> >
> >
> > > > The only unusual thing about the tsync_works_return() function would
> > > > be that it is only OK to return the very last tsync_work struct which
> > > > was returned from tsync_works_provide().
> > >
> > > What about renaming tsync_works_provide() to tsync_works_push() and this
> > > new one to tsync_works_pop()?
> >
> > I think I would find that naming slightly confusing: When a function
> > is called "push", I would normally expect to pass a value to it, but
> > we're getting one from it.
>
> Well, it pushes the thread and returns the wrapped object.
>
> > And when a method is called "pop" I would
> > expect to get a value from it. But the inverse is true here.
>
> Fair
>
> > With
> > the names "provide" and "return" it feel that the directionality of
> > argument passing would be clearer.
>
> I don't understand the logic with "return": this tsync_works_return()
> would not return anything.
>
> What about something like tsync_works_shrink()?
tsync_works_trim() may be better.
^ permalink raw reply
* Re: [RFC] Landlock: mutable domains (and supervisor notification uAPI options)
From: Justin Suess @ 2026-02-16 21:27 UTC (permalink / raw)
To: utilityemal77
Cc: amir73il, gnoack, jack, jannh, linux-security-module, m, mic,
penguin-kernel, song
In-Reply-To: <20260215212353.3549464-1-utilityemal77@gmail.com>
On Sun, Feb 15, 2026 at 02:54:08AM +0000, Tingmao Wang wrote:
> [...]
> Background
> ----------
>
> A while ago I sent a "Landlock supervise" RFC patch series [1], in which I
> proposed to extend Landlock with additional functionality to support
> "interactive" rule enforcement. In discussion with Mickaël, we decided to
> split this work into 3 stages: quiet flag, mutable domains, and finally
> supervisor notification. Relevant discussions are at [4] and in replies
> to [1].
>
> The patch for quiet flag [5] has gone through multiple review iterations
> already. It is useful on its own, but it was also motivated by the
> eventual use in controlling supervisor notification.
>
> The next stage is to introduce "mutable domains". The motivation for this
> is two fold:
>
> 1. This allows the supervisor to allow access to (large) file hierarchies
> without needing to be woken up again for each access.
> 2. Because we cannot block within security_path_mknod and other
> directory-modification related hooks [6], the proposal was to return
> immediately from those hooks after queuing the supervisor notification,
> then wait in a separate task_work. This however means that we cannot
> directly "allow" access (and even if we can, it may introduce TOCTOU
> problems). In order to allow access to requested files, the supervisor
> has to add additional rules to the (now mutable) domain which will
> allow the required access.
Is blocking during connect(2) allowed either if the socket is non-blocking?
This may be another example case that needs to be handled differently than calls
we can block in safely.
> [...]
> Why require a commit operation?
> -------------------------------
>
> This is not a strictly necessary requirement with an rbtree based
> implementation - it can be made thread-safe with RCU while still allowing
> lockless access checks without too much overhead (although the code is
> indeed more tricky to write). However, there is a possibility that the
> domain lookup might become a hashtable with some future enhancement [8],
> at which point it would be better to have an explicit commit operation to
> avoid rebuilding the hashtable for every landlock_add_rule(). Having a
> commit operation will likely also make some atomicity properties easier to
> achieve, depending on the supervisor's needs.
>
> I've actually previously implemented hashtable domains [9], but after
> benchmarking it I did not find a very significant performance improvement
> (2.2% with 10 dir depth and 10 rules, 8.6% with 29 depth and 1000 rules) [10]
> especially considering the complexity of the changes required. After
> discussion with Mickaël I've decided to not pursue it for now, but I'm
> open to suggestions. If Mickaël and Günther are open to taking it, I can
> revive the patch.
>
> [8]: https://github.com/landlock-lsm/linux/issues/1
> [9]: https://lore.kernel.org/all/cover.1751814658.git.m@maowtm.org/
> Note that the benchmark posted here was inaccurate, due to the
> relatively high cost of kfunc probes compared to the work required
> to handle one openat(). For a more proper benchmark, refer to the
> comment below:
> [10]: https://github.com/landlock-lsm/landlock-test-tools/pull/17#issuecomment-3594121269
> See specifically the collapsed section "parse-microbench.py
> base-vm.log arraydomain-vm.log"
>
>
> Proposed implementation
> -----------------------
>
> In order to store additional data and locks for the supervisor, we create
> a new `struct landlock_supervisor`. Both the supervisor and supervisee
> rulesets, and the landlock_hierarchy of each layer, will point to this
> struct. (A future revision may optimize on this to reduce pointer chasing
> when needing to check supervisor rulesets of parent layers.)
>
> One of the main tricky areas of this work is the implementation of
> LANDLOCK_ADD_RULE_COMMIT_SUPERVISOR and the access checks. We want:
>
> - atomic commit: the supervised program should not "experience" any rule
> changes until they are committed, and once it is committed it should see
> all the changes together
>
> - lockless access checks (even when the supervisee ruleset does not allow
> the access, necessitating checking the supervisor rulesets, this should
> still not involve any locks)
>
> - atomic access checks: an access check should either be completely based
> on the "old" rules or the "new" rules, even if a commit happens in the
> middle of a path walk. This prevents incorrect denials when a commit
> moves a rule from /a to /a/b when we've just finished checking /a/b and
> about to check /a.
>
> In order to achieve atomic commit, the supervisor fd cannot actually point
> to (and thus allow editing) the "live" ruleset. Instead, when a
> `LANDLOCK_ADD_RULE_COMMIT_SUPERVISOR` is requested, a new `struct
> landlock_ruleset` is created, the rules are copied over from the existing
> supervisor ruleset, and the pointer in the landlock_supervisor is swapped.
>
> In order to keep access checks lockless (as it is currently), the live
> ruleset pointer needs to be RCU-protected. To reduce complexity, this
> initial implementation uses synchronize_rcu() directly in the calling
> thread of `LANDLOCK_ADD_RULE_COMMIT_SUPERVISOR`, and frees the old
> supervisor ruleset afterwards, but this can be rewritten to use call_rcu()
> in a future iteration if necessary (which will allow quicker commits,
> which can be quite impactful if we use this to auto-generate rulesets).
>
> During access checks, for each step of the path walk, after
> landlock_unmask_layers()-ing the supervisee rule, if the access is not
> already allowed, we check for rules in the supervisor ruleset and
> effectively does landlock_unmask_layers() on them too.
>
> In order to have atomic access checks, we need to pre-capture the
> supervisor committed ruleset pointers for all layers at the start of the
> path walk (in `is_access_to_paths_allowed`). Storing this on the stack,
> this takes the space of 16 pointers, hence 128 bytes on 64-bit (I'm keen
> to hear suggestions on how best to mitigate this). Another effect of this
> "caching" is that in order to be able to release rcu in the path walk
> (which is required for the path_put()), we actually need to take refcount
> on the committed ruleset (and free it at the end of
> is_access_to_paths_allowed).
>
>
> Optional accesses
> -----------------
>
> Optional access (truncate and ioctl) handling is also tricky. There are
> two possible alternatives:
>
> - The allowed optional actions are still entirely determined at file open
> time. This likely works in the majority of cases, where truncate (and
> maybe also ioctl) are given or taken away together with write access.
> However, this may mean that we need to send an access request
> notification immediately at open() time if e.g. write access is given
> but truncate (or ioctl) is not, even if truncate (or ioctl) is not
> attempted yet, since the supervisor would not be able to allow it later.
> (or alternatively we can choose to not send this notification, and the
> supervisor will just have to "know" to add truncate/ioctl rights if
> required, in advance.)
>
> - The allowed optional actions are considered to be determined at
> operation time (even though for a static ruleset it is cached). This
> means that for supervised layers, we will always have to re-check their
> supervisor rulesets, whether or not the access was initially allowed,
> which will involve doing a path walk. This does however means that the
> supervisor can be notified "in the moment" when a truncate (or more
> likely to be relevant - ioctl) is attempted.
>
> The PoC partially implements the second one (but has bugs), but I'm not
> sure which is best. The second one is most flexible and makes more sense
> to me from a user perspective, but does come with performance
> implications.
>
>
> (Disallowing) self-supervision
> ------------------------------
>
> We should figure out a way to ensure that a process cannot call
> landlock_restrict_self() with a ruleset that has a supervisor for which it
> has access to (i.e. via a supervisor ruleset fd). This prevents
> accidental misuse, and also prevents deadlocks as discussed in [11]. I'm
> not sure if this will be easy to implement, however.
This seems like a graph acyclicity problem.
Here are a couple cases to consider:
1. LANDLOCK_RESTRICT_SELF_TSYNC misuse:
In the case where a user wants to use this supervisor to supervise other
threads within the same process, a user could naively call
LANDLOCK_RESTRICT_SELF_TSYNC (merged into 7.0) when enforcing the
supervisee_fd. This would enforce the same policy on the thread running
the supervisor and the supervisee.
2. Transfer of the supervisee_fd (SCM_RIGHTS)
It's possible to transfer file descriptors over unix domain sockets. If
we had a supervisor daemon that used this form of IPC to send precooked
supervisee_fds to other threads, and one of those ended up in a parent
process of the supervisor, we could inadvertently end up with problems.
3. Blocking in other LSMs (pointed out in your source [11])
The hardest case to deal with, other LSMs like TOMOYO can also block and
cause dependency cycles.
---
This gets tricky, and I don't know if just checking parent / child
relationships would work. Because the supervisor and supervisee rulesets
are just file descriptors, and there are potentially unlimited number of
ways these FDs could be transfered or instantiated.
I think the best way to deal with this is constraining the problem space:
An idea (binding supervisors/supervisees to domains on first use)
Whenever landlock_restrict_self(supervisee_fd,...) is called, check the
current domain credentials and verify that the domain is a *proper
subset* of the supervisors domain. Then permanently close the
supervisee_fd and never allow reenforcement. Similarly, once a
supervisor_fd is created, never allow commiting from a context with
"current landlock domain != original landlock domain at creation"
This prevents post-enforcement usage of the supervisee_fd by a parent
domain, and post-commit usage of a supervisee_fd by any subdomain.
I'm not sure if it's possible to check whether one domain is a
proper subset of another (ie supervisor domain includes but *doesn't equal*
supervisee domain), but I think that's one way do do it.
This idea would help, but doesn't address case 3 above.
>
> [11]: https://lore.kernel.org/all/cc3e131f-f9a3-417b-9267-907b45083dc3@maowtm.org/
>
>
> Supervisor notification
> -----------------------
>
> The above RFC only covers mutable domains. The natural next stage of this
> work is to send notification to the supervisor on access denials, so that
> it can decide whether to allow the access or not. For that, there are
> also lots of questions at this stage:
>
>
> - Should we in fact implement that first, before mutable domains? This
> means that the supervisor would only be able to find out about denials,
> but not allow them without a sandbox restart. We still eventually want
> the mutable domains, since that makes this a lot more useful, but I can
> see some use cases for just the notification part (e.g. island denial
> log), and I can't see a likely use case for just mutable domains, aside
> from live reload of landlock-config (maybe that _is_ useful on its own,
> considering that you can also find out about denials from the kernel
> audit log, and add missing rules based on that).
>
>
> - Earlier when implementing the Landlock supervise v1 RFC, I basically
> came up with an ad-hoc uAPI for the notification [12], and the PoC code
> linked to above also uses this uAPI. There are of course many problems
> with this as it stands, e.g. it only having one destname, which means
> that for rename, the fd1 needs to be the child being moved, which does
> not align with the vfs semantic and how Landlock treat it (i.e. the
> thing being updated here is the parent directory, not the child itself).
> Same for delete, which currently sends the child as fd1.
>
> But also, in discussion with Mickaël last year, he mentioned that we
> could reuse the fsnotify infrastructure, and perhaps additionally, use
> fanotify to deliver these notifications. I do think there is some
> potential here, as fanotify already implements an event header, a
> mechanism for receiving and replying to events, etc. We could possibly
> extend it to send Landlock specific notifications via a new kind of mark
> (FAN_MARK_LANDLOCK_DOMAIN ??) and add one or more new corresponding
> event types. Mickaël mentioned mount notifications [13] as an example
> of using fanotify to send notifications other than file/dir
> modifications.
>
> I'm not sure if directly extending the fanotify uAPI is a good idea tho,
> considering that Landlock is not a feature specific to the filesystem -
> we will also have denial events for net_port rules, and perhaps more in
> the future. However, Mickaël mentioned that there might be some
> internal infrastructure which we can re-use (even if we have our own
> notification uAPI).
I think that a new FAN_MARK would be required to use fanotify uAPI.
There are a couple questions I have with this: (if we extend fanotify)
1. What FAN_CLASS_* would notifications use?
FAN_CLASS_* specifies the type of notification, when the notification is
triggered.
See [1] for the current classes.
If we want interactive, pre-access blocking, that would correspond to
FAN_CLASS_PRE_CONTENT or FAN_CLASS_CONTENT. Both of which currently
require CAP_SYS_ADMIN regardless of FAN_MARK. Which requiring that
would require that supervisors have CAP_SYS_ADMIN, if the current
CAP_SYS_ADMIN requirements remain in place.
(If we don't have interactive blocking denials, we could just use
FAN_CLASS_NOTIF)
2. How would fanotify events be encoded?
Events in fanotify use this structure for event data (one or more of the
following must be recieved in a notification) [2]
struct fanotify_event_metadata {
__u32 event_len;
__u8 vers;
__u8 reserved;
__u16 metadata_len;
__aligned_u64 mask;
__s32 fd;
__s32 pid;
};
There are access classes landlock restricts that might not have an fd at
all, like abstract unix sockets, tcp ports, signals etc.
Good news is fanotify supports multiple types of additional information
records, and we could potentially extend fanotify to support new ones as
you alluded to.
For examples of this, see struct fanotify_event_info_mnt,
fanotify_event_info_pidfd.
These records get attached to the event so they could be used to pass
landlock access data.
3. If we support interactive permission decisions (even for a
subset of landlock access rights only), do we use the response code?
(question might be moot if we don't do blocking/responses at all)
From [2]:
For permission events, the application must write(2) a structure
of the following form to the fanotify file descriptor:
struct fanotify_response {
__s32 fd;
__u32 response;
};
response is a FAN_ALLOW or FAN_DENY. This is used by fanotify as a
one-time access decision. Would this be used to do one-off exceptions to
policy, or would we require policy decisions to go through the
supervisor_fd and ignore the response code?
4. How would we reconcile the disparity between fanotify access rights
and landlock access rights?
There's no clean 1:1 mapping between fanotify access rights and landlock
access rights as Mickaël pointed out. [2] [3]
Many fs rights (creation, deletion, rename, linking) are not handled or
implemented, (not even considering network/unix/signal scoping), so we'd
be adding all these landlock specific rights.
We could make a "catch-all" FAN_LANDLOCK_ACCESS or similar and ignore
all the existing rights, and put the actual access data in the event
record. It's awkward either way.
---
In conclusion, I think extending fanotify is more viable than seccomp,
from a purely technical standpoint. because it seems extensible,
and because it runs post-lsm hooks.
That being said, it's awkward, requires large extensions to the API, and
definition of permissions that are specific to landlock.
Whether or not landlock makes sense in fanotify from a semantic point of
view is an entirely different question. There's no precedent for
non-filesystem access controls in fanotify, so it's a little... out-of-place
for an LSM to expose features on a filesystem access notification api?
Curious on what people think.
[1]: https://man7.org/linux/man-pages/man2/fanotify_init.2.html
[2]: https://man7.org/linux/man-pages/man7/fanotify.7.html
[3]: https://lore.kernel.org/all/20250304.Choo7foe2eoj@digikod.net/
>
>
> - The other uAPI alternative which I have been thinking of is to extend
> seccomp-unotify. For example, a Landlock denial could result in the
> syscall being trapped and a `struct seccomp_notif` being sent to the
> seccomp supervisor (via the existing mechanism), with additional
> information (mostly, the file(s) / net ports being accessed and access
> rights requested) attached to the notification _somehow_. Then the
> supervisor can use the same kind of responses one would use for
> seccomp-unotify to cause the syscall to either be retried (possibly via
> `SECCOMP_USER_NOTIF_FLAG_CONTINUE`) or return with an error code of its
> choice (or alternatively, carry out the operation on behalf of the
> child, and pretend that the syscall succeed, which might be useful to
> implement an "allow file creation but only this file" / "allow `mktemp
> -d` but not arbitrary create on anything under /tmp").
>
> Looking at `struct seccomp_notif` and `struct seccomp_data` however, I'm
> not sure how feasible / doable this extension would be. Also,
> seccomp-unotify is supposed to trigger before a syscall is actually
> executed, whereas if we use it this way, we will want it to trigger
> after we're already midway through the syscall (in the LSM hook). This
> might make it hard to implement (and also twists a bit the uAPI
> semantics of seccomp-unotify).
>
(Some of the stuff discussed with seccomp below is derived from a side
conversation with Tingmao over this proposal)
There are some problems with extending seccomp unotify. Passing the
full context needed through this api to the supervisor is problematic.
seccomp unotify notifications look like this [4]:
struct seccomp_notif {
__u64 id; /* Cookie */
__u32 pid; /* TID of target thread */
__u32 flags; /* Currently unused (0) */
struct seccomp_data data; /* See seccomp(2) */
};
And struct seccomp_data [5]:
struct seccomp_data {
int nr; /* System call number */
__u32 arch; /* AUDIT_ARCH_* value
(see <linux/audit.h>) */
__u64 instruction_pointer; /* CPU instruction pointer */
__u64 args[6]; /* Up to 6 system call arguments */
};
Even if we pass the syscall data, for the userspace to actually decode
the arguments to figure out what the access is doing we have two
critical problems (1,2) and one annoyance (3):
1. The syscall itself doesn't necessarily contain the full context of the access.
2. We cannot decode the pointer-based arguments from userspace for a syscall
in seccomp without TOCTOU. It also requires reaching into userspace
memory. [6]
3. Decoding the syscall number is an arch-specific operation that we now have
to expect userspace to deal with.
So unless there's something I'm missing extending seccomp unotify doesn't really make
sense. It's not as much of an extensible API like fanotify.
Unless we artificially trigger some notification after the fact, and figure out how
to jam the relevant access information into the notification or pass it through
a side channel, it's gonna be a difficult path forward to use seccomp directly.
[4]: https://man7.org/linux/man-pages/man2/seccomp_unotify.2.html
[5]: https://man7.org/linux/man-pages/man2/seccomp.2.html
[6]: https://blog.skepticfx.com/post/seccomp-pointers/
>
> Are there any immediate reasons, from Landlock's perspective, to rule out
> either of them? (I will probably wait for at least a first review from
I think direct extensions to seccomp are awkward at best, and it's
difficult to reason about an extension that would make sense.
fanotify seems more viable, but would require heavy extensions
(new record types, permission types) and adding landlock to it would be
inconsistent with the existing implementation semantically. (landlock is
not VFS specific).
I think the most viable path forward if this is to be done is a
dedicated uAPI. That being said, I think what Mickaël said about reusing
the internals is viable.
> the Landlock side before directing this explicitly to the fanotify and/or
> seccomp-unotify maintainers, in case the plan significantly changes, but
> if somehow a maintainer/reviewer from either of those areas are already
> reading this, firstly thanks, and feedback would be very valuable :D )
>
> [12]: https://lore.kernel.org/all/cde6bbf0b52710b33170f2787fdcb11538e40813.1741047969.git.m@maowtm.org/#iZ31include:uapi:linux:landlock.h
> [13]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.15-rc1&id=fd101da676362aaa051b4f5d8a941bd308603041
^ permalink raw reply
* Re: [PATCH v1 1/2] landlock: Fully release unused TSYNC work entries
From: Günther Noack @ 2026-02-16 21:42 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Jann Horn
In-Reply-To: <20260216.chunooXu4ahl@digikod.net>
On Mon, Feb 16, 2026 at 09:10:59PM +0100, Mickaël Salaün wrote:
> On Mon, Feb 16, 2026 at 08:57:34PM +0100, Mickaël Salaün wrote:
> > On Mon, Feb 16, 2026 at 08:33:05PM +0100, Günther Noack wrote:
> > > On Mon, Feb 16, 2026 at 06:43:25PM +0100, Mickaël Salaün wrote:
> > > > On Mon, Feb 16, 2026 at 04:25:53PM +0100, Günther Noack wrote:
> > > > > On Mon, Feb 16, 2026 at 03:26:38PM +0100, Mickaël Salaün wrote:
>
> > > > > > @@ -389,6 +389,15 @@ static bool schedule_task_work(struct tsync_works *works,
> > > > > > */
> > > > > > put_task_struct(ctx->task);
> > > > > > ctx->task = NULL;
> > > > > > + ctx->shared_ctx = NULL;
> > > > > > +
> > > > > > + /*
> > > > > > + * Cancel the tsync_works_provide() change to recycle the reserved
> > > > > > + * memory for the next thread, if any. This also ensures that
> > > > > > + * cancel_tsync_works() and tsync_works_release() do not see any
> > > > > > + * NULL task pointers.
> > > > > > + */
> > > > > > + works->size--;
> > > > >
> > > > > Looks good.
> > > > >
> > > > > [Optional code arrangement remarks:
> > > > >
> > > > > I would recommend to put that logic in a helper function
> > > > > "tsync_works_return(struct tsync_works *s, struct tsync_work *)", to
> > > > > be in line with the existing implementation where the manipulation of
> > > > > struct tsync_works is encapsulated in the "tsync_*" helper functions.
> > > > >
> > > > > The scope of that function would be to do the inverse of
> > > > > "tsync_works_provide()" -- putting the task_struct, decreasing
> > > > > works->size, and then, to be safe, also clearing the contents of the
> > > > > tsync_work struct (although that is strictly speaking not required if
> > > > > we decrease the size, I think).
> > > >
> > > > Should we move the atomic_inc() to tsync_works_provide() and the
> > > > atomic_dec() to this new helper?
> > >
> > > No, I would keep the atomic_inc() and atomic_dec() calls in the
> > > functions where they are now.
> > >
> > > The atomic counters belong logically to the synchronization scheme
> > > between the different threads, and I think it's clearer if we keep
> > > that synchronization code outside of the struct task_works
> > > abstraction.
> > >
> > > I see the struct tsync_works and its operations (functions starting
> > > with "tsync_works_") as logically belonging together in an
> > > OO/encapsulation sense, and I think it's useful to have a clear
> > > boundary of responsibilities. These functions are only in the
> > > business of managing the direct values stored in the "struct
> > > tsync_works", and in the business of allocating the memory for that
> > > data structure and incrementing refcounts to the struct task_struct.
> > > (The latter is mostly useful to have in tsync_works_provide() because
> > > the inverse put_task_struct() is useful to have in
> > > tsync_works_release(), and then it is symmetric.)
> >
> > This makes sense.
> >
> > >
> > >
> > > > > The only unusual thing about the tsync_works_return() function would
> > > > > be that it is only OK to return the very last tsync_work struct which
> > > > > was returned from tsync_works_provide().
> > > >
> > > > What about renaming tsync_works_provide() to tsync_works_push() and this
> > > > new one to tsync_works_pop()?
> > >
> > > I think I would find that naming slightly confusing: When a function
> > > is called "push", I would normally expect to pass a value to it, but
> > > we're getting one from it.
> >
> > Well, it pushes the thread and returns the wrapped object.
> >
> > > And when a method is called "pop" I would
> > > expect to get a value from it. But the inverse is true here.
> >
> > Fair
> >
> > > With
> > > the names "provide" and "return" it feel that the directionality of
> > > argument passing would be clearer.
> >
> > I don't understand the logic with "return": this tsync_works_return()
> > would not return anything.
> >
> > What about something like tsync_works_shrink()?
>
> tsync_works_trim() may be better.
The idea with "return" is that we are returning the previously
provided tsync_work item back into the struct tsync_works. But I can
see that it can be confused with C's "return" statement.
tsync_works_shrink() or tsync_works_trim() is also OK.
Other options, btw, include "reclaim()" or "recycle()", if you like
that better (these LLMs are useful as thesaurus... 8-)).
I'm fine with either name, as long as the function still puts the
task_struct of the returned task_work item. (That would be good to
keep doing, for symmetry with the _provide() and _release()
functions.)
–Günther
^ permalink raw reply
* Re: [PATCH 2/2] keys/trusted_keys: move TPM-specific fields into trusted_tpm_options
From: Srish Srinivasan @ 2026-02-17 6:52 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, keyrings, James.Bottomley, zohar, nayna, stefanb,
linux-kernel, linux-security-module, Srish Srinivasan
In-Reply-To: <aXZMLbQ2ykqPQp48@kernel.org>
Hi Jarkko,
thanks for taking a look.
And, apologies for the delayed response.
On 1/25/26 10:30 PM, Jarkko Sakkinen wrote:
> On Fri, Jan 23, 2026 at 10:25:04PM +0530, Srish Srinivasan wrote:
>> The trusted_key_options struct contains TPM-specific fields (keyhandle,
>> keyauth, blobauth_len, blobauth, pcrinfo_len, pcrinfo, pcrlock, hash,
>> policydigest_len, policydigest, and policyhandle). This leads to the
>> accumulation of backend-specific fields in the generic options structure.
>>
>> Define trusted_tpm_options structure and move the TPM-specific fields
>> there. Store a pointer to trusted_tpm_options in trusted_key_options's
>> private.
>>
>> No functional change intended.
>>
>> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>> ---
>> include/keys/trusted-type.h | 11 ---
>> include/keys/trusted_tpm.h | 14 +++
>> security/keys/trusted-keys/trusted_tpm1.c | 103 ++++++++++++++--------
>> security/keys/trusted-keys/trusted_tpm2.c | 62 ++++++++-----
>> 4 files changed, 121 insertions(+), 69 deletions(-)
>>
>> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
>> index 03527162613f..b80f250305b8 100644
>> --- a/include/keys/trusted-type.h
>> +++ b/include/keys/trusted-type.h
>> @@ -39,17 +39,6 @@ struct trusted_key_payload {
>>
>> struct trusted_key_options {
>> uint16_t keytype;
>> - uint32_t keyhandle;
>> - unsigned char keyauth[TPM_DIGEST_SIZE];
>> - uint32_t blobauth_len;
>> - unsigned char blobauth[TPM_DIGEST_SIZE];
>> - uint32_t pcrinfo_len;
>> - unsigned char pcrinfo[MAX_PCRINFO_SIZE];
>> - int pcrlock;
>> - uint32_t hash;
>> - uint32_t policydigest_len;
>> - unsigned char policydigest[MAX_DIGEST_SIZE];
>> - uint32_t policyhandle;
>> void *private;
>> };
>>
>> diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
>> index 0fadc6a4f166..355ebd36cbfd 100644
>> --- a/include/keys/trusted_tpm.h
>> +++ b/include/keys/trusted_tpm.h
>> @@ -7,6 +7,20 @@
>>
>> extern struct trusted_key_ops trusted_key_tpm_ops;
>>
>> +struct trusted_tpm_options {
>> + uint32_t keyhandle;
>> + unsigned char keyauth[TPM_DIGEST_SIZE];
>> + uint32_t blobauth_len;
>> + unsigned char blobauth[TPM_DIGEST_SIZE];
>> + uint32_t pcrinfo_len;
>> + unsigned char pcrinfo[MAX_PCRINFO_SIZE];
>> + int pcrlock;
>> + uint32_t hash;
>> + uint32_t policydigest_len;
>> + unsigned char policydigest[MAX_DIGEST_SIZE];
>> + uint32_t policyhandle;
>> +};
>> +
>> int tpm2_seal_trusted(struct tpm_chip *chip,
>> struct trusted_key_payload *payload,
>> struct trusted_key_options *options);
>> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
>> index 636acb66a4f6..0ab0234ebe37 100644
>> --- a/security/keys/trusted-keys/trusted_tpm1.c
>> +++ b/security/keys/trusted-keys/trusted_tpm1.c
>> @@ -50,12 +50,14 @@ enum {
>> #if TPM_DEBUG
>> static inline void dump_options(struct trusted_key_options *o)
>> {
>> + struct trusted_tpm_options *tpm_opts = o->private;
>
> TPM context is obvious i.e., actually private would be a better name.
Noted. Will make the change.
>
>> +
>> pr_info("sealing key type %d\n", o->keytype);
>> - pr_info("sealing key handle %0X\n", o->keyhandle);
>> - pr_info("pcrlock %d\n", o->pcrlock);
>> - pr_info("pcrinfo %d\n", o->pcrinfo_len);
>> + pr_info("sealing key handle %0X\n", tpm_opts->keyhandle);
>> + pr_info("pcrlock %d\n", tpm_opts->pcrlock);
>> + pr_info("pcrinfo %d\n", tpm_opts->pcrinfo_len);
>> print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
>> - 16, 1, o->pcrinfo, o->pcrinfo_len, 0);
>> + 16, 1, tpm_opts->pcrinfo, tpm_opts->pcrinfo_len, 0);
>> }
> Should be replaced with pr_debug() and KERN_DEBUG as precursory patch
> (and remove TPM_DEBUG).
Will fix this, and make it a preparatory clean-up patch.
>
>>
>> static inline void dump_sess(struct osapsess *s)
>> @@ -624,6 +626,7 @@ static int tpm_unseal(struct tpm_buf *tb,
>> static int key_seal(struct trusted_key_payload *p,
>> struct trusted_key_options *o)
>> {
>> + struct trusted_tpm_options *tpm_opts;
>> struct tpm_buf tb;
>> int ret;
>>
>> @@ -634,9 +637,12 @@ static int key_seal(struct trusted_key_payload *p,
>> /* include migratable flag at end of sealed key */
>> p->key[p->key_len] = p->migratable;
>>
>> - ret = tpm_seal(&tb, o->keytype, o->keyhandle, o->keyauth,
>> + tpm_opts = o->private;
> Not sure why this is not done in the declaration.
Will fix this.
>
>> +
>> + ret = tpm_seal(&tb, o->keytype, tpm_opts->keyhandle, tpm_opts->keyauth,
>> p->key, p->key_len + 1, p->blob, &p->blob_len,
>> - o->blobauth, o->pcrinfo, o->pcrinfo_len);
>> + tpm_opts->blobauth, tpm_opts->pcrinfo,
>> + tpm_opts->pcrinfo_len);
>> if (ret < 0)
>> pr_info("srkseal failed (%d)\n", ret);
>>
>> @@ -650,6 +656,7 @@ static int key_seal(struct trusted_key_payload *p,
>> static int key_unseal(struct trusted_key_payload *p,
>> struct trusted_key_options *o)
>> {
>> + struct trusted_tpm_options *tpm_opts;
>> struct tpm_buf tb;
>> int ret;
>>
>> @@ -657,8 +664,10 @@ static int key_unseal(struct trusted_key_payload *p,
>> if (ret)
>> return ret;
>>
>> - ret = tpm_unseal(&tb, o->keyhandle, o->keyauth, p->blob, p->blob_len,
>> - o->blobauth, p->key, &p->key_len);
>> + tpm_opts = o->private;
>> +
>> + ret = tpm_unseal(&tb, tpm_opts->keyhandle, tpm_opts->keyauth, p->blob,
>> + p->blob_len, tpm_opts->blobauth, p->key, &p->key_len);
>> if (ret < 0)
>> pr_info("srkunseal failed (%d)\n", ret);
>> else
>> @@ -695,6 +704,7 @@ static const match_table_t key_tokens = {
>> static int getoptions(char *c, struct trusted_key_payload *pay,
>> struct trusted_key_options *opt)
>> {
>> + struct trusted_tpm_options *tpm_opts;
>> substring_t args[MAX_OPT_ARGS];
>> char *p = c;
>> int token;
>> @@ -710,7 +720,9 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>> if (tpm2 < 0)
>> return tpm2;
>>
>> - opt->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
>> + tpm_opts = opt->private;
>> +
> I'd remove this empty line.
Will fix this.
>
>> + tpm_opts->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
>>
>> if (!c)
>> return 0;
>> @@ -724,11 +736,11 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>>
>> switch (token) {
>> case Opt_pcrinfo:
>> - opt->pcrinfo_len = strlen(args[0].from) / 2;
>> - if (opt->pcrinfo_len > MAX_PCRINFO_SIZE)
>> + tpm_opts->pcrinfo_len = strlen(args[0].from) / 2;
>> + if (tpm_opts->pcrinfo_len > MAX_PCRINFO_SIZE)
>> return -EINVAL;
>> - res = hex2bin(opt->pcrinfo, args[0].from,
>> - opt->pcrinfo_len);
>> + res = hex2bin(tpm_opts->pcrinfo, args[0].from,
>> + tpm_opts->pcrinfo_len);
>> if (res < 0)
>> return -EINVAL;
>> break;
>> @@ -737,12 +749,12 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>> if (res < 0)
>> return -EINVAL;
>> opt->keytype = SEAL_keytype;
>> - opt->keyhandle = handle;
>> + tpm_opts->keyhandle = handle;
>> break;
>> case Opt_keyauth:
>> if (strlen(args[0].from) != 2 * SHA1_DIGEST_SIZE)
>> return -EINVAL;
>> - res = hex2bin(opt->keyauth, args[0].from,
>> + res = hex2bin(tpm_opts->keyauth, args[0].from,
>> SHA1_DIGEST_SIZE);
>> if (res < 0)
>> return -EINVAL;
>> @@ -753,21 +765,23 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>> * hex strings. TPM 2.0 authorizations are simple
>> * passwords (although it can take a hash as well)
>> */
>> - opt->blobauth_len = strlen(args[0].from);
>> + tpm_opts->blobauth_len = strlen(args[0].from);
>>
>> - if (opt->blobauth_len == 2 * TPM_DIGEST_SIZE) {
>> - res = hex2bin(opt->blobauth, args[0].from,
>> + if (tpm_opts->blobauth_len == 2 * TPM_DIGEST_SIZE) {
>> + res = hex2bin(tpm_opts->blobauth, args[0].from,
>> TPM_DIGEST_SIZE);
>> if (res < 0)
>> return -EINVAL;
>>
>> - opt->blobauth_len = TPM_DIGEST_SIZE;
>> + tpm_opts->blobauth_len = TPM_DIGEST_SIZE;
>> break;
>> }
>>
>> - if (tpm2 && opt->blobauth_len <= sizeof(opt->blobauth)) {
>> - memcpy(opt->blobauth, args[0].from,
>> - opt->blobauth_len);
>> + if (tpm2 &&
>> + tpm_opts->blobauth_len <=
>> + sizeof(tpm_opts->blobauth)) {
>> + memcpy(tpm_opts->blobauth, args[0].from,
>> + tpm_opts->blobauth_len);
>> break;
>> }
>>
>> @@ -785,14 +799,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>> res = kstrtoul(args[0].from, 10, &lock);
>> if (res < 0)
>> return -EINVAL;
>> - opt->pcrlock = lock;
>> + tpm_opts->pcrlock = lock;
>> break;
>> case Opt_hash:
>> if (test_bit(Opt_policydigest, &token_mask))
>> return -EINVAL;
>> for (i = 0; i < HASH_ALGO__LAST; i++) {
>> if (!strcmp(args[0].from, hash_algo_name[i])) {
>> - opt->hash = i;
>> + tpm_opts->hash = i;
>> break;
>> }
>> }
>> @@ -804,14 +818,14 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>> }
>> break;
>> case Opt_policydigest:
>> - digest_len = hash_digest_size[opt->hash];
>> + digest_len = hash_digest_size[tpm_opts->hash];
>> if (!tpm2 || strlen(args[0].from) != (2 * digest_len))
>> return -EINVAL;
>> - res = hex2bin(opt->policydigest, args[0].from,
>> + res = hex2bin(tpm_opts->policydigest, args[0].from,
>> digest_len);
>> if (res < 0)
>> return -EINVAL;
>> - opt->policydigest_len = digest_len;
>> + tpm_opts->policydigest_len = digest_len;
>> break;
>> case Opt_policyhandle:
>> if (!tpm2)
>> @@ -819,7 +833,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>> res = kstrtoul(args[0].from, 16, &handle);
>> if (res < 0)
>> return -EINVAL;
>> - opt->policyhandle = handle;
>> + tpm_opts->policyhandle = handle;
>> break;
>> default:
>> return -EINVAL;
>> @@ -830,6 +844,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
>>
>> static struct trusted_key_options *trusted_options_alloc(void)
>> {
>> + struct trusted_tpm_options *tpm_opts;
>> struct trusted_key_options *options;
>> int tpm2;
>>
>> @@ -842,14 +857,23 @@ static struct trusted_key_options *trusted_options_alloc(void)
>> /* set any non-zero defaults */
>> options->keytype = SRK_keytype;
>>
>> - if (!tpm2)
>> - options->keyhandle = SRKHANDLE;
>> + tpm_opts = kzalloc(sizeof(*tpm_opts), GFP_KERNEL);
>> + if (!tpm_opts) {
>> + kfree_sensitive(options);
>> + options = NULL;
>> + } else {
>> + if (!tpm2)
>> + tpm_opts->keyhandle = SRKHANDLE;
>> +
>> + options->private = tpm_opts;
>> + }
>> }
>> return options;
>> }
>>
>> static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
>> {
>> + struct trusted_tpm_options *tpm_opts = NULL;
>> struct trusted_key_options *options = NULL;
>> int ret = 0;
>> int tpm2;
>> @@ -867,7 +891,9 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
>> goto out;
>> dump_options(options);
>>
>> - if (!options->keyhandle && !tpm2) {
>> + tpm_opts = options->private;
>> +
>> + if (!tpm_opts->keyhandle && !tpm2) {
>> ret = -EINVAL;
>> goto out;
>> }
>> @@ -881,20 +907,22 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
>> goto out;
>> }
>>
>> - if (options->pcrlock) {
>> - ret = pcrlock(options->pcrlock);
>> + if (tpm_opts->pcrlock) {
>> + ret = pcrlock(tpm_opts->pcrlock);
>> if (ret < 0) {
>> pr_info("pcrlock failed (%d)\n", ret);
>> goto out;
>> }
>> }
>> out:
>> + kfree_sensitive(options->private);
>> kfree_sensitive(options);
>> return ret;
>> }
>>
>> static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
>> {
>> + struct trusted_tpm_options *tpm_opts = NULL;
>> struct trusted_key_options *options = NULL;
>> int ret = 0;
>> int tpm2;
>> @@ -912,7 +940,9 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
>> goto out;
>> dump_options(options);
>>
>> - if (!options->keyhandle && !tpm2) {
>> + tpm_opts = options->private;
>> +
>> + if (!tpm_opts->keyhandle && !tpm2) {
>> ret = -EINVAL;
>> goto out;
>> }
>> @@ -924,14 +954,15 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
>> if (ret < 0)
>> pr_info("key_unseal failed (%d)\n", ret);
>>
>> - if (options->pcrlock) {
>> - ret = pcrlock(options->pcrlock);
>> + if (tpm_opts->pcrlock) {
>> + ret = pcrlock(tpm_opts->pcrlock);
>> if (ret < 0) {
>> pr_info("pcrlock failed (%d)\n", ret);
>> goto out;
>> }
>> }
>> out:
>> + kfree_sensitive(options->private);
>> kfree_sensitive(options);
>> return ret;
>> }
>> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
>> index 6340823f8b53..568c4af9010c 100644
>> --- a/security/keys/trusted-keys/trusted_tpm2.c
>> +++ b/security/keys/trusted-keys/trusted_tpm2.c
>> @@ -24,6 +24,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>> struct trusted_key_options *options,
>> u8 *src, u32 len)
>> {
>> + struct trusted_tpm_options *tpm_opts;
>> const int SCRATCH_SIZE = PAGE_SIZE;
>> u8 *scratch = kmalloc(SCRATCH_SIZE, GFP_KERNEL);
>> u8 *work = scratch, *work1;
>> @@ -46,7 +47,9 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>> work = asn1_encode_oid(work, end_work, tpm2key_oid,
>> asn1_oid_len(tpm2key_oid));
>>
>> - if (options->blobauth_len == 0) {
>> + tpm_opts = options->private;
>> +
>> + if (tpm_opts->blobauth_len == 0) {
>> unsigned char bool[3], *w = bool;
>> /* tag 0 is emptyAuth */
>> w = asn1_encode_boolean(w, w + sizeof(bool), true);
>> @@ -69,7 +72,7 @@ static int tpm2_key_encode(struct trusted_key_payload *payload,
>> goto err;
>> }
>>
>> - work = asn1_encode_integer(work, end_work, options->keyhandle);
>> + work = asn1_encode_integer(work, end_work, tpm_opts->keyhandle);
>> work = asn1_encode_octet_string(work, end_work, pub, pub_len);
>> work = asn1_encode_octet_string(work, end_work, priv, priv_len);
>>
>> @@ -102,6 +105,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
>> struct trusted_key_options *options,
>> u8 **buf)
>> {
>> + struct trusted_tpm_options *tpm_opts;
>> int ret;
>> struct tpm2_key_context ctx;
>> u8 *blob;
>> @@ -120,8 +124,10 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
>> if (!blob)
>> return -ENOMEM;
>>
>> + tpm_opts = options->private;
>> +
>> *buf = blob;
>> - options->keyhandle = ctx.parent;
>> + tpm_opts->keyhandle = ctx.parent;
>>
>> memcpy(blob, ctx.priv, ctx.priv_len);
>> blob += ctx.priv_len;
>> @@ -233,6 +239,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>> struct trusted_key_payload *payload,
>> struct trusted_key_options *options)
>> {
>> + struct trusted_tpm_options *tpm_opts;
>> off_t offset = TPM_HEADER_SIZE;
>> struct tpm_buf buf, sized;
>> int blob_len = 0;
>> @@ -240,11 +247,13 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>> u32 flags;
>> int rc;
>>
>> - hash = tpm2_find_hash_alg(options->hash);
>> + tpm_opts = options->private;
>> +
>> + hash = tpm2_find_hash_alg(tpm_opts->hash);
>> if (hash < 0)
>> return hash;
>>
>> - if (!options->keyhandle)
>> + if (!tpm_opts->keyhandle)
>> return -EINVAL;
>>
>> rc = tpm_try_get_ops(chip);
>> @@ -268,18 +277,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>> goto out_put;
>> }
>>
>> - rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
>> + rc = tpm_buf_append_name(chip, &buf, tpm_opts->keyhandle, NULL);
>> if (rc)
>> goto out;
>>
>> tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
>> - options->keyauth, TPM_DIGEST_SIZE);
>> + tpm_opts->keyauth, TPM_DIGEST_SIZE);
>>
>> /* sensitive */
>> - tpm_buf_append_u16(&sized, options->blobauth_len);
>> + tpm_buf_append_u16(&sized, tpm_opts->blobauth_len);
>>
>> - if (options->blobauth_len)
>> - tpm_buf_append(&sized, options->blobauth, options->blobauth_len);
>> + if (tpm_opts->blobauth_len)
>> + tpm_buf_append(&sized, tpm_opts->blobauth,
>> + tpm_opts->blobauth_len);
>>
>> tpm_buf_append_u16(&sized, payload->key_len);
>> tpm_buf_append(&sized, payload->key, payload->key_len);
>> @@ -292,14 +302,15 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>>
>> /* key properties */
>> flags = 0;
>> - flags |= options->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
>> + flags |= tpm_opts->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
>> flags |= payload->migratable ? 0 : (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT);
>> tpm_buf_append_u32(&sized, flags);
>>
>> /* policy */
>> - tpm_buf_append_u16(&sized, options->policydigest_len);
>> - if (options->policydigest_len)
>> - tpm_buf_append(&sized, options->policydigest, options->policydigest_len);
>> + tpm_buf_append_u16(&sized, tpm_opts->policydigest_len);
>> + if (tpm_opts->policydigest_len)
>> + tpm_buf_append(&sized, tpm_opts->policydigest,
>> + tpm_opts->policydigest_len);
>>
>> /* public parameters */
>> tpm_buf_append_u16(&sized, TPM_ALG_NULL);
>> @@ -373,6 +384,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>> u32 *blob_handle)
>> {
>> u8 *blob_ref __free(kfree) = NULL;
>> + struct trusted_tpm_options *tpm_opts;
>> struct tpm_buf buf;
>> unsigned int private_len;
>> unsigned int public_len;
>> @@ -391,8 +403,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>> blob_ref = blob;
>> }
>>
>> + tpm_opts = options->private;
>> +
>> /* new format carries keyhandle but old format doesn't */
>> - if (!options->keyhandle)
>> + if (!tpm_opts->keyhandle)
>> return -EINVAL;
>>
>> /* must be big enough for at least the two be16 size counts */
>> @@ -433,11 +447,11 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>> return rc;
>> }
>>
>> - rc = tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
>> + rc = tpm_buf_append_name(chip, &buf, tpm_opts->keyhandle, NULL);
>> if (rc)
>> goto out;
>>
>> - tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
>> + tpm_buf_append_hmac_session(chip, &buf, 0, tpm_opts->keyauth,
>> TPM_DIGEST_SIZE);
>>
>> tpm_buf_append(&buf, blob, blob_len);
>> @@ -481,6 +495,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>> struct trusted_key_options *options,
>> u32 blob_handle)
>> {
>> + struct trusted_tpm_options *tpm_opts;
>> struct tpm_header *head;
>> struct tpm_buf buf;
>> u16 data_len;
>> @@ -502,10 +517,12 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>> if (rc)
>> goto out;
>>
>> - if (!options->policyhandle) {
>> + tpm_opts = options->private;
>> +
>> + if (!tpm_opts->policyhandle) {
>> tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
>> - options->blobauth,
>> - options->blobauth_len);
>> + tpm_opts->blobauth,
>> + tpm_opts->blobauth_len);
>> } else {
>> /*
>> * FIXME: The policy session was generated outside the
>> @@ -518,9 +535,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>> * could repeat our actions with the exfiltrated
>> * password.
>> */
>> - tpm2_buf_append_auth(&buf, options->policyhandle,
>> + tpm2_buf_append_auth(&buf, tpm_opts->policyhandle,
>> NULL /* nonce */, 0, 0,
>> - options->blobauth, options->blobauth_len);
>> + tpm_opts->blobauth,
>> + tpm_opts->blobauth_len);
>> if (tpm2_chip_auth(chip)) {
>> tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
>> } else {
>> --
>> 2.43.0
>>
> BR, Jarkko
I will shortly send out v2 with the changes.
Thanks,
Srish.
^ permalink raw reply
* Re: [PATCH RFC] security: add LSM blob and hooks for namespaces
From: Christian Brauner @ 2026-02-17 8:54 UTC (permalink / raw)
To: Paul Moore; +Cc: James Morris, linux-security-module, linux-kernel
In-Reply-To: <19c67cca5d8.2843.85c95baa4474aabc7814e68940a78392@paul-moore.com>
On Mon, Feb 16, 2026 at 07:53:11PM +0100, Paul Moore wrote:
> On February 16, 2026 2:52:34 PM Christian Brauner <brauner@kernel.org> wrote:
> > All namespace types now share the same ns_common infrastructure. Extend
> > this to include a security blob so LSMs can start managing namespaces
> > uniformly without having to add one-off hooks or security fields to
> > every individual namespace type.
> >
> > Add a ns_security pointer to ns_common and the corresponding lbs_ns
> > blob size to lsm_blob_sizes. Allocation and freeing hooks are called
> > from the common __ns_common_init() and __ns_common_free() paths so
> > every namespace type gets covered in one go. All information about the
> > namespace type and the appropriate casting helpers to get at the
> > containing namespace are available via ns_common making it
> > straightforward for LSMs to differentiate when they need to.
> >
> > A namespace_install hook is called from validate_ns() during setns(2)
> > giving LSMs a chance to enforce policy on namespace transitions.
> >
> > Individual namespace types can still have their own specialized security
> > hooks when needed. This is just the common baseline that makes it easy
> > to track and manage namespaces from the security side without requiring
> > every namespace type to reinvent the wheel.
> >
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> > include/linux/lsm_hook_defs.h | 3 ++
> > include/linux/lsm_hooks.h | 1 +
> > include/linux/ns/ns_common_types.h | 3 ++
> > include/linux/security.h | 20 ++++++++++
> > kernel/nscommon.c | 12 ++++++
> > kernel/nsproxy.c | 8 +++-
> > security/lsm_init.c | 2 +
> > security/security.c | 76 ++++++++++++++++++++++++++++++++++++++
> > 8 files changed, 124 insertions(+), 1 deletion(-)
>
> I still have limited network access for a few more days, but a couple of
> quick comments in no particular order ...
>
> Generally speaking we don't add things to the LSM interface without a user,
> and I can't think of a good reason why we would want to do things
> differently here. This means that when you propose something like this you
> should also propose an addition to one of the in-tree LSMs to make use of
> it. While the guidance doc linked below (also linked in the LSM MAINTAINERS
> entry) doesn't have any guidance for the LSM blobs as they are generally a
> byproduct of the hooks, if you are looking for some general info I think the
> bits on adding a new LSM hook would be very close to what we would expect
> for blob additions.
>
> https://github.com/LinuxSecurityModule/kernel/blob/main/README.md
>
> Getting to the specifics of namespace related APIs, we've had a lot of
> discussions about namespacing and my current opinion is that we need to sort
> out if we want a userspace API at the LSM framework layer, or if we want to
> do that at the individual LSM layer; there is a lot of nuance there and
> while one option may seem like an obvious choice, we need some more
> discussion and I need a chance to get caught up on the threads. Once we have
> an API decision then we can start sorting out the implementation details
> like the LSM blobs.
I might be misunderstanding you but what you are talking about seems
namespacing the LSM layer itself.
But I cannot stress enough this is not at all what this patchset is
doing. :)
All that this patchset does is add new security hooks that get called
whenever a new pid, net, mount, ipc, ... namespace is created, freed, or
when someone wants to change into another namespace. We plan on using
this in systemd to supervise and track services.
^ permalink raw reply
* Re: [PATCH RFC] security: add LSM blob and hooks for namespaces
From: Christian Brauner @ 2026-02-17 9:38 UTC (permalink / raw)
To: Casey Schaufler
Cc: Paul Moore, James Morris, linux-security-module, linux-kernel
In-Reply-To: <fb40e938-f6b2-45cf-b8ab-246cd6939582@schaufler-ca.com>
On Mon, Feb 16, 2026 at 09:34:57AM -0800, Casey Schaufler wrote:
> On 2/16/2026 5:52 AM, Christian Brauner wrote:
> > All namespace types now share the same ns_common infrastructure. Extend
> > this to include a security blob so LSMs can start managing namespaces
> > uniformly without having to add one-off hooks or security fields to
> > every individual namespace type.
>
> The implementation appears sound.
>
> I have to question whether having LSM controls on namespaces is reasonable.
This is already in active use today but only in a very limited capacity.
This generalizes it.
> I suppose that you could have a system where (for example) SELinux runs
> in permissive mode except within a specific user namespace, where it would
> enforce policy. Do you have a use case in mind?
We will use it in systemd services and containers to monitor and
supervise namespaces.
^ permalink raw reply
* Re: [PATCH 0/2] fanotify: avid some premature LSM checks
From: Jan Kara @ 2026-02-17 11:09 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Jan Kara, Amir Goldstein, Matthew Bobrowski, linux-fsdevel,
linux-security-module, selinux, linux-kernel
In-Reply-To: <20260216150625.793013-1-omosnace@redhat.com>
On Mon 16-02-26 16:06:23, Ondrej Mosnacek wrote:
> Restructure some of the validity and security checks in
> fs/notify/fanotify/fanotify_user.c to avoid generating LSM access
> denials in the audit log where hey shouldn't be.
>
> Ondrej Mosnacek (2):
> fanotify: avoid/silence premature LSM capability checks
> fanotify: call fanotify_events_supported() before path_permission()
> and security_path_notify()
>
> fs/notify/fanotify/fanotify_user.c | 50 ++++++++++++++----------------
> 1 file changed, 23 insertions(+), 27 deletions(-)
The series looks good to me as well. Thanks! I'll commit the series to my
tree once the merge window closes and fixup the comment formatting on
commit. No need to resend.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH RFC] security: add LSM blob and hooks for namespaces
From: Paul Moore @ 2026-02-17 11:33 UTC (permalink / raw)
To: Christian Brauner; +Cc: James Morris, linux-security-module, linux-kernel
In-Reply-To: <20260217-armer-wegfielen-ffb2cdc60283@brauner>
On February 17, 2026 9:54:42 AM Christian Brauner <brauner@kernel.org> wrote:
> On Mon, Feb 16, 2026 at 07:53:11PM +0100, Paul Moore wrote:
>> On February 16, 2026 2:52:34 PM Christian Brauner <brauner@kernel.org> wrote:
>>> All namespace types now share the same ns_common infrastructure. Extend
>>> this to include a security blob so LSMs can start managing namespaces
>>> uniformly without having to add one-off hooks or security fields to
>>> every individual namespace type.
>>>
>>> Add a ns_security pointer to ns_common and the corresponding lbs_ns
>>> blob size to lsm_blob_sizes. Allocation and freeing hooks are called
>>> from the common __ns_common_init() and __ns_common_free() paths so
>>> every namespace type gets covered in one go. All information about the
>>> namespace type and the appropriate casting helpers to get at the
>>> containing namespace are available via ns_common making it
>>> straightforward for LSMs to differentiate when they need to.
>>>
>>> A namespace_install hook is called from validate_ns() during setns(2)
>>> giving LSMs a chance to enforce policy on namespace transitions.
>>>
>>> Individual namespace types can still have their own specialized security
>>> hooks when needed. This is just the common baseline that makes it easy
>>> to track and manage namespaces from the security side without requiring
>>> every namespace type to reinvent the wheel.
>>>
>>> Signed-off-by: Christian Brauner <brauner@kernel.org>
>>> ---
>>> include/linux/lsm_hook_defs.h | 3 ++
>>> include/linux/lsm_hooks.h | 1 +
>>> include/linux/ns/ns_common_types.h | 3 ++
>>> include/linux/security.h | 20 ++++++++++
>>> kernel/nscommon.c | 12 ++++++
>>> kernel/nsproxy.c | 8 +++-
>>> security/lsm_init.c | 2 +
>>> security/security.c | 76 ++++++++++++++++++++++++++++++++++++++
>>> 8 files changed, 124 insertions(+), 1 deletion(-)
>>
>> I still have limited network access for a few more days, but a couple of
>> quick comments in no particular order ...
>>
>> Generally speaking we don't add things to the LSM interface without a user,
>> and I can't think of a good reason why we would want to do things
>> differently here. This means that when you propose something like this you
>> should also propose an addition to one of the in-tree LSMs to make use of
>> it. While the guidance doc linked below (also linked in the LSM MAINTAINERS
>> entry) doesn't have any guidance for the LSM blobs as they are generally a
>> byproduct of the hooks, if you are looking for some general info I think the
>> bits on adding a new LSM hook would be very close to what we would expect
>> for blob additions.
>>
>> https://github.com/LinuxSecurityModule/kernel/blob/main/README.md
>>
>> Getting to the specifics of namespace related APIs, we've had a lot of
>> discussions about namespacing and my current opinion is that we need to sort
>> out if we want a userspace API at the LSM framework layer, or if we want to
>> do that at the individual LSM layer; there is a lot of nuance there and
>> while one option may seem like an obvious choice, we need some more
>> discussion and I need a chance to get caught up on the threads. Once we have
>> an API decision then we can start sorting out the implementation details
>> like the LSM blobs.
>
> I might be misunderstanding you but what you are talking about seems
> namespacing the LSM layer itself.
>
> But I cannot stress enough this is not at all what this patchset is
> doing. :)
Likely also a misunderstanding on my end as I triage email/patches via phone.
Regardless, the guidance in the doc I linked regarding the addition of new
LSM hooks would appear to apply here.
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH] fs/pidfs: Add permission check to pidfd_info()
From: Christian Brauner @ 2026-02-17 12:01 UTC (permalink / raw)
To: Daniel Durning
Cc: linux-fsdevel, linux-security-module, selinux, viro, jack, paul,
stephen.smalley.work, omosnace, Oleg Nesterov
In-Reply-To: <CAKrb_fEXR0uQnX5iK-ACH=amKMQ8qBSPGXmJb=1PgvEq8qsDEQ@mail.gmail.com>
On Wed, Feb 11, 2026 at 02:43:21PM -0500, Daniel Durning wrote:
> On Mon, Feb 9, 2026 at 9:01 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Fri, Feb 06, 2026 at 06:02:48PM +0000, danieldurning.work@gmail.com wrote:
> > > From: Daniel Durning <danieldurning.work@gmail.com>
> > >
> > > Added a permission check to pidfd_info(). Originally, process info
> > > could be retrieved with a pidfd even if proc was mounted with hidepid
> > > enabled, allowing pidfds to be used to bypass those protections. We
> > > now call ptrace_may_access() to perform some DAC checking as well
> > > as call the appropriate LSM hook.
> > >
> > > The downside to this approach is that there are now more restrictions
> > > on accessing this info from a pidfd than when just using proc (without
> > > hidepid). I am open to suggestions if anyone can think of a better way
> > > to handle this.
> >
> > This isn't really workable since this would regress userspace quite a
> > bit. I think we need a different approach. I've given it some thought
> > and everything's kinda ugly but this might work.
> >
> > In struct pid_namespace record whether anyone ever mounted a procfs
> > with hidepid turned on for this pidns. In pidfd_info() we check whether
> > hidepid was ever turned on. If it wasn't we're done and can just return
> > the info. This will be the common case. If hidepid was ever turned on
> > use kern_path("/proc") to lookup procfs. If not found check
> > ptrace_may_access() to decide whether to return the info or not. If
> > /proc is found check it's hidepid settings and make a decision based on
> > that.
> >
> > You can probably reorder this to call ptrace_may_access() first and then
> > do the procfs lookup dance. Thoughts?
>
> Thanks for the feedback. I think your solution makes sense.
>
> Unfortunately, it seems like systemd mounts procfs with hidepid enabled on
> boot for services with the ProtectProc option enabled. This means that
> procfs will always have been mounted with hidepid in the init pid namespace.
> Do you think it would be viable to record whether or not procfs was mounted
> with hidepid enabled in the mount namespace instead?
I guess we can see what it looks like.
^ permalink raw reply
* [PATCH v2 2/2] landlock: Improve TSYNC types
From: Mickaël Salaün @ 2026-02-17 12:23 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, linux-security-module, Jann Horn
In-Reply-To: <20260217122341.2359582-1-mic@digikod.net>
Constify pointers when it makes sense.
Consistently use size_t for loops, especially to match works->size type.
Add new lines to improve readability.
Cc: Jann Horn <jannh@google.com>
Reviewed-by: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
Changes since v1:
- Added Reviewed-by Günther.
---
security/landlock/tsync.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
index 42cc0ef0c704..c588cdd111d3 100644
--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -290,13 +290,14 @@ static int tsync_works_grow_by(struct tsync_works *s, size_t n, gfp_t flags)
* tsync_works_contains - checks for presence of task in s
*/
static bool tsync_works_contains_task(const struct tsync_works *s,
- struct task_struct *task)
+ const struct task_struct *task)
{
size_t i;
for (i = 0; i < s->size; i++)
if (s->works[i]->task == task)
return true;
+
return false;
}
@@ -318,6 +319,7 @@ static void tsync_works_release(struct tsync_works *s)
for (i = 0; i < s->capacity; i++)
kfree(s->works[i]);
+
kfree(s->works);
s->works = NULL;
s->size = 0;
@@ -329,7 +331,7 @@ static void tsync_works_release(struct tsync_works *s)
*/
static size_t count_additional_threads(const struct tsync_works *works)
{
- struct task_struct *thread, *caller;
+ const struct task_struct *caller, *thread;
size_t n = 0;
caller = current;
@@ -368,7 +370,8 @@ static bool schedule_task_work(struct tsync_works *works,
struct tsync_shared_context *shared_ctx)
{
int err;
- struct task_struct *thread, *caller;
+ const struct task_struct *caller;
+ struct task_struct *thread;
struct tsync_work *ctx;
bool found_more_threads = false;
@@ -438,10 +441,10 @@ static bool schedule_task_work(struct tsync_works *works,
* shared_ctx->num_preparing and shared_ctx->num_unfished and mark the two
* completions if needed, as if the task was never scheduled.
*/
-static void cancel_tsync_works(struct tsync_works *works,
+static void cancel_tsync_works(const struct tsync_works *works,
struct tsync_shared_context *shared_ctx)
{
- int i;
+ size_t i;
for (i = 0; i < works->size; i++) {
if (WARN_ON_ONCE(!works->works[i]->task))
--
2.53.0
^ permalink raw reply related
* [PATCH v2 1/2] landlock: Fully release unused TSYNC work entries
From: Mickaël Salaün @ 2026-02-17 12:23 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, linux-security-module, Jann Horn
If task_work_add() failed, ctx->task is put but the tsync_works struct
is not reset to its previous state. The first consequence is that the
kernel allocates memory for dying threads, which could lead to
user-accounted memory exhaustion (not very useful nor specific to this
case). The second consequence is that task_work_cancel(), called by
cancel_tsync_works(), can dereference a NULL task pointer.
Fix this issues by keeping a consistent works->size wrt the added task
work. This is done in a new tsync_works_trim() helper which also cleans
up the shared_ctx and work fields.
As a safeguard, add a pointer check to cancel_tsync_works() and update
tsync_works_release() accordingly.
Cc: Günther Noack <gnoack@google.com>
Cc: Jann Horn <jannh@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
Changes since v1:
https://lore.kernel.org/all/20260216142641.2100407-1-mic@digikod.net/
- Move the return/release logic into a new tsync_works_trim() helper
(suggested by Günther).
- Reset the whole ctx with memset().
- Add an unlinkely(err).
---
security/landlock/tsync.c | 47 ++++++++++++++++++++++++++++++++++-----
1 file changed, 41 insertions(+), 6 deletions(-)
diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
index 0d2b9c646030..42cc0ef0c704 100644
--- a/security/landlock/tsync.c
+++ b/security/landlock/tsync.c
@@ -203,6 +203,40 @@ static struct tsync_work *tsync_works_provide(struct tsync_works *s,
return ctx;
}
+/**
+ * tsync_works_trim - Put the last tsync_work element
+ *
+ * @s: TSYNC works to trim.
+ *
+ * Put the last task and decrement the size of @s.
+ *
+ * This helper does not cancel a running task, but just reset the last element
+ * to zero.
+ */
+static void tsync_works_trim(struct tsync_works *s)
+{
+ struct tsync_work *ctx;
+
+ if (WARN_ON_ONCE(s->size <= 0))
+ return;
+
+ ctx = s->works[s->size - 1];
+
+ /*
+ * For consistency, remove the task from ctx so that it does not look like
+ * we handed it a task_work.
+ */
+ put_task_struct(ctx->task);
+ memset(ctx, 0, sizeof(*ctx));
+
+ /*
+ * Cancel the tsync_works_provide() change to recycle the reserved memory
+ * for the next thread, if any. This also ensures that cancel_tsync_works()
+ * and tsync_works_release() do not see any NULL task pointers.
+ */
+ s->size--;
+}
+
/*
* tsync_works_grow_by - preallocates space for n more contexts in s
*
@@ -276,7 +310,7 @@ static void tsync_works_release(struct tsync_works *s)
size_t i;
for (i = 0; i < s->size; i++) {
- if (!s->works[i]->task)
+ if (WARN_ON_ONCE(!s->works[i]->task))
continue;
put_task_struct(s->works[i]->task);
@@ -379,16 +413,14 @@ static bool schedule_task_work(struct tsync_works *works,
init_task_work(&ctx->work, restrict_one_thread_callback);
err = task_work_add(thread, &ctx->work, TWA_SIGNAL);
- if (err) {
+ if (unlikely(err)) {
/*
* task_work_add() only fails if the task is about to exit. We
* checked that earlier, but it can happen as a race. Resume
* without setting an error, as the task is probably gone in the
- * next loop iteration. For consistency, remove the task from ctx
- * so that it does not look like we handed it a task_work.
+ * next loop iteration.
*/
- put_task_struct(ctx->task);
- ctx->task = NULL;
+ tsync_works_trim(works);
atomic_dec(&shared_ctx->num_preparing);
atomic_dec(&shared_ctx->num_unfinished);
@@ -412,6 +444,9 @@ static void cancel_tsync_works(struct tsync_works *works,
int i;
for (i = 0; i < works->size; i++) {
+ if (WARN_ON_ONCE(!works->works[i]->task))
+ continue;
+
if (!task_work_cancel(works->works[i]->task,
&works->works[i]->work))
continue;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2 1/2] landlock: Fully release unused TSYNC work entries
From: Günther Noack @ 2026-02-17 12:41 UTC (permalink / raw)
To: Mickaël Salaün; +Cc: linux-security-module, Jann Horn
In-Reply-To: <20260217122341.2359582-1-mic@digikod.net>
On Tue, Feb 17, 2026 at 01:23:39PM +0100, Mickaël Salaün wrote:
> If task_work_add() failed, ctx->task is put but the tsync_works struct
> is not reset to its previous state. The first consequence is that the
> kernel allocates memory for dying threads, which could lead to
> user-accounted memory exhaustion (not very useful nor specific to this
> case). The second consequence is that task_work_cancel(), called by
> cancel_tsync_works(), can dereference a NULL task pointer.
>
> Fix this issues by keeping a consistent works->size wrt the added task
> work. This is done in a new tsync_works_trim() helper which also cleans
> up the shared_ctx and work fields.
>
> As a safeguard, add a pointer check to cancel_tsync_works() and update
> tsync_works_release() accordingly.
>
> Cc: Günther Noack <gnoack@google.com>
> Cc: Jann Horn <jannh@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>
> Changes since v1:
> https://lore.kernel.org/all/20260216142641.2100407-1-mic@digikod.net/
> - Move the return/release logic into a new tsync_works_trim() helper
> (suggested by Günther).
> - Reset the whole ctx with memset().
> - Add an unlinkely(err).
> ---
> security/landlock/tsync.c | 47 ++++++++++++++++++++++++++++++++++-----
> 1 file changed, 41 insertions(+), 6 deletions(-)
>
> diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> index 0d2b9c646030..42cc0ef0c704 100644
> --- a/security/landlock/tsync.c
> +++ b/security/landlock/tsync.c
> @@ -203,6 +203,40 @@ static struct tsync_work *tsync_works_provide(struct tsync_works *s,
> return ctx;
> }
>
> +/**
> + * tsync_works_trim - Put the last tsync_work element
> + *
> + * @s: TSYNC works to trim.
> + *
> + * Put the last task and decrement the size of @s.
> + *
> + * This helper does not cancel a running task, but just reset the last element
> + * to zero.
> + */
> +static void tsync_works_trim(struct tsync_works *s)
> +{
> + struct tsync_work *ctx;
> +
> + if (WARN_ON_ONCE(s->size <= 0))
> + return;
> +
> + ctx = s->works[s->size - 1];
> +
> + /*
> + * For consistency, remove the task from ctx so that it does not look like
> + * we handed it a task_work.
> + */
> + put_task_struct(ctx->task);
> + memset(ctx, 0, sizeof(*ctx));
Minor (and highly optional) remark, this is the same as
*ctx = (struct tsync_work){};
which I find slightly easier to read when resetting a struct value.
Both is fine though.
> +
> + /*
> + * Cancel the tsync_works_provide() change to recycle the reserved memory
> + * for the next thread, if any. This also ensures that cancel_tsync_works()
> + * and tsync_works_release() do not see any NULL task pointers.
> + */
> + s->size--;
> +}
> +
> /*
> * tsync_works_grow_by - preallocates space for n more contexts in s
> *
> @@ -276,7 +310,7 @@ static void tsync_works_release(struct tsync_works *s)
> size_t i;
>
> for (i = 0; i < s->size; i++) {
> - if (!s->works[i]->task)
> + if (WARN_ON_ONCE(!s->works[i]->task))
> continue;
>
> put_task_struct(s->works[i]->task);
> @@ -379,16 +413,14 @@ static bool schedule_task_work(struct tsync_works *works,
>
> init_task_work(&ctx->work, restrict_one_thread_callback);
> err = task_work_add(thread, &ctx->work, TWA_SIGNAL);
> - if (err) {
> + if (unlikely(err)) {
> /*
> * task_work_add() only fails if the task is about to exit. We
> * checked that earlier, but it can happen as a race. Resume
> * without setting an error, as the task is probably gone in the
> - * next loop iteration. For consistency, remove the task from ctx
> - * so that it does not look like we handed it a task_work.
> + * next loop iteration.
> */
> - put_task_struct(ctx->task);
> - ctx->task = NULL;
> + tsync_works_trim(works);
>
> atomic_dec(&shared_ctx->num_preparing);
> atomic_dec(&shared_ctx->num_unfinished);
> @@ -412,6 +444,9 @@ static void cancel_tsync_works(struct tsync_works *works,
> int i;
>
> for (i = 0; i < works->size; i++) {
> + if (WARN_ON_ONCE(!works->works[i]->task))
> + continue;
> +
> if (!task_work_cancel(works->works[i]->task,
> &works->works[i]->work))
> continue;
> --
> 2.53.0
>
Reviewed-by: Günther Noack <gnoack@google.com>
Thanks for spotting and fixing this!
—Günther
^ permalink raw reply
* Re: [PATCH v2 1/2] landlock: Fully release unused TSYNC work entries
From: Mickaël Salaün @ 2026-02-17 13:52 UTC (permalink / raw)
To: Günther Noack; +Cc: linux-security-module, Jann Horn
In-Reply-To: <aZRh52TIPAmMPJxc@google.com>
On Tue, Feb 17, 2026 at 01:41:11PM +0100, Günther Noack wrote:
> On Tue, Feb 17, 2026 at 01:23:39PM +0100, Mickaël Salaün wrote:
> > If task_work_add() failed, ctx->task is put but the tsync_works struct
> > is not reset to its previous state. The first consequence is that the
> > kernel allocates memory for dying threads, which could lead to
> > user-accounted memory exhaustion (not very useful nor specific to this
> > case). The second consequence is that task_work_cancel(), called by
> > cancel_tsync_works(), can dereference a NULL task pointer.
> >
> > Fix this issues by keeping a consistent works->size wrt the added task
> > work. This is done in a new tsync_works_trim() helper which also cleans
> > up the shared_ctx and work fields.
> >
> > As a safeguard, add a pointer check to cancel_tsync_works() and update
> > tsync_works_release() accordingly.
> >
> > Cc: Günther Noack <gnoack@google.com>
> > Cc: Jann Horn <jannh@google.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> >
> > Changes since v1:
> > https://lore.kernel.org/all/20260216142641.2100407-1-mic@digikod.net/
> > - Move the return/release logic into a new tsync_works_trim() helper
> > (suggested by Günther).
> > - Reset the whole ctx with memset().
> > - Add an unlinkely(err).
> > ---
> > security/landlock/tsync.c | 47 ++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 41 insertions(+), 6 deletions(-)
> >
> > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> > index 0d2b9c646030..42cc0ef0c704 100644
> > --- a/security/landlock/tsync.c
> > +++ b/security/landlock/tsync.c
> > @@ -203,6 +203,40 @@ static struct tsync_work *tsync_works_provide(struct tsync_works *s,
> > return ctx;
> > }
> >
> > +/**
> > + * tsync_works_trim - Put the last tsync_work element
> > + *
> > + * @s: TSYNC works to trim.
> > + *
> > + * Put the last task and decrement the size of @s.
> > + *
> > + * This helper does not cancel a running task, but just reset the last element
> > + * to zero.
> > + */
> > +static void tsync_works_trim(struct tsync_works *s)
> > +{
> > + struct tsync_work *ctx;
> > +
> > + if (WARN_ON_ONCE(s->size <= 0))
> > + return;
> > +
> > + ctx = s->works[s->size - 1];
> > +
> > + /*
> > + * For consistency, remove the task from ctx so that it does not look like
> > + * we handed it a task_work.
> > + */
> > + put_task_struct(ctx->task);
> > + memset(ctx, 0, sizeof(*ctx));
>
> Minor (and highly optional) remark, this is the same as
>
> *ctx = (struct tsync_work){};
What about:
*ctx = (typeof(*ctx)){};
>
> which I find slightly easier to read when resetting a struct value.
> Both is fine though.
>
> > +
> > + /*
> > + * Cancel the tsync_works_provide() change to recycle the reserved memory
> > + * for the next thread, if any. This also ensures that cancel_tsync_works()
> > + * and tsync_works_release() do not see any NULL task pointers.
> > + */
> > + s->size--;
> > +}
> > +
> > /*
> > * tsync_works_grow_by - preallocates space for n more contexts in s
> > *
> > @@ -276,7 +310,7 @@ static void tsync_works_release(struct tsync_works *s)
> > size_t i;
> >
> > for (i = 0; i < s->size; i++) {
> > - if (!s->works[i]->task)
> > + if (WARN_ON_ONCE(!s->works[i]->task))
> > continue;
> >
> > put_task_struct(s->works[i]->task);
> > @@ -379,16 +413,14 @@ static bool schedule_task_work(struct tsync_works *works,
> >
> > init_task_work(&ctx->work, restrict_one_thread_callback);
> > err = task_work_add(thread, &ctx->work, TWA_SIGNAL);
> > - if (err) {
> > + if (unlikely(err)) {
> > /*
> > * task_work_add() only fails if the task is about to exit. We
> > * checked that earlier, but it can happen as a race. Resume
> > * without setting an error, as the task is probably gone in the
> > - * next loop iteration. For consistency, remove the task from ctx
> > - * so that it does not look like we handed it a task_work.
> > + * next loop iteration.
> > */
> > - put_task_struct(ctx->task);
> > - ctx->task = NULL;
> > + tsync_works_trim(works);
> >
> > atomic_dec(&shared_ctx->num_preparing);
> > atomic_dec(&shared_ctx->num_unfinished);
> > @@ -412,6 +444,9 @@ static void cancel_tsync_works(struct tsync_works *works,
> > int i;
> >
> > for (i = 0; i < works->size; i++) {
> > + if (WARN_ON_ONCE(!works->works[i]->task))
> > + continue;
> > +
> > if (!task_work_cancel(works->works[i]->task,
> > &works->works[i]->work))
> > continue;
> > --
> > 2.53.0
> >
>
> Reviewed-by: Günther Noack <gnoack@google.com>
>
> Thanks for spotting and fixing this!
> —Günther
>
^ permalink raw reply
* Re: [PATCH v2 1/2] landlock: Fully release unused TSYNC work entries
From: Günther Noack @ 2026-02-17 16:35 UTC (permalink / raw)
To: Mickaël Salaün; +Cc: linux-security-module, Jann Horn
In-Reply-To: <20260217.cheoghae8Ahh@digikod.net>
On Tue, Feb 17, 2026 at 02:52:46PM +0100, Mickaël Salaün wrote:
> On Tue, Feb 17, 2026 at 01:41:11PM +0100, Günther Noack wrote:
> > On Tue, Feb 17, 2026 at 01:23:39PM +0100, Mickaël Salaün wrote:
> > > + memset(ctx, 0, sizeof(*ctx));
> >
> > Minor (and highly optional) remark, this is the same as
> >
> > *ctx = (struct tsync_work){};
>
> What about:
>
> *ctx = (typeof(*ctx)){};
I find that harder to read, because it is less commonly seen and the typeof() is
an indirection that makes me think as a reader. But at this point, this is only
a vague opinion and I don't feel strongly about it. Please submit either one of
these three options :)
—Günther
^ permalink raw reply
* Re: [PATCH RFC] security: add LSM blob and hooks for namespaces
From: Casey Schaufler @ 2026-02-17 17:29 UTC (permalink / raw)
To: Christian Brauner
Cc: Paul Moore, James Morris, linux-security-module, linux-kernel,
Casey Schaufler
In-Reply-To: <20260217-glasur-hinnimmt-ac72b3e67661@brauner>
On 2/17/2026 1:38 AM, Christian Brauner wrote:
> On Mon, Feb 16, 2026 at 09:34:57AM -0800, Casey Schaufler wrote:
>> On 2/16/2026 5:52 AM, Christian Brauner wrote:
>>> All namespace types now share the same ns_common infrastructure. Extend
>>> this to include a security blob so LSMs can start managing namespaces
>>> uniformly without having to add one-off hooks or security fields to
>>> every individual namespace type.
>> The implementation appears sound.
>>
>> I have to question whether having LSM controls on namespaces is reasonable.
> This is already in active use today but only in a very limited capacity.
> This generalizes it.
>
>> I suppose that you could have a system where (for example) SELinux runs
>> in permissive mode except within a specific user namespace, where it would
>> enforce policy. Do you have a use case in mind?
> We will use it in systemd services and containers to monitor and
> supervise namespaces.
While I am not among them, many people have objected strongly to making
containers an identified entity in the kernel. If these hooks were available
implementing a container scheme completely within the kernel would be
reasonably strait forward. I might consider tackling it myself.
I am also reminded of the kdbus effort of a decade ago:
https://www.linuxfoundation.org/blog/blog/kdbus-details
Are we ready for ksystemd? UNIX systems of the 1980's suffered greatly from
an excessive enthusiasm for pushing user space functionality (e.g. STREAMS)
into the kernel. Systemd is a fine scheme, but so was inittab, in a very
different way. Adding kernel facilities to support particular application
schemes is very tempting, but often leads to dead code and interfaces that
require maintenance long after the user space scheme has moved on.
^ permalink raw reply
* Re: [PATCH] xfrm: kill xfrm_dev_{state,policy}_flush_secctx_check()
From: Steffen Klassert @ 2026-02-18 9:22 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Paul Moore, SELinux, linux-security-module, Herbert Xu,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Network Development
In-Reply-To: <d20f1b63-714f-48ba-9bee-cd074205404f@I-love.SAKURA.ne.jp>
On Fri, Feb 13, 2026 at 10:59:15PM +0900, Tetsuo Handa wrote:
> On 2026/02/13 19:19, Steffen Klassert wrote:
> On 2026/02/13 19:19, Steffen Klassert wrote:
> >> The NETDEV_UNREGISTER path can be triggered by just doing "unshare -n ip addr show"
> >> (i.e. implicit cleanup of a network namespace due to termination of init process in
> >> that namespace). We are not allowed to reject the cleanup_net() route.
> >
> > And here we come to the other problem I mentioned. When a LSM policy
> > rejects to flush the xfrm states and policies on network namespace
> > exit, we leak all the xfrm states and policies in that namespace.
> > Here we have no other option, we must flush the xfrm states and
> > policies regardless of any LSM policy. This can be fixed with
> > something like that:
>
> This something is what I explained at
> https://lkml.kernel.org/r/1bb453af-3ef2-4ab6-a909-0705bd07c136@I-love.SAKURA.ne.jp .
> The "task_valid" argument does not always reflect whether LSM policy can reject or not.
That was to fix the memleak on network namespace exit.
The task_valid check should be ok for xfrm_policy_flush()
and xfrm_state_flush().
>
> Anyway, the patch to add xfrm_dev_unregister(dev) seems OK if we do like
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit?h=next-20260123&id=fc0f090e41e652d158f946c616cdd82baed3c8f4 ?
That would be OK as a first fix. Later we should
just unlink policies and states from the device,
as explained in my last mail.
^ permalink raw reply
* Re: [PATCH v5 2/9] landlock: Control pathname UNIX domain socket resolution by path
From: Mickaël Salaün @ 2026-02-18 9:37 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, Tingmao Wang, Justin Suess, Jann Horn,
linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Tahera Fahimi
In-Reply-To: <20260215105158.28132-3-gnoack3000@gmail.com>
On Sun, Feb 15, 2026 at 11:51:50AM +0100, Günther Noack wrote:
> * Add a new access right LANDLOCK_ACCESS_FS_RESOLVE_UNIX, which
> controls the look up operations for named UNIX domain sockets. The
> resolution happens during connect() and sendmsg() (depending on
> socket type).
> * Hook into the path lookup in unix_find_bsd() in af_unix.c, using a
> LSM hook. Make policy decisions based on the new access rights
> * Increment the Landlock ABI version.
> * Minor test adaptions to keep the tests working.
>
> With this access right, access is granted if either of the following
> conditions is met:
>
> * The target socket's filesystem path was allow-listed using a
> LANDLOCK_RULE_PATH_BENEATH rule, *or*:
> * The target socket was created in the same Landlock domain in which
> LANDLOCK_ACCESS_FS_RESOLVE_UNIX was restricted.
>
> In case of a denial, connect() and sendmsg() return EACCES, which is
> the same error as it is returned if the user does not have the write
> bit in the traditional Unix file system permissions of that file.
>
> This feature was created with substantial discussion and input from
> Justin Suess, Tingmao Wang and Mickaël Salaün.
>
> Cc: Tingmao Wang <m@maowtm.org>
> Cc: Justin Suess <utilityemal77@gmail.com>
> Cc: Mickaël Salaün <mic@digikod.net>
> Suggested-by: Jann Horn <jannh@google.com>
> Link: https://github.com/landlock-lsm/linux/issues/36
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> include/uapi/linux/landlock.h | 10 ++
> security/landlock/access.h | 11 +-
> security/landlock/audit.c | 1 +
> security/landlock/fs.c | 102 ++++++++++++++++++-
> security/landlock/limits.h | 2 +-
> security/landlock/syscalls.c | 2 +-
> tools/testing/selftests/landlock/base_test.c | 2 +-
> tools/testing/selftests/landlock/fs_test.c | 5 +-
> 8 files changed, 128 insertions(+), 7 deletions(-)
>
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index f88fa1f68b77..3a8fc3af0d64 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -248,6 +248,15 @@ struct landlock_net_port_attr {
> *
> * This access right is available since the fifth version of the Landlock
> * ABI.
> + * - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX: Look up pathname UNIX domain sockets
> + * (:manpage:`unix(7)`). On UNIX domain sockets, this restricts both calls to
> + * :manpage:`connect(2)` as well as calls to :manpage:`sendmsg(2)` with an
> + * explicit recipient address.
> + *
> + * This access right only applies to connections to UNIX server sockets which
> + * were created outside of the newly created Landlock domain (e.g. from within
> + * a parent domain or from an unrestricted process). Newly created UNIX
> + * servers within the same Landlock domain continue to be accessible.
It might help to add a reference to the explicit scope mechanism.
Please squash patch 9/9 into this one and also add a reference here to
the rationale described in security/landlock.rst
> *
> * Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used
> * with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as
> @@ -333,6 +342,7 @@ struct landlock_net_port_attr {
> #define LANDLOCK_ACCESS_FS_REFER (1ULL << 13)
> #define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14)
> #define LANDLOCK_ACCESS_FS_IOCTL_DEV (1ULL << 15)
> +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX (1ULL << 16)
> /* clang-format on */
>
> /**
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 42c95747d7bd..9a2991688835 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -34,7 +34,7 @@
> LANDLOCK_ACCESS_FS_IOCTL_DEV)
> /* clang-format on */
>
> -typedef u16 access_mask_t;
> +typedef u32 access_mask_t;
>
> /* Makes sure all filesystem access rights can be stored. */
> static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
> @@ -76,6 +76,15 @@ struct layer_access_masks {
> access_mask_t access[LANDLOCK_MAX_NUM_LAYERS];
> };
>
> +static inline bool
> +layer_access_masks_empty(const struct layer_access_masks *masks)
> +{
> + for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++)
> + if (masks->access[i])
> + return false;
> + return true;
> +}
> +
> /*
> * Tracks domains responsible of a denied access. This avoids storing in each
> * object the full matrix of per-layer unfulfilled access rights, which is
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index 60ff217ab95b..8d0edf94037d 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -37,6 +37,7 @@ static const char *const fs_access_strings[] = {
> [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer",
> [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate",
> [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev",
> + [BIT_INDEX(LANDLOCK_ACCESS_FS_RESOLVE_UNIX)] = "fs.resolve_unix",
> };
>
> static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index e764470f588c..76035c6f2bf1 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -27,6 +27,7 @@
> #include <linux/lsm_hooks.h>
> #include <linux/mount.h>
> #include <linux/namei.h>
> +#include <linux/net.h>
> #include <linux/path.h>
> #include <linux/pid.h>
> #include <linux/rcupdate.h>
> @@ -314,7 +315,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> LANDLOCK_ACCESS_FS_WRITE_FILE | \
> LANDLOCK_ACCESS_FS_READ_FILE | \
> LANDLOCK_ACCESS_FS_TRUNCATE | \
> - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
> /* clang-format on */
>
> /*
> @@ -1561,6 +1563,103 @@ static int hook_path_truncate(const struct path *const path)
> return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
> }
>
> +/**
> + * unmask_scoped_access - Remove access right bits in @masks in all layers
> + * where @client and @server have the same domain
> + *
> + * This does the same as domain_is_scoped(), but unmasks bits in @masks.
> + * It can not return early as domain_is_scoped() does.
> + *
> + * @client: Client domain
> + * @server: Server domain
> + * @masks: Layer access masks to unmask
> + * @access: Access bit that controls scoping
> + */
> +static void unmask_scoped_access(const struct landlock_ruleset *const client,
> + const struct landlock_ruleset *const server,
> + struct layer_access_masks *const masks,
> + const access_mask_t access)
This helper should be moved to task.c and factored out with
domain_is_scoped(). This should be a dedicated patch.
> +{
> + int client_layer, server_layer;
> + const struct landlock_hierarchy *client_walker, *server_walker;
> +
> + if (WARN_ON_ONCE(!client))
> + return; /* should not happen */
> +
> + if (!server)
> + return; /* server has no Landlock domain; nothing to clear */
> +
> + client_layer = client->num_layers - 1;
> + client_walker = client->hierarchy;
> + server_layer = server->num_layers - 1;
> + server_walker = server->hierarchy;
> +
> + /*
> + * Clears the access bits at all layers where the client domain is the
> + * same as the server domain. We start the walk at min(client_layer,
> + * server_layer). The layer bits until there can not be cleared because
> + * either the client or the server domain is missing.
> + */
> + for (; client_layer > server_layer; client_layer--)
> + client_walker = client_walker->parent;
> +
> + for (; server_layer > client_layer; server_layer--)
> + server_walker = server_walker->parent;
> +
> + for (; client_layer >= 0; client_layer--) {
> + if (masks->access[client_layer] & access &&
> + client_walker == server_walker)
> + masks->access[client_layer] &= ~access;
> +
> + client_walker = client_walker->parent;
> + server_walker = server_walker->parent;
> + }
> +}
> +
> +static int hook_unix_find(const struct path *const path, struct sock *other,
> + int flags)
> +{
> + const struct landlock_ruleset *dom_other;
> + const struct landlock_cred_security *subject;
> + struct layer_access_masks layer_masks;
> + struct landlock_request request = {};
> + static const struct access_masks fs_resolve_unix = {
> + .fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> + };
> +
> + /* Lookup for the purpose of saving coredumps is OK. */
> + if (unlikely(flags & SOCK_COREDUMP))
> + return 0;
> +
> + /* Access to the same (or a lower) domain is always allowed. */
> + subject = landlock_get_applicable_subject(current_cred(),
> + fs_resolve_unix, NULL);
> +
> + if (!subject)
> + return 0;
> +
> + if (!landlock_init_layer_masks(subject->domain, fs_resolve_unix.fs,
> + &layer_masks, LANDLOCK_KEY_INODE))
> + return 0;
> +
> + /* Checks the layers in which we are connecting within the same domain. */
> + dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
We need to call unix_state_lock(other) before reading it, and check for
SOCK_DEAD, and check sk_socket before dereferencing it. Indeed,
the socket can be make orphan (see unix_dgram_sendmsg and
unix_stream_connect). I *think* a socket cannot be "resurrected" or
recycled once dead, so we may assume there is no race condition wrt
dom_other, but please double check. This lockless call should be made
clear in the LSM hook. It's OK to not lock the socket before
security_unix_find() (1) because no LSM might implement and (2) they
might not need to lock the socket (e.g. if the caller is not sandboxed).
The updated code should look something like this:
unix_state_unlock(other);
if (unlikely(sock_flag(other, SOCK_DEAD) || !other->sk_socket)) {
unix_state_unlock(other);
return 0;
}
dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
unix_state_unlock(other);
> + unmask_scoped_access(subject->domain, dom_other, &layer_masks,
> + fs_resolve_unix.fs);
> +
> + if (layer_access_masks_empty(&layer_masks))
> + return 0;
> +
> + /* Checks the connections to allow-listed paths. */
> + if (is_access_to_paths_allowed(subject->domain, path,
> + fs_resolve_unix.fs, &layer_masks,
> + &request, NULL, 0, NULL, NULL, NULL))
> + return 0;
> +
> + landlock_log_denial(subject, &request);
> + return -EACCES;
> +}
> +
> /* File hooks */
>
> /**
> @@ -1838,6 +1937,7 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
> LSM_HOOK_INIT(path_unlink, hook_path_unlink),
> LSM_HOOK_INIT(path_rmdir, hook_path_rmdir),
> LSM_HOOK_INIT(path_truncate, hook_path_truncate),
> + LSM_HOOK_INIT(unix_find, hook_unix_find),
>
> LSM_HOOK_INIT(file_alloc_security, hook_file_alloc_security),
> LSM_HOOK_INIT(file_open, hook_file_open),
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index eb584f47288d..b454ad73b15e 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -19,7 +19,7 @@
> #define LANDLOCK_MAX_NUM_LAYERS 16
> #define LANDLOCK_MAX_NUM_RULES U32_MAX
>
> -#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_IOCTL_DEV
> +#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_RESOLVE_UNIX
> #define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
> #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
>
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 0d66a68677b7..933902d43241 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -164,7 +164,7 @@ static const struct file_operations ruleset_fops = {
> * If the change involves a fix that requires userspace awareness, also update
> * the errata documentation in Documentation/userspace-api/landlock.rst .
> */
> -const int landlock_abi_version = 8;
> +const int landlock_abi_version = 9;
>
> /**
> * sys_landlock_create_ruleset - Create a new ruleset
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 0fea236ef4bd..30d37234086c 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -76,7 +76,7 @@ TEST(abi_version)
> const struct landlock_ruleset_attr ruleset_attr = {
> .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
> };
> - ASSERT_EQ(8, landlock_create_ruleset(NULL, 0,
> + ASSERT_EQ(9, landlock_create_ruleset(NULL, 0,
> LANDLOCK_CREATE_RULESET_VERSION));
>
> ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 968a91c927a4..b318627e7561 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -575,9 +575,10 @@ TEST_F_FORK(layout1, inval)
> LANDLOCK_ACCESS_FS_WRITE_FILE | \
> LANDLOCK_ACCESS_FS_READ_FILE | \
> LANDLOCK_ACCESS_FS_TRUNCATE | \
> - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
>
> -#define ACCESS_LAST LANDLOCK_ACCESS_FS_IOCTL_DEV
> +#define ACCESS_LAST LANDLOCK_ACCESS_FS_RESOLVE_UNIX
>
> #define ACCESS_ALL ( \
> ACCESS_FILE | \
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH v5 8/9] landlock: Document FS access right for pathname UNIX sockets
From: Mickaël Salaün @ 2026-02-18 9:39 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, Justin Suess, linux-security-module, Tingmao Wang,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi
In-Reply-To: <20260215105158.28132-9-gnoack3000@gmail.com>
On Sun, Feb 15, 2026 at 11:51:56AM +0100, Günther Noack wrote:
> Cc: Justin Suess <utilityemal77@gmail.com>
> Cc: Mickaël Salaün <mic@digikod.net>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> Documentation/userspace-api/landlock.rst | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
> index 13134bccdd39..3ba73afcbc4b 100644
> --- a/Documentation/userspace-api/landlock.rst
> +++ b/Documentation/userspace-api/landlock.rst
> @@ -77,7 +77,8 @@ to be explicit about the denied-by-default access rights.
> LANDLOCK_ACCESS_FS_MAKE_SYM |
> LANDLOCK_ACCESS_FS_REFER |
> LANDLOCK_ACCESS_FS_TRUNCATE |
> - LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + LANDLOCK_ACCESS_FS_IOCTL_DEV |
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
> .handled_access_net =
> LANDLOCK_ACCESS_NET_BIND_TCP |
> LANDLOCK_ACCESS_NET_CONNECT_TCP,
> @@ -127,6 +128,12 @@ version, and only use the available subset of access rights:
> /* Removes LANDLOCK_SCOPE_* for ABI < 6 */
> ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
> LANDLOCK_SCOPE_SIGNAL);
> + __attribute__((fallthrough));
> + case 7:
> + __attribute__((fallthrough));
I don't think the fallthrough attribute is needed here. Same for the
sample.
> + case 8:
> + /* Removes LANDLOCK_ACCESS_FS_RESOLVE_UNIX for ABI < 8 */
ABI < 9
> + ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_RESOLVE_UNIX;
> }
>
> This enables the creation of an inclusive ruleset that will contain our rules.
> @@ -685,6 +692,13 @@ enforce Landlock rulesets across all threads of the calling process
> using the ``LANDLOCK_RESTRICT_SELF_TSYNC`` flag passed to
> sys_landlock_restrict_self().
>
> +Pathname UNIX sockets (ABI < 9)
> +-------------------------------
> +
> +Starting with the Landlock ABI version 9, it is possible to restrict
> +connections to pathname UNIX domain sockets (:manpage:`unix(7)`) using
> +the new ``LANDLOCK_ACCESS_FS_RESOLVE_UNIX`` right.
> +
> .. _kernel_support:
>
> Kernel support
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH v5 1/9] lsm: Add LSM hook security_unix_find
From: Mickaël Salaün @ 2026-02-18 9:36 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, Paul Moore, James Morris, Serge E . Hallyn,
Tingmao Wang, Justin Suess, linux-security-module,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Simon Horman, netdev, Alexander Viro,
Christian Brauner
In-Reply-To: <20260215105158.28132-2-gnoack3000@gmail.com>
On Sun, Feb 15, 2026 at 11:51:49AM +0100, Günther Noack wrote:
> From: Justin Suess <utilityemal77@gmail.com>
>
> Add a LSM hook security_unix_find.
>
> This hook is called to check the path of a named unix socket before a
> connection is initiated. The peer socket may be inspected as well.
>
> Why existing hooks are unsuitable:
>
> Existing socket hooks, security_unix_stream_connect(),
> security_unix_may_send(), and security_socket_connect() don't provide
> TOCTOU-free / namespace independent access to the paths of sockets.
>
> (1) We cannot resolve the path from the struct sockaddr in existing hooks.
> This requires another path lookup. A change in the path between the
> two lookups will cause a TOCTOU bug.
>
> (2) We cannot use the struct path from the listening socket, because it
> may be bound to a path in a different namespace than the caller,
> resulting in a path that cannot be referenced at policy creation time.
>
> Cc: Günther Noack <gnoack3000@gmail.com>
> Cc: Tingmao Wang <m@maowtm.org>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
> include/linux/lsm_hook_defs.h | 5 +++++
> include/linux/security.h | 11 +++++++++++
> net/unix/af_unix.c | 8 ++++++++
> security/security.c | 20 ++++++++++++++++++++
> 4 files changed, 44 insertions(+)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 8c42b4bde09c..7a0fd3dbfa29 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -317,6 +317,11 @@ LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
> LSM_HOOK(int, 0, watch_key, struct key *key)
> #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other,
> + int flags)
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_SECURITY_NETWORK
> LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
> struct sock *newsk)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f..99a33d8eb28d 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1931,6 +1931,17 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
> }
> #endif /* CONFIG_SECURITY_NETWORK */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +
> +int security_unix_find(const struct path *path, struct sock *other, int flags);
> +
> +#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_SECURITY_INFINIBAND
> int security_ib_pkey_access(void *sec, u64 subnet_prefix, u16 pkey);
> int security_ib_endport_manage_subnet(void *sec, const char *name, u8 port_num);
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index d0511225799b..369812b79dd8 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1230,6 +1230,14 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
> if (!sk)
> goto path_put;
>
> + /*
> + * We call the hook because we know that the inode is a socket and we
> + * hold a valid reference to it via the path.
> + */
> + err = security_unix_find(&path, sk, flags);
> + if (err)
> + goto sock_put;
> +
> err = -EPROTOTYPE;
> if (sk->sk_type == type)
I think this hook call should be moved here, just before the
touch_atime() call for consistency with the socket type check, and to
avoid doing useless check in the hook.
> touch_atime(&path);
> diff --git a/security/security.c b/security/security.c
> index 67af9228c4e9..c73196b8db4b 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4731,6 +4731,26 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
>
> #endif /* CONFIG_SECURITY_NETWORK */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +/**
> + * security_unix_find() - Check if a named AF_UNIX socket can connect
> + * @path: path of the socket being connected to
> + * @other: peer sock
> + * @flags: flags associated with the socket
> + *
> + * This hook is called to check permissions before connecting to a named
> + * AF_UNIX socket.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_unix_find(const struct path *path, struct sock *other, int flags)
> +{
> + return call_int_hook(unix_find, path, other, flags);
> +}
> +EXPORT_SYMBOL(security_unix_find);
> +
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_SECURITY_INFINIBAND
> /**
> * security_ib_pkey_access() - Check if access to an IB pkey is allowed
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH v5 3/9] samples/landlock: Add support for named UNIX domain socket restrictions
From: Mickaël Salaün @ 2026-02-18 9:37 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, Justin Suess, linux-security-module, Tingmao Wang,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi
In-Reply-To: <20260215105158.28132-4-gnoack3000@gmail.com>
On Sun, Feb 15, 2026 at 11:51:51AM +0100, Günther Noack wrote:
> The access right for UNIX domain socket lookups is grouped with the
> read-write rights in the sample tool. Rationale: In the general case,
> any operations are possible through a UNIX domain socket, including
> data-mutating operations.
>
> Cc: Justin Suess <utilityemal77@gmail.com>
> Cc: Mickaël Salaün <mic@digikod.net>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> samples/landlock/sandboxer.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index e7af02f98208..0bbbc5c9ead6 100644
> --- a/samples/landlock/sandboxer.c
> +++ b/samples/landlock/sandboxer.c
> @@ -111,7 +111,8 @@ static int parse_path(char *env_path, const char ***const path_list)
> LANDLOCK_ACCESS_FS_WRITE_FILE | \
> LANDLOCK_ACCESS_FS_READ_FILE | \
> LANDLOCK_ACCESS_FS_TRUNCATE | \
> - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
>
> /* clang-format on */
>
> @@ -295,11 +296,12 @@ static bool check_ruleset_scope(const char *const env_var,
> LANDLOCK_ACCESS_FS_MAKE_SYM | \
> LANDLOCK_ACCESS_FS_REFER | \
> LANDLOCK_ACCESS_FS_TRUNCATE | \
> - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
>
> /* clang-format on */
>
> -#define LANDLOCK_ABI_LAST 7
> +#define LANDLOCK_ABI_LAST 9
>
> #define XSTR(s) #s
> #define STR(s) XSTR(s)
> @@ -444,6 +446,13 @@ int main(const int argc, char *const argv[], char *const *const envp)
> "provided by ABI version %d (instead of %d).\n",
> LANDLOCK_ABI_LAST, abi);
> __attribute__((fallthrough));
> + case 7:
> + __attribute__((fallthrough));
The current code should print the hint when ABI <= 7. Please send a
dedicated patch to fix the TSYNC-related changes.
> + case 8:
> + /* Removes LANDLOCK_ACCESS_FS_RESOLVE_UNIX for ABI < 9 */
> + ruleset_attr.handled_access_fs &=
> + ~LANDLOCK_ACCESS_FS_RESOLVE_UNIX;
> + __attribute__((fallthrough));
> case LANDLOCK_ABI_LAST:
> break;
> default:
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH RFC] security: add LSM blob and hooks for namespaces
From: Dr. Greg @ 2026-02-18 11:15 UTC (permalink / raw)
To: Christian Brauner
Cc: Casey Schaufler, Paul Moore, James Morris, linux-security-module,
linux-kernel
In-Reply-To: <20260217-glasur-hinnimmt-ac72b3e67661@brauner>
On Tue, Feb 17, 2026 at 10:38:33AM +0100, Christian Brauner wrote:
Good morning, I hope the week is going well for everyone.
> On Mon, Feb 16, 2026 at 09:34:57AM -0800, Casey Schaufler wrote:
> > On 2/16/2026 5:52 AM, Christian Brauner wrote:
> > > All namespace types now share the same ns_common infrastructure. Extend
> > > this to include a security blob so LSMs can start managing namespaces
> > > uniformly without having to add one-off hooks or security fields to
> > > every individual namespace type.
> >
> > The implementation appears sound.
> >
> > I have to question whether having LSM controls on namespaces is reasonable.
> This is already in active use today but only in a very limited capacity.
> This generalizes it.
This seems to be a tacid indication of the need for namespace specific
LSM policies and/or controls, and further acknowledgement, that such
controls are in active use out in the wild.
More below on the implications of this.
> > I suppose that you could have a system where (for example) SELinux runs
> > in permissive mode except within a specific user namespace, where it would
> > enforce policy. Do you have a use case in mind?
> We will use it in systemd services and containers to monitor and
> supervise namespaces.
Christian, you are no doubt not familiar with our work, but over the
last six years our team has developed and have in production the most
sophisticated implementation of LSM namespacing that has been done.
With the caveat, of course, of implementations that have been made
public.
That work has been driven by what is the clear and apparent need to
have namespace specific and orthogonal security controls and policies,
something your patch and comments seems to clearly acknowledge. This
need is particularily important with respect to the advancements that
are needed for AI based security modeling and interdiction.
So our comments are driven by having done a bit of this before.
There has been some dialogue and debate as to whether and how LSM
namespacing should be implemented. The essential ingredient is the
need to have a task specific context of data, which can be inherited
by subordinate processes, that can be used to evaluate the LSM
security events/hooks that are executed by tasks having access to
that context of data.
Unless we misinterpret the implementation, your patch provides such
context for any process that wishes to unshare any namespace that it
is participating in.
This in turn implies that your patch is a fundamental step forward in
LSM namespacing. This isn't a criticism, just an observation.
The reason we can feel pretty strongly about this is that we initially
used the same strategy that you are using in a very early
implementation of TSEM. We abandoned that approach, since the
dynamics/politics of Linux kernel development, particularily in
security, tends to disfavor having to touch core kernel
infrastructure, so we implemented the equivalent of your approach
entirely in the context of our LSM.
To widen the scope of the impact of this, your patch also lays the
framework for implementing LSM specific security policy with kernel
modules. Again, not a criticism, just an observation, because we
implement the same capability with TSEM.
For those reading along at home. The reason that this is safe with a
classic namespace approach and not with previous 'loadable LSM'
strategies is that a process can verify that a policy module is loaded
and prepared to handle requests to interpret the events, before the
namespace installation/activation that would drive use of the module
actually takes effect.
Your approach is quite generic, which is positive. The open question
is whether or not the strategy is generic enough to handle LSM's that
may have very dynamic and varied requirements with respect to how to
configure the policy that will be implemented for the namespace.
Hopefully all of this will enable further discussions on this issue.
Best wishes for a productive remainder of the week.
As always,
Dr. Greg
The Quixote Project - Flailing at the Travails of Cybersecurity
https://github.com/Quixote-Project
^ permalink raw reply
* Re: [PATCH 0/2] fanotify: avid some premature LSM checks
From: Ondrej Mosnacek @ 2026-02-18 12:36 UTC (permalink / raw)
To: Jan Kara
Cc: Amir Goldstein, Matthew Bobrowski, linux-fsdevel,
linux-security-module, selinux, linux-kernel
In-Reply-To: <yk2qcux2ee7afr24xw6p7wp4t3islu64ttfsrheac2zwr6odnw@kmagnqbldb3f>
On Tue, Feb 17, 2026 at 12:09 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 16-02-26 16:06:23, Ondrej Mosnacek wrote:
> > Restructure some of the validity and security checks in
> > fs/notify/fanotify/fanotify_user.c to avoid generating LSM access
> > denials in the audit log where hey shouldn't be.
> >
> > Ondrej Mosnacek (2):
> > fanotify: avoid/silence premature LSM capability checks
> > fanotify: call fanotify_events_supported() before path_permission()
> > and security_path_notify()
> >
> > fs/notify/fanotify/fanotify_user.c | 50 ++++++++++++++----------------
> > 1 file changed, 23 insertions(+), 27 deletions(-)
>
> The series looks good to me as well. Thanks! I'll commit the series to my
> tree once the merge window closes and fixup the comment formatting on
> commit. No need to resend.
Great, thanks!
--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
^ 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