* Re: [PATCH 12/13] ovl: remove ovl_lock_rename_workdir()
From: Amir Goldstein @ 2026-02-05 9:45 UTC (permalink / raw)
To: NeilBrown
Cc: Christian Brauner, Alexander Viro, David Howells, Jan Kara,
Chuck Lever, Jeff Layton, Miklos Szeredi, John Johansen,
Paul Moore, James Morris, Serge E. Hallyn, Stephen Smalley,
linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-13-neilb@ownmail.net>
On Wed, Feb 4, 2026 at 6:09 AM NeilBrown <neilb@ownmail.net> wrote:
>
> From: NeilBrown <neil@brown.name>
>
> This function is unused.
>
I am confused.
What was this "fix" fixing an unused function:
e9c70084a64e5 ovl: fail ovl_lock_rename_workdir() if either target is unhashed
What am I missing?
Otherwise, feel free to add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Thanks,
Amir.
^ permalink raw reply
* Re: [PATCH 11/13] ovl: use is_subdir() for testing if one thing is a subdir of another
From: Amir Goldstein @ 2026-02-05 9:37 UTC (permalink / raw)
To: NeilBrown
Cc: Christian Brauner, Alexander Viro, David Howells, Jan Kara,
Chuck Lever, Jeff Layton, Miklos Szeredi, John Johansen,
Paul Moore, James Morris, Serge E. Hallyn, Stephen Smalley,
linux-kernel, netfs, linux-fsdevel, linux-nfs, linux-unionfs,
apparmor, linux-security-module, selinux
In-Reply-To: <20260204050726.177283-12-neilb@ownmail.net>
On Wed, Feb 4, 2026 at 6:09 AM NeilBrown <neilb@ownmail.net> wrote:
>
> From: NeilBrown <neil@brown.name>
>
> Rather than using lock_rename(), use the more obvious is_subdir() for
> ensuring that neither upper nor workdir contain the other.
> Also be explicit in the comment that the two directories cannot be the
> same.
>
> As this is a point-it-time sanity check and does not provide any
> on-going guarantees, the removal of locking does not introduce any
> interesting races.
>
> Signed-off-by: NeilBrown <neil@brown.name>
Looks reasonable
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
> fs/overlayfs/super.c | 15 +++++----------
> 1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index ba9146f22a2c..2fd3e0aee50e 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -451,18 +451,13 @@ static int ovl_lower_dir(const char *name, const struct path *path,
> return 0;
> }
>
> -/* Workdir should not be subdir of upperdir and vice versa */
> +/*
> + * Workdir should not be subdir of upperdir and vice versa, and
> + * they should not be the same.
> + */
> static bool ovl_workdir_ok(struct dentry *workdir, struct dentry *upperdir)
> {
> - bool ok = false;
> -
> - if (workdir != upperdir) {
> - struct dentry *trap = lock_rename(workdir, upperdir);
> - if (!IS_ERR(trap))
> - unlock_rename(workdir, upperdir);
> - ok = (trap == NULL);
> - }
> - return ok;
> + return !is_subdir(workdir, upperdir) && !is_subdir(upperdir, workdir);
> }
>
> static int ovl_setup_trap(struct super_block *sb, struct dentry *dir,
> --
> 2.50.0.107.gf914562f5916.dirty
>
^ permalink raw reply
* Re: [PATCH v3 2/6] landlock: Implement LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
From: Günther Noack @ 2026-02-05 8:17 UTC (permalink / raw)
To: Tingmao Wang
Cc: Mickaël Salaün, Günther Noack, Demi Marie Obenour,
Alyssa Ross, Jann Horn, Tahera Fahimi, Justin Suess,
linux-security-module
In-Reply-To: <c5b090acf2c16f120d340ec376ed3a538d535158.1770160146.git.m@maowtm.org>
On Tue, Feb 03, 2026 at 11:12:29PM +0000, Tingmao Wang wrote:
> Extend the existing abstract UNIX socket scoping to pathname sockets as
> well. Basically all of the logic is reused between the two types, just
> that pathname sockets scoping are controlled by another bit, and has its
> own audit request type (since the current one is named
> "abstract_unix_socket").
>
> Closes: https://github.com/landlock-lsm/linux/issues/51
> Signed-off-by: Tingmao Wang <m@maowtm.org>
A note in the context of this patch that occurred to me; While I was
discussing in my head which access bit should be introduced first [1].
I realized that if the LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET were to be
introduced first, this might be at odds with our other plans where we
do both the scope and path check in the same hook [2]. The scope
check may be difficult to move once we have started doing the check in
the other LSM hook, because there are other error conditions in
between the hooks. That only makes a difference when connect()/send()
is denied - if we do the scope check in the new introduced
unix_find_bsd hook in the future, then some erroring connect()/send()
operations may return Landlock's error code instead of another one in
some cases.
If it is not possible to move the check into the other hook, we might
then in the end have to hook into both LSM hooks and carry information
between them, once the other patch set lands. But the implementation
will be harder.
[1] https://lore.kernel.org/all/20260205.8531e4005118@gnoack.org/
[2] https://lore.kernel.org/all/aYMenaSmBkAsFowd@google.com/
–Günther
^ permalink raw reply
* Re: [PATCH v2 0/6] Landlock: Implement scope control for pathname Unix sockets
From: Günther Noack @ 2026-02-05 8:02 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Tingmao Wang, Justin Suess,
Demi Marie Obenour, Alyssa Ross, Jann Horn, Tahera Fahimi,
linux-security-module, Matthieu Buffet
In-Reply-To: <20260204.quaiyeiW9ipo@digikod.net>
On Wed, Feb 04, 2026 at 06:43:24PM +0100, Mickaël Salaün wrote:
> On Wed, Feb 04, 2026 at 12:44:29PM +0100, Günther Noack wrote:
> > On Tue, Feb 03, 2026 at 09:53:11PM +0000, Tingmao Wang wrote:
> > > On 2/3/26 17:54, Günther Noack wrote:
> > > >> Furthermore, an application / Landlock config etc can always opt to not
> > > >> use the scope bit at all, if it "knows" all the locations where the
> > > >> application's sockets would be placed, and just use RESOLVE_UNIX access
> > > >> right (or nothing if it is not supported).
> > > >>
> > > >> (The following is a bit of a side note, not terribly relevant if we're
> > > >> deciding to go with the patch as is.)
> > > >>
> > > >>>> [...]
> > > >>>> Another way to put it is that, if FS-based and scope-based controls
> > > >>>> interacts in the above proposed way, both mechanisms feel like "poking
> > > >>>> holes" in the other. But as Mickaël said, one can think of the two
> > > >>>> mechanisms not as independent controls, but rather as two interfaces for
> > > >>>> the same control. The socket access control is "enabled" if either the
> > > >>>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX access is handled, or the scope bit
> > > >>>> proposed in this patch is enabled.
> > > >>>>
> > > >>>> With that said, I can think of some alternative ways that might make this
> > > >>>> API look "better" (from a subjective point of view, feedback welcome),
> > > >>>> however it does mean more delays, and specifically, these will depend on
> > > >>>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX:
> > > >>>>
> > > >>>> One possibility is to simply always allow a Landlock domain to connect to
> > > >>>> its own sockets (in the case where LANDLOCK_ACCESS_FS_RESOLVE_UNIX is
> > > >>>> handled, otherwise all sockets are allowed). This might be reasonable, as
> > > >>>> one can only connect to a socket it creates if it has the permission to
> > > >>>> create it in the first place, which is already controlled by
> > > >>>> LANDLOCK_ACCESS_FS_MAKE_SOCK, so we don't really lose any policy
> > > >>>> flexibility here - if for some reason the sandboxer don't want to allow
> > > >>>> access to any (pathname) sockets, even the sandboxed app's own ones, it
> > > >>>> can just not allow LANDLOCK_ACCESS_FS_MAKE_SOCK anywhere.
> > > >>>
> > > >>> LANDLOCK_ACCESS_FS_MAKE_SOCK is only required to bind/listen to a
> > > >>> socket, not to connect. I guess you was thinking about
> > > >>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX in this case?
> > > >>
> > > >> In this "allow same-scope connect unconditionally" proposal, the
> > > >> application would still be able to (bind to and) connect to its own
> > > >> sockets, even if LANDLOCK_ACCESS_FS_RESOLVE_UNIX is handled and nothing is
> > > >> allowed to have LANDLOCK_ACCESS_FS_RESOLVE_UNIX access. But a sandboxer
> > > >> which for whatever reason doesn't want this "allow same scope" default can
> > > >> still prevent the use of (pathname) sockets by restricting
> > > >> LANDLOCK_ACCESS_FS_MAKE_SOCK, because if an app can't connect to any
> > > >> sockets it doesn't own, and can't create any sockets itself either, then
> > > >> it effectively can't connect to any sockets at all.
> > > >>
> > > >> (Although on second thought, I guess there could be a case where an app
> > > >> first creates some socket files before doing landlock_restrict_self(),
> > > >> then it might still be able to bind to these even without
> > > >> LANDLOCK_ACCESS_FS_MAKE_SOCK?)
> > > >
> > > > FWIW, I also really liked Tingmao's first of the two listed
> > > > possibilities in [1], where she proposed to introduce both rights
> > > > together. In my understanding, the arguments we have discussed so far
> > > > for that are:
> > > >
> > > > IN FAVOR:
> > > >
> > > > (pro1) Connecting to a UNIX socket in the same scope is always safe,
> > > > and it makes it possible to use named UNIX sockets between the
> > > > processes within a Landlock domains. (Mickaël convinced me in
> > > > discussion at FOSDEM that this is true.)
> > > >
> > > > If someone absolutely does not want that, they can restrict
> > > > LANDLOCK_ACCESS_FS_MAKE_SOCK and achieve the same effect (as
> > > > Tingmao said above).
> > > >
> > > > (pro2) The implementation of this is simpler.
> > > >
> > > > (I attempted to understand how the "or" semantics would be
> > > > implemented, and I found it non-trivial when you try to do it
> > > > for all layers at once. (Kernighan's Law applies, IMHO))
> > >
> > > I think the logic would basically be:
> > >
> > > 1. if any layers deny the access due to handled RESOLVE_UNIX but does not
> > > have the scope bit set, then we will deny rightaway, without calling
> > > domain_is_scoped().
> > >
> > > 2. Call domain_is_scoped() with a bitmask of "rules_covered" layers where
> > > there are RESOLVE_UNIX rules covering the socket being accessed, and
> > > essentially ignore those layers in the scope violation check.
> > >
> > > I definitely agree that it is tricky, but making same-scope access be
> > > allowed (i.e. the suggested idea above) would only get rid of step 1,
> > > which I think is the "simpler" bit. The extra logic in step 2 is still
> > > needed.
> > >
> > > I definitely agree with pro1 tho.
> >
> > Yes, you are describing the logic for the variant where
> > LANDLOCK_ACCESS_FS_RESOLVE_UNIX does not implicitly permit access from
> > within the same scope. In that variant, there can be situations where
> > the first hook can deny the action immediately.
> >
> > In the variant where LANDLOCK_ACCESS_FS_RESOLVE_UNIX *does* implicitly
> > allow access from within the same scope, that shortcutting is not
> > possible. On the upside however, there is no need to distinguish
> > whether the scope flag is set when we are in the security_unix_find()
> > hook, because access from within the same scope is always permitted.
> > (That is the simplification I meant.)
>
> This proposal make sense, improve IPC restriction consistency (by always
> be able to connect to peers from the same domain), and would simplify
> some checks.
> What bothers me the most is the implicit scoping induced
> by the FS_RESOLVE_UNIX handling.
I see this as similar to other decisions we have taken, e.g. on the
(in-review) socket type patch, we have also discussed at the time that
socketpair() should always be allowed, even though it creates sockets,
because this syscall only establishes IPC communication channels
within the same Landlock domain.
Admittedly, in this case it is a bit more clearly about the Landlock
domain boundaries, but still OK in my mind. I think we are aligned
though, see my answer to your proposal below.
> A related issue is the mix of checks
> for the FS_RESOLVE_UNIX implementation, which could make the code more
> complex, but I don't see a better way. Landlock already has an implicit
> ptrace scoping but it's not (and should never be) optional.
I am afraid that we don't have a better option. Even if we only
introduced one of the two patch sets in its original form now, we
would have to introduce that implementation as soon as the other is
added.
> See my proposal below about implicit
> LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET. That would not change the kernel
> checks you described though.
> > > > AGAINST:
> > > >
> > > > (con1) It would work differently than the other scoped access rights
> > > > that we already have.
> > > >
> > > > A speculative feature that could potentially be built with the
> > > > scoped access rights is that we could add a rule to permit IPC
> > > > to other Landlock scopes, e.g. introducing a new rule type
> > > >
> > > > struct landlock_scope_attr {
> > > > __u64 allowed_access; /* for "scoped" bits */
> > > > /* some way to identify domains */
> > > > }
> > > >
> > > > so that we could make IPC access to other Landlock domains
> > > > configurable.
> > > >
> > > > If the scoped bit and the FS RESOLVE_UNIX bit were both
> > > > conflated in RESOLVE_UNIX, it would not be possible to make
> > > > UNIX connections configurable in such a way.
> > >
> > > This exact API would no longer work, but if we give up the equivalence
> > > between scope bits and the landlock_scope_attr struct, then we can do
> > > something like:
> > >
> > > struct landlock_scope_attr {
> > > __u64 ptrace:1; /* Note that this is not a (user controllable) scope bit! */
> > > __u64 abstract_unix_socket:1;
> > > __u64 pathname_unix_socket:1;
> > > /* ... */
> > >
> > > __u64 allowed_signals;
> > >
> > > /*
> > > * some way to identify domains, maybe we could use the audit domain
> > > * ID, with 0 denoting "allow access to non-Landlocked processes?
> > > */
> > > }
> >
> > Yes, it would be possible to use such a struct for that scenario where
> > IPC access gets allowed for other Landlock scopes. It would mean that
> > we would not need to introduce a scoped flag for the pathname UNIX
> > socket connections. But the relationship between that struct
> > landlock_scope_attr and the flags and access rights in struct
> > landlock_ruleset_attr would become less clear, which is a slight
> > downside, and maybe error prone for users to work with.
> >
> > If we introduced an additional scoped flag, it would also be
> > consistent though.
> >
> > (con1) was written under the assumption that we do not have an
> > additional scoped flag. If that is lacking, it is not possible to
> > express UNIX connect() access to other Landlock domains with that
> > struct. But as outlined in the proposal below, if we *do* (later)
> > introduce the additional scoped flag *in addition* to the FS access
> > right, this *both* stays consistent in semantics with the signal and
> > abstract UNIX support, *and* it starts working in a world where ICP
> > access can be allowed to talk to other Landlock domains.
>
> In this case, we need to assume the scoped flag is already there, and
> because we have the implementation, to actually add it now.
>
> As a side note, I don't really like this landlock_scope_attr interface.
> A dedicated attr per kind of IPC would be much more flexible (e.g. the
> handled_signal_number you described below). For instance, even if at
> first we don't make it possible to describe different signals within the
> attr struct, this struct will be able to grow. So it's better to have a
> dedicated interface per IPC type than a generic scope interface that can
> only express a subset of it.
>
> Anyway, this proposal could work for pathname UNIX sockets, but a future
> signal attr would be a bit inconsistent wrt to the pathname UNIX one.
See my answer to your proposal below.
> > > > (con2) Consistent behaviour between scoped flags and their
> > > > interactions with other access rights:
> > > >
> > > > The existing scoped access rights (signal, abstract sockets)
> > > > could hypothetically be extended with a related access right of
> > > > another type. For instance, there could be an access right type
> > > >
> > > > __u64 handled_signal_number;
> > > >
> > > > and then you could add a rule to permit the use of certain
> > > > signal numbers. The interaction between the scoped flags and
> > > > other access rights should work the same.
> > > >
> > > >
> > > > Constructive Proposal for consideration: Why not both?
> > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > I will think about the following a bit more but I'm afraid that I feel
> > > like it might get slightly confusing. With this, the only reason for
> > > having LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET is for API consistency when we
> > > later enable allowing access to other domains (if I understood correctly),
> > > in which case I personally feel like the suggestion on landlock_scope_attr
> > > above, where we essentially accept that it is decoupled with the scope
> > > bits in the ruleset, might be simpler...?
> >
> > Mickaël expressed the opinion to me that he would like to APIs to stay
> > consistent between signals, abstract UNIX sockets, named UNIX sockets
> > and other future "scoped" operations, in scenarios where:
> >
> > * the "scoped" (IPC) operations can be configured to give access to
> > other Landlock domains (and that should work for UNIX connections too)
> > * the existing "scoped" operations also start having matching access rights
> >
> > I think with the way I proposed, that would be consistent.
>
> What about always implicitly set LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET (in
> the kernel) when LANDLOCK_ACCESS_FS_RESOLVE_UNIX is set?
>
> *Requiring* users to set LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET would add a
> new way to trigger an error, but setting
> LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET *implicitly* looks safer. This
> might also help for the documentation BTW.
>
> If we agree on this, I could merge the scope_pathname_unix in 7.0, and
> merge the resolve_unix in 7.1 (or later if something unexpected happen).
> Otherwise, I'll merge both at the same time with the same ABI version
> (with the risk to postpone again). WDYT?
In my understanding, the only thing in which our two proposals differ
is the order in which we introduce the
LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET and
LANDLOCK_ACCESS_FS_RESOLVE_UNIX bits to the UAPI.
As soon as both bits are added, our two proposals do the same thing.
My proposal (introduce LANDLOCK_ACCESS_FS_RESOLVE_UNIX first)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The reason why I have been arguing to introduce
LANDLOCK_ACCESS_FS_RESOLVE_UNIX first is:
(a) It is possible (which initially wasn't obvious to me):
When implementing this, handling LANDLOCK_ACCESS_FS_RESOLVE_UNIX
implies the LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET semantics(!), even
when that bit is not exposed in the UAPI yet. (When
LANDLOCK_ACCESS_FS_RESOLVE_UNIX is handled, UNIX-connect always
works within the same Landlock domain, even for the time when the
scoped flag is not exposed in the UAPI yet.)
That way, everything stays compatible even when the scoped flag is
later introduced.
(b) When introducing LANDLOCK_ACCESS_FS_RESOLVE_UNIX first, the UAPI
will initially only expose one of the two bits.
This simplifies the UAPI because users initially do not need to
deal with the fact that two bits in the UAPI "interact".
We can still introduce LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET later.
But the only ruleset-building feature for which this needed would
be if we want to have rules that allow outwards IPC interactions
with other Landlock domains.
Possibility of YAGNI: I am not sure how certain you are that the
speculative allowing-scope-access feature is needed; I mentioned
it here only because it is an option we want to keep open. If we
end up *not* needing this feature though, we will *not need to
introduce the LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET bit at all*,
because LANDLOCK_ACCESS_FS_RESOLVE_UNIX can already do all that is
needed.
The implementation of this approach would be that we would have to
join the functionality from the scoped and FS-based patch set, but
without introducing the LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET flag in
the UAPI.
Your proposal (introduce LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET first)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
* Upside: users won't be surprised if UNIX-connect within
the same Landlock domain works, because the bit is there for it
* Downside: They'll have to understand that the FS bit implies the
scope flag.
But as I said in the beginning -- if we add both flags, these two
proposals end up being the same in the end.
Maybe the main point that is unclear to me is what I called the
"Possibility of YAGNI" above: it is at this point not 100% clear to me
that the feature for allowing IPC to other domains is going to happen
(we don't have a bug tracker issue for it either, AFAICT). So in the
case where this does not actually end up happening, I think that my
approach has the advantage of introducing one flag less, so the UAPI
stays simpler.
If you know that that feature will happen though, I think the two
approaches are equally good in the long run.
But either way, Mickaël, you are the maintainer here :); if you prefer
to do the LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET patch set in its current
form first, it's also OK with me. As I said, I think these two
approaches are the same.
P.S. Tingmao: One additional complication, in case the
LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET patch set gets sent first -- your
current patch set uses a different LSM hook than the final state which
we are aiming for. We may need to double check that this will stay
compatible even if we move the scoped-check into the new LSM hook in
the future. (If we move the check into a different hook, there may be
some obscure scenarios in which a denied connect()/send() results in a
different error code.) I may comment on the patch set.
> > > > Why not do both what Tingmao proposed in [1] **and** reserve the
> > > > option to add the matching "scoped flag" later?
> > > >
> > > > * Introduce LANDLOCK_ACCESS_FS_RESOLVE_UNIX.
> > > >
> > > > If it is handled, UNIX connections are allowed either:
> > > >
> > > > (1) if the connection is to a service in the same scope, or
> > > > (2) if the path was allow-listed with a "path beneath" rule.
> > > >
> > > > * Add LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET later, if needed.
> > > >
> > > >
> > > > Let's go through the arguments again:
> > > >
> > > > We have observed that it is harmless to allow connections to services
> > > > in the same scope (1), and that if users absolutely don't want that,
> > > > they can actually prohibit it through LANDLOCK_ACCESS_FS_MAKE_SOCK
> > > > (pro1).
> > > >
> > > > (con1): Can we still implement the feature idea where we poke a hole
> > > > to get UNIX-connect() access to other Landlock domains?
> > > >
> > > > I think the answer is yes. The implementation strategy is:
> > > >
> > > > * Add the scoped bit LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
> > > > * The scoped bit can now be used to allow-list connections to
> > > > other Landlock domains.
> > > >
> > > > For users, just setting the scoped bit on its own does the same as
> > > > handling LANDLOCK_ACCESS_FS_RESOLVE_UNIX. That way, the kernel-side
> > > > implementation can also stay simple. The only reason why the scoped
> > > > bit is needed is because it makes it possible to allow-list
> > > > connections to other Landlock domains, but at the same time, it is
> > > > safe if libraries set the scoped bit once it exists, as it does not
> > > > have any bad runtime impact either.
> > > >
> > > > (con2): Consistency: Do all the scoped flags interact with their
> > > > corresponding access rights in the same way?
> > > >
> > > > The other scope flags do not have corresponding access rights, so
> > > > far.
> > > >
> > > > If we were to add corresponding access rights for the other scope
> > > > flags, I would argue that we could apply a consistent logic there,
> > > > because IPC access within the same scope is always safe:
> > > >
> > > > - A hypothetical access right type for "signal numbers" would only
> > > > restrict signals that go beyond the current scope.
> > > >
> > > > - A hypothetical access right type for "abstract UNIX domain socket
> > > > names" would only restrict connections to abstract UNIX domain
> > > > servers that go beyond the current scope.
> > > >
> > > > I can not come up with a scenario where this doesn't work.
> > > >
> > > >
> > > > In conclusion, I think the approach has significant upsides:
> > > >
> > > > * Simpler UAPI: Users only have one access bit to deal with, in the
> > > > near future. Once we do add a scope flag for UNIX connections, it
> > > > does not interact in a surprising way with the corresponding FS
> > > > access right, because with either of these, scoped access is
> > > > allowed.
> > > >
> > > > If users absolutely need to restrict scoped access, they can
> > > > restrict LANDLOCK_ACCESS_FS_MAKE_SOCK. It is a slightly obscure
> > > > API, but in line with the "make easy things easy, make hard things
> > > > possible" API philosophy. And needing this should be the
> > > > exception rather than the norm, after all.
> > > >
> > > > * Consistent behaviour between scoped flags and regular access
> > > > rights, also for speculative access rights affecting the existing
> > > > scoped flags for signals and abstract UNIX domain sockets.
> > > >
> > > > [1] https://lore.kernel.org/all/f07fe41a-96c5-4d3a-9966-35b30b3a71f1@maowtm.org/
>
> Thanks for this summary Günther!
^ permalink raw reply
* Re: [PATCH] apparmor: replace strcpy with strscpy
From: John Johansen @ 2026-02-05 7:52 UTC (permalink / raw)
To: Ryan Foster
Cc: paul, jmorris, serge, apparmor, linux-security-module,
linux-kernel
In-Reply-To: <20260120145024.261806-1-foster.ryan.r@gmail.com>
On 1/20/26 06:50, Ryan Foster wrote:
> Found by checkpatch. Replace strcpy() with strscpy() for safer
> string handling per KSPP recommendations.
>
> Two changes:
> - apparmorfs.c: gen_symlink_name() uses tracked buffer size
> - lib.c: aa_policy_init() uses exact allocation size
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Ryan Foster <foster.ryan.r@gmail.com>
this collides with two different patches I previously pulled in
1c90ed1f14c98 apparmor: Replace deprecated strcpy with memcpy in gen_symlink_name
b31d3f7385fbb apparmor: Replace sprintf/strcpy with scnprintf/strscpy in aa_policy_init
if you want to refresh this, I will look at pulling it in
> ---
> security/apparmor/apparmorfs.c | 2 +-
> security/apparmor/lib.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 907bd2667e28..f38974231df2 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -1614,7 +1614,7 @@ static char *gen_symlink_name(int depth, const char *dirname, const char *fname)
> return ERR_PTR(-ENOMEM);
>
> for (; depth > 0; depth--) {
> - strcpy(s, "../../");
> + strscpy(s, "../../", size);
> s += 6;
> size -= 6;
> }
> diff --git a/security/apparmor/lib.c b/security/apparmor/lib.c
> index 82dbb97ad406..7cb393f91a10 100644
> --- a/security/apparmor/lib.c
> +++ b/security/apparmor/lib.c
> @@ -487,7 +487,7 @@ bool aa_policy_init(struct aa_policy *policy, const char *prefix,
> } else {
> hname = aa_str_alloc(strlen(name) + 1, gfp);
> if (hname)
> - strcpy(hname, name);
> + strscpy(hname, name, strlen(name) + 1);
> }
> if (!hname)
> return false;
^ permalink raw reply
* Re: [PATCH] apparmor: add .kunitconfig
From: John Johansen @ 2026-02-05 7:41 UTC (permalink / raw)
To: Ryota Sakamoto, Paul Moore, James Morris, Serge E. Hallyn
Cc: linux-kernel, apparmor, linux-security-module
In-Reply-To: <20260125-add-apparmor-kunitconfig-v1-1-e815cec415df@gmail.com>
On 1/25/26 02:05, Ryota Sakamoto wrote:
> Add .kunitconfig file to the AppArmor directory to enable easy execution of
> KUnit tests.
>
> AppArmor tests (CONFIG_SECURITY_APPARMOR_KUNIT_TEST) depend on
> CONFIG_SECURITY_APPARMOR which also depends on CONFIG_SECURITY and
> CONFIG_NET. Without explicitly enabling these configs in the .kunitconfig,
> developers will need to specify config manually.
>
> With the .kunitconfig, developers can run the tests:
> $ ./tools/testing/kunit/kunit.py run --kunitconfig security/apparmor
>
> Signed-off-by: Ryota Sakamoto <sakamo.ryota@gmail.com>
Acked-by: John Johansen <john.johansen@canonical.com>
sorry, for the delay I actually pulled this in a while ago, but the initial reply seems
to have gone into the void.
> ---
> security/apparmor/.kunitconfig | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/security/apparmor/.kunitconfig b/security/apparmor/.kunitconfig
> new file mode 100644
> index 0000000000000000000000000000000000000000..aa842a0266e9d33c3333ec2ea180206187b0eb4c
> --- /dev/null
> +++ b/security/apparmor/.kunitconfig
> @@ -0,0 +1,5 @@
> +CONFIG_KUNIT=y
> +CONFIG_NET=y
> +CONFIG_SECURITY=y
> +CONFIG_SECURITY_APPARMOR=y
> +CONFIG_SECURITY_APPARMOR_KUNIT_TEST=y
>
> ---
> base-commit: d91a46d6805af41e7f2286e0fc22d498f45a682b
> change-id: 20260125-add-apparmor-kunitconfig-28aba43c1580
>
> Best regards,
^ permalink raw reply
* Re: [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs
From: Mimi Zohar @ 2026-02-05 4:26 UTC (permalink / raw)
To: Frederick Lawler, Roberto Sassu
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, Darrick J. Wong, Christian Brauner,
Josef Bacik, Jeff Layton, linux-kernel, linux-integrity,
linux-security-module, kernel-team
In-Reply-To: <aYO4fj0Uw0aUWXOX@CMGLRV3>
On Wed, 2026-02-04 at 15:22 -0600, Frederick Lawler wrote:
> That said, I think Mimi pointed out in an email [2] where multi-grain
> file systems are impacted regardless of stacked fs or not due to the last
> writer check.
>
> I don't recall coming across that in my tests, but perhaps I did that
> specific test wrong? To be sure, I created the C program, and on the VM,
> created a XFS disk, mounted it on loopback, ran the rdwr program on
> "somefile" multiple times, and only got 1 audit log for it, until I
> mutated it with touch, and only got 2 hits: original + after mutation
> after running the program multiple times.
>
> I'm not sure what's going on there, so I'll look into that a bit more,
> but so far the impact is stacked file systems & multigrain ctime AFAIK.
Make sure you're testing without your patch set applied or at least the last
patch.
Mimi
^ permalink raw reply
* Re: [PATCH net-next v2 0/4] net: uapi: Provide an UAPI definition of 'struct sockaddr'
From: Jakub Kicinski @ 2026-02-05 1:55 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Eric Dumazet, Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn,
David S. Miller, Simon Horman, Shuah Khan, Matthieu Baerts,
Mat Martineau, Geliang Tang, Mickaël Salaün,
Günther Noack, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, Stanislav Fomichev,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Hao Luo, Jiri Olsa, netdev, linux-kernel,
linux-api, Arnd Bergmann, linux-kselftest, mptcp,
linux-security-module, bpf, libc-alpha, Carlos O'Donell,
Adhemerval Zanella, Rich Felker, klibc, Florian Weimer
In-Reply-To: <20260204064248-d9c4ab78-f6d4-4ac6-8d55-e939bc1df6d2@linutronix.de>
On Wed, 4 Feb 2026 06:51:46 +0100 Thomas Weißschuh wrote:
> > make -j16 O="$kobj" INSTALL_HDR_PATH="${kobj}/hdr" headers_install
> > popd
> >
> > pushd uapi
> > find . -type f -name '*.h' -exec cp -v "${kobj}/hdr/include/{}" {} \;
>
> Here only those headers which already exist in ethtool's uapi/ directory
> are copied. As linux/typelimits.h is new, it is now missing.
> Honestly, if a user fiddles with the internals of the UAPI headers like
> this, it is on them to update their code if the internal structure
> changes. In your case a simple 'touch uapi/linux/typelimits.h'
> before running the script will be enough. Also internal.h now requires
> an explicit inclusion of <limits.h>, as that is not satisfied by the
> UAPI anymore.
Hopefully you understand that while due to uapi header copy this is not
a huge issue for ethtool itself, but it is a proof that your changes
can break normal user space applications which do not vendor in uapi.
^ permalink raw reply
* [PATCH] apparmor: Remove redundant if check in sk_peer_get_label
From: Thorsten Blum @ 2026-02-04 22:07 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn
Cc: Thorsten Blum, apparmor, linux-security-module, linux-kernel
Remove the redundant if check in sk_peer_get_label() and return
ERR_PTR(-ENOPROTOOPT) directly.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
security/apparmor/lsm.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index a87cd60ed206..54343f7c96a4 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1536,15 +1536,11 @@ static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
static struct aa_label *sk_peer_get_label(struct sock *sk)
{
struct aa_sk_ctx *ctx = aa_sock(sk);
- struct aa_label *label = ERR_PTR(-ENOPROTOOPT);
if (rcu_access_pointer(ctx->peer))
return aa_get_label_rcu(&ctx->peer);
- if (sk->sk_family != PF_UNIX)
- return ERR_PTR(-ENOPROTOOPT);
-
- return label;
+ return ERR_PTR(-ENOPROTOOPT);
}
/**
--
Thorsten Blum <thorsten.blum@linux.dev>
GPG: 1D60 735E 8AEF 3BE4 73B6 9D84 7336 78FD 8DFE EAD4
^ permalink raw reply related
* [PATCH] AppArmor: Add missing SPDX id line to sig_names.h
From: Tim Bird @ 2026-02-04 21:58 UTC (permalink / raw)
To: john.johansen, paul, jmorris, serge
Cc: linux-spdx, apparmor, linux-security-module, linux-kernel,
Tim Bird
sig_names.h is missing an SPDX-License-Identifier line.
Add one with the ID of GPL-2.0-only. This matches all
the other files in AppArmor.
Signed-off-by: Tim Bird <tim.bird@sony.com>
---
security/apparmor/include/sig_names.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/security/apparmor/include/sig_names.h b/security/apparmor/include/sig_names.h
index c772668cdc62..3f617c1a64cd 100644
--- a/security/apparmor/include/sig_names.h
+++ b/security/apparmor/include/sig_names.h
@@ -1,3 +1,4 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
#include <linux/signal.h>
#include "signal.h"
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs
From: Frederick Lawler @ 2026-02-04 21:22 UTC (permalink / raw)
To: Roberto Sassu
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton, linux-kernel,
linux-integrity, linux-security-module, kernel-team
In-Reply-To: <c449523aef301a6b199e06d4c3fbf7587d1218c5.camel@huaweicloud.com>
On Wed, Feb 04, 2026 at 01:36:09PM +0100, Roberto Sassu wrote:
> On Fri, 2026-01-30 at 16:39 -0600, Frederick Lawler wrote:
> > IMA performs unnecessary measurements on files in stacked file systems
> > that do not set kstat.change_cookie to an inode's i_version.
> >
> > For example: TMPFS (upper) is stacked onto XFS (lower).
> > Actions files result in re-measurement because commit 1cf7e834a6fb
> > ("xfs: switch to multigrain timestamps") introduced multigrain
> > timestamps into XFS which changed the kstat.change_cookie semantics
> > to no longer supply an i_version to compare against in
> > integrity_inode_attributes_changed(). Once the inode is in TMPFS,
> > the change detection behavior operates as normal because TMPFS updates
> > kstat.change_cookie to the i_version.
> >
> > Instead, fall back onto a ctime comparison. This also gives file systems
> > that do not support i_version an opportunity avoid the same behavior,
> > as they're more likely to have ctime support.
> >
> > timespec64_to_ns() is chosen to avoid adding extra storage to
> > integrity_inode_attributes by leveraging the existing version field.
>
> Correct me if I'm wrong, but this issue seems to me xfs-specific. In
> that case, maybe it is better to remove the stacked filesystem part,
> which might be misleading.
I'm using XFS because that's a clear example, but as Jeff pointed out in
an earlier email, there's too many file systems to account for. It's
clear from Jeff's prior email[1] that the intention is to stop using
change cookie for change detection for more file systems in favor
of multigrain ctime.
The stacked part is important because AFAIK, if not on stacked file system
the check in process_measurement() line 329 is skipped because
of the last_writer check doing the atomic read on the write count.
That said, I think Mimi pointed out in an email [2] where multi-grain
file systems are impacted regardless of stacked fs or not due to the last
writer check.
I don't recall coming across that in my tests, but perhaps I did that
specific test wrong? To be sure, I created the C program, and on the VM,
created a XFS disk, mounted it on loopback, ran the rdwr program on
"somefile" multiple times, and only got 1 audit log for it, until I
mutated it with touch, and only got 2 hits: original + after mutation
after running the program multiple times.
I'm not sure what's going on there, so I'll look into that a bit more,
but so far the impact is stacked file systems & multigrain ctime AFAIK.
[1]: https://lore.kernel.org/all/75618d9f454aece9a991d74eaf7ae5160828e901.camel@kernel.org/
[2]: https://lore.kernel.org/all/69540ac3aca536db948b6585b7076946a12ebe36.camel@linux.ibm.com/
>
> Thanks
>
> Roberto
>
> > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > ---
> > include/linux/integrity.h | 6 +++++-
> > security/integrity/ima/ima_api.c | 11 ++++++++---
> > security/integrity/ima/ima_main.c | 2 +-
> > 3 files changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > index 382c783f0fa3ae4a938cdf9559291ba1903a378e..ec2c94907f417c4a71ecce29ac79edac9bc2c6f8 100644
> > --- a/include/linux/integrity.h
> > +++ b/include/linux/integrity.h
> > @@ -10,6 +10,7 @@
> > #include <linux/fs.h>
> > #include <linux/iversion.h>
> > #include <linux/kernel.h>
> > +#include <linux/time64.h>
> >
> > enum integrity_status {
> > INTEGRITY_PASS = 0,
> > @@ -58,6 +59,9 @@ integrity_inode_attrs_stat_changed
> > if (stat->result_mask & STATX_CHANGE_COOKIE)
> > return stat->change_cookie != attrs->version;
> >
> > + if (stat->result_mask & STATX_CTIME)
> > + return timespec64_to_ns(&stat->ctime) != (s64)attrs->version;
> > +
> > return true;
> > }
> >
> > @@ -84,7 +88,7 @@ integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > * only for IMA if vfs_getattr_nosec() fails.
> > */
> > if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> > - STATX_CHANGE_COOKIE,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT))
> > return !IS_I_VERSION(inode) ||
> > !inode_eq_iversion(inode, attrs->version);
> > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > index c35ea613c9f8d404ba4886e3b736c3bab29d1668..e47d6281febc15a0ac1bd2ea1d28fea4d0cd5c58 100644
> > --- a/security/integrity/ima/ima_api.c
> > +++ b/security/integrity/ima/ima_api.c
> > @@ -272,10 +272,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > * to an initial measurement/appraisal/audit, but was modified to
> > * assume the file changed.
> > */
> > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > + result = vfs_getattr_nosec(&file->f_path, &stat,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT);
> > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > - i_version = stat.change_cookie;
> > + if (!result) {
> > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > + i_version = stat.change_cookie;
> > + else if (stat.result_mask & STATX_CTIME)
> > + i_version = timespec64_to_ns(&stat.ctime);
> > + }
> > hash.hdr.algo = algo;
> > hash.hdr.length = hash_digest_size[algo];
> >
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 8cb17c9d446caaa5a98f5ec8f027c17ba7babca8..776db158b0bd8a0d053729ac0cc15af8b6020a98 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -199,7 +199,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > &iint->atomic_flags);
> > if ((iint->flags & IMA_NEW_FILE) ||
> > vfs_getattr_nosec(&file->f_path, &stat,
> > - STATX_CHANGE_COOKIE,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT) ||
> > integrity_inode_attrs_stat_changed(&iint->real_inode,
> > &stat)) {
> >
>
^ permalink raw reply
* Re: [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs
From: Frederick Lawler @ 2026-02-04 19:35 UTC (permalink / raw)
To: Roberto Sassu
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton, linux-kernel,
linux-integrity, linux-security-module, kernel-team
In-Reply-To: <9c25c201f384d7320fd83e648d61a0086016ee36.camel@huaweicloud.com>
On Wed, Feb 04, 2026 at 01:34:09PM +0100, Roberto Sassu wrote:
> On Fri, 2026-01-30 at 16:39 -0600, Frederick Lawler wrote:
> > Align integrity_inode_attrs_changed() to ima_check_last_writer()'s
> > semantics when detecting changes.
> >
> > For IMA, stacked file systems that do not set kstat.change_cookie,
> > integrity_inode_attrs_changed() will compare zero to zero, thus no
>
> I setup overlay with two xfs filesystems, kept the file I want to be
> audited in the lower filesystem.
>
> Without this patch set, if I modify the lower file, changes are
> detected, because actually the i_version is incremented.
Correct, but the test example in 00/03 demonstrates that there's no
modification. For workloads that only execute and not modify,
IMA shouldn't evaluate more than once, but that's what we're
observing at least for XFS.
>
> In which situation there is a comparison zero to zero?
My mistake. You're right, but for the wrong reason.
To be clear, these patches are about the STATX_CHANGE_COOKIE mechanic.
XFS updates the i_version regardless of the multigrain ctime changes.
You're correct in that with/without this patch there is no zero-zero
comparison for XFS, and that's because XFS isn't setting the
STATX_CHANGE_COOKIE in the result mask either for last writer check or
the attrs changed check, thus a change is always detected with
integrity_inode_attrs_stat_changed(), and thus maintains current
IMA behavior for XFS.
That said, should a file system set STATX_CHANGE_COOKIE in the result mask,
and not update the i_version (say its kept at zero), then it's always
zero-zero. I don't know how likely that scenario is.
I should reword this commit, but I am a bit hesitant to say "don't
squash this in with patch 3" due to that uncertainty.
>
> Thanks
>
> Roberto
>
> > change detected. This is not dissimilar to what
> > ima_check_last_writer() does.
> >
> > No logical change intended for EVM.
> >
> > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > ---
> > include/linux/integrity.h | 28 ++++++++++++++++++++++++----
> > security/integrity/evm/evm_main.c | 5 ++---
> > security/integrity/ima/ima_main.c | 5 ++---
> > 3 files changed, 28 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > index beb9ab19fa6257e79266b58bcb5f55b0c5445828..382c783f0fa3ae4a938cdf9559291ba1903a378e 100644
> > --- a/include/linux/integrity.h
> > +++ b/include/linux/integrity.h
> > @@ -9,6 +9,7 @@
> >
> > #include <linux/fs.h>
> > #include <linux/iversion.h>
> > +#include <linux/kernel.h>
> >
> > enum integrity_status {
> > INTEGRITY_PASS = 0,
> > @@ -62,14 +63,33 @@ integrity_inode_attrs_stat_changed
> >
> > /*
> > * On stacked filesystems detect whether the inode or its content has changed.
> > + *
> > + * Must be called in process context.
> > */
> > static inline bool
> > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > - const struct inode *inode)
> > + struct file *file, struct inode *inode)
> > {
> > - return (inode->i_sb->s_dev != attrs->dev ||
> > - inode->i_ino != attrs->ino ||
> > - !inode_eq_iversion(inode, attrs->version));
> > + struct kstat stat;
> > +
> > + might_sleep();
> > +
> > + if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
> > + return true;
> > +
> > + /*
> > + * EVM currently relies on backing inode i_version. While IS_I_VERSION
> > + * is not a good indicator of i_version support, this still retains
> > + * the logic such that a re-evaluation should still occur for EVM, and
> > + * only for IMA if vfs_getattr_nosec() fails.
> > + */
> > + if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> > + STATX_CHANGE_COOKIE,
> > + AT_STATX_SYNC_AS_STAT))
> > + return !IS_I_VERSION(inode) ||
> > + !inode_eq_iversion(inode, attrs->version);
> > +
> > + return integrity_inode_attrs_stat_changed(attrs, &stat);
> > }
> >
> >
> > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
> > --- a/security/integrity/evm/evm_main.c
> > +++ b/security/integrity/evm/evm_main.c
> > @@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > bool ret = false;
> >
> > if (iint) {
> > - ret = (!IS_I_VERSION(metadata_inode) ||
> > - integrity_inode_attrs_changed(&iint->metadata_inode,
> > - metadata_inode));
> > + ret = integrity_inode_attrs_changed(&iint->metadata_inode,
> > + NULL, metadata_inode);
> > if (ret)
> > iint->evm_status = INTEGRITY_UNKNOWN;
> > }
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 6570ad10887b9ea1172c78274cf62482350e87ff..8cb17c9d446caaa5a98f5ec8f027c17ba7babca8 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -328,9 +328,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > real_inode = d_real_inode(file_dentry(file));
> > if (real_inode != inode &&
> > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > - if (!IS_I_VERSION(real_inode) ||
> > - integrity_inode_attrs_changed(&iint->real_inode,
> > - real_inode)) {
> > + if (integrity_inode_attrs_changed(&iint->real_inode,
> > + file, real_inode)) {
> > iint->flags &= ~IMA_DONE_MASK;
> > iint->measured_pcrs = 0;
> > }
> >
>
^ permalink raw reply
* Re: [PATCH v4] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Mimi Zohar @ 2026-02-04 19:32 UTC (permalink / raw)
To: Frederick Lawler, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team
In-Reply-To: <1da265acd3168a5c2b7390ddafc6a46d5e2d5613.camel@linux.ibm.com>
Hi Fred,
On Fri, 2026-01-30 at 08:00 -0500, Mimi Zohar wrote:
> > Examples are fine, but first describe the problem - not detecting file change on
> > xfs.
> >
> > >
> > > In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
> > > will require re-evaluation. Stacking TMPFS on XFS for instance, once the
> > > inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
> > > leverages generic_fillattr() to update the change cookie.
> >
> > This sounds like the same issue - not detecting file change on xfs. The problem
> > is simply manifesting itself on stacked filesystems.
>
> Splitting this patch, so that the base XFS changes are in one patch and the
> stacked filesystem changes are in the other, would really help clarify what is
> needed and the reason why.
Tweaking your script with the following changes, you'll see duplicate audit
entries on the base XFS system, without requiring an overlay filesystem.
echo "audit func=FILE_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
setpriv --reuid nobody $PWD/rdwr "$TEST_FILE"
setpriv --reuid nobody $PWD/rdwr "$TEST_FILE"
audit_count=$(grep -e "file=\"$TEST_FILE\"" /var/log/audit/audit.log | grep -c
"comm=\"rdwr\"")
"rdwr" is a C program that opens the file read-write, using the open() syscall.
Mimi
^ permalink raw reply
* Re: [PATCH v2 0/6] Landlock: Implement scope control for pathname Unix sockets
From: Mickaël Salaün @ 2026-02-04 17:43 UTC (permalink / raw)
To: Günther Noack
Cc: Tingmao Wang, Justin Suess, Günther Noack,
Demi Marie Obenour, Alyssa Ross, Jann Horn, Tahera Fahimi,
linux-security-module, Matthieu Buffet
In-Reply-To: <aYMxHRwHCao30HfX@google.com>
On Wed, Feb 04, 2026 at 12:44:29PM +0100, Günther Noack wrote:
> On Tue, Feb 03, 2026 at 09:53:11PM +0000, Tingmao Wang wrote:
> > On 2/3/26 17:54, Günther Noack wrote:
> > > BTW, regarding the implementation: To have *OR* semantics for "within
> > > scope" and "allow-listed path", the implementation will be
> > > non-trivial, and I suspect we won't hit the merge window if we try to
> > > get them both in for 7.0. But in my mind, a simple UAPI is more
> > > important than trying to make it in time for the next merge window.
> > >
> > > (The implementation is difficult because the path-based and
> > > scope-based check currently happen in different LSM hooks, and none of
> > > the two hooks has enough information to make the decision alone. The
> > > second hook only gets called if the first returns 0. It'll require
> > > some further discussion to make it work together.)
> >
> > Right. In that case, would it make sense to pass sk into the new
> > security_unix_find() hook, perhaps with the new argument named `struct
> > sock *other`? Then we can use this hook for the scope check as well by
> > using landlock_cred(other->sk_socket->file->f_cred)->domain etc.
> >
> > diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> > index 227467236930..db9d279b3883 100644
> > --- a/net/unix/af_unix.c
> > +++ b/net/unix/af_unix.c
> > @@ -1223,24 +1223,24 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
> >
> > err = -ECONNREFUSED;
> > inode = d_backing_inode(path.dentry);
> > if (!S_ISSOCK(inode->i_mode))
> > goto path_put;
> >
> > + err = -ECONNREFUSED;
> > + sk = unix_find_socket_byinode(inode);
> > + 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, type, flags);
> > + err = security_unix_find(&path, sk, flags);
> > if (err)
> > - goto path_put;
> > -
> > - err = -ECONNREFUSED;
> > - sk = unix_find_socket_byinode(inode);
> > - if (!sk)
> > - goto path_put;
> > + goto sock_put;
> >
> > err = -EPROTOTYPE;
> > if (sk->sk_type == type)
> > touch_atime(&path);
> > else
> > goto sock_put;
> >
> > By doing this we won't even need to pass `type` separately anymore. The
> > only change would be that now one can determine if a socket is bound or
> > not even without being allowed RESOLVE_UNIX access. I'm not sure how much
> > of an issue this is, but we could also call the hook anyway with a NULL in
> > place of the new argument, if unix_find_socket_byinode() fails. Other
> > LSMs can then decide what to do in that case (either return -ECONNREFUSED
> > or -EPERM).
>
> Thank you for the suggestion.
>
> Small caveat is that the LSM interface is very central and we should
> be careful. We have previously gotten the advice from Paul to design
> the hooks in an LSM-independent way that ideally reflects the
> arguments to the unix_find_bsd() function, and this would now deviate
> (slightly) from that, but simplifying the implementation for us. In
> my personal opinion, this might be worth doing the trade-off, if
> AppArmor people also agree, but we should double check.
I think that's a good idea. The new hook would more generic and make it
possible to have more context to take an access control decision, which
is better for the LSM framework.
>
> To keep the discussion of implementation and interface separate, I
> have raised this question in the pathname-restricted-UNIX patch set
> thread in [1].
>
> [1] https://lore.kernel.org/all/aYMenaSmBkAsFowd@google.com/
>
>
>
> > >> Furthermore, an application / Landlock config etc can always opt to not
> > >> use the scope bit at all, if it "knows" all the locations where the
> > >> application's sockets would be placed, and just use RESOLVE_UNIX access
> > >> right (or nothing if it is not supported).
> > >>
> > >> (The following is a bit of a side note, not terribly relevant if we're
> > >> deciding to go with the patch as is.)
> > >>
> > >>>> [...]
> > >>>> Another way to put it is that, if FS-based and scope-based controls
> > >>>> interacts in the above proposed way, both mechanisms feel like "poking
> > >>>> holes" in the other. But as Mickaël said, one can think of the two
> > >>>> mechanisms not as independent controls, but rather as two interfaces for
> > >>>> the same control. The socket access control is "enabled" if either the
> > >>>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX access is handled, or the scope bit
> > >>>> proposed in this patch is enabled.
> > >>>>
> > >>>> With that said, I can think of some alternative ways that might make this
> > >>>> API look "better" (from a subjective point of view, feedback welcome),
> > >>>> however it does mean more delays, and specifically, these will depend on
> > >>>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX:
> > >>>>
> > >>>> One possibility is to simply always allow a Landlock domain to connect to
> > >>>> its own sockets (in the case where LANDLOCK_ACCESS_FS_RESOLVE_UNIX is
> > >>>> handled, otherwise all sockets are allowed). This might be reasonable, as
> > >>>> one can only connect to a socket it creates if it has the permission to
> > >>>> create it in the first place, which is already controlled by
> > >>>> LANDLOCK_ACCESS_FS_MAKE_SOCK, so we don't really lose any policy
> > >>>> flexibility here - if for some reason the sandboxer don't want to allow
> > >>>> access to any (pathname) sockets, even the sandboxed app's own ones, it
> > >>>> can just not allow LANDLOCK_ACCESS_FS_MAKE_SOCK anywhere.
> > >>>
> > >>> LANDLOCK_ACCESS_FS_MAKE_SOCK is only required to bind/listen to a
> > >>> socket, not to connect. I guess you was thinking about
> > >>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX in this case?
> > >>
> > >> In this "allow same-scope connect unconditionally" proposal, the
> > >> application would still be able to (bind to and) connect to its own
> > >> sockets, even if LANDLOCK_ACCESS_FS_RESOLVE_UNIX is handled and nothing is
> > >> allowed to have LANDLOCK_ACCESS_FS_RESOLVE_UNIX access. But a sandboxer
> > >> which for whatever reason doesn't want this "allow same scope" default can
> > >> still prevent the use of (pathname) sockets by restricting
> > >> LANDLOCK_ACCESS_FS_MAKE_SOCK, because if an app can't connect to any
> > >> sockets it doesn't own, and can't create any sockets itself either, then
> > >> it effectively can't connect to any sockets at all.
> > >>
> > >> (Although on second thought, I guess there could be a case where an app
> > >> first creates some socket files before doing landlock_restrict_self(),
> > >> then it might still be able to bind to these even without
> > >> LANDLOCK_ACCESS_FS_MAKE_SOCK?)
> > >
> > > FWIW, I also really liked Tingmao's first of the two listed
> > > possibilities in [1], where she proposed to introduce both rights
> > > together. In my understanding, the arguments we have discussed so far
> > > for that are:
> > >
> > > IN FAVOR:
> > >
> > > (pro1) Connecting to a UNIX socket in the same scope is always safe,
> > > and it makes it possible to use named UNIX sockets between the
> > > processes within a Landlock domains. (Mickaël convinced me in
> > > discussion at FOSDEM that this is true.)
> > >
> > > If someone absolutely does not want that, they can restrict
> > > LANDLOCK_ACCESS_FS_MAKE_SOCK and achieve the same effect (as
> > > Tingmao said above).
> > >
> > > (pro2) The implementation of this is simpler.
> > >
> > > (I attempted to understand how the "or" semantics would be
> > > implemented, and I found it non-trivial when you try to do it
> > > for all layers at once. (Kernighan's Law applies, IMHO))
> >
> > I think the logic would basically be:
> >
> > 1. if any layers deny the access due to handled RESOLVE_UNIX but does not
> > have the scope bit set, then we will deny rightaway, without calling
> > domain_is_scoped().
> >
> > 2. Call domain_is_scoped() with a bitmask of "rules_covered" layers where
> > there are RESOLVE_UNIX rules covering the socket being accessed, and
> > essentially ignore those layers in the scope violation check.
> >
> > I definitely agree that it is tricky, but making same-scope access be
> > allowed (i.e. the suggested idea above) would only get rid of step 1,
> > which I think is the "simpler" bit. The extra logic in step 2 is still
> > needed.
> >
> > I definitely agree with pro1 tho.
>
> Yes, you are describing the logic for the variant where
> LANDLOCK_ACCESS_FS_RESOLVE_UNIX does not implicitly permit access from
> within the same scope. In that variant, there can be situations where
> the first hook can deny the action immediately.
>
> In the variant where LANDLOCK_ACCESS_FS_RESOLVE_UNIX *does* implicitly
> allow access from within the same scope, that shortcutting is not
> possible. On the upside however, there is no need to distinguish
> whether the scope flag is set when we are in the security_unix_find()
> hook, because access from within the same scope is always permitted.
> (That is the simplification I meant.)
This proposal make sense, improve IPC restriction consistency (by always
be able to connect to peers from the same domain), and would simplify
some checks. What bother me the most is the implicit scoping induced
by the FS_RESOLVE_UNIX handling. A related issue is the mix of checks
for the FS_RESOLVE_UNIX implementation, which could make the code more
complex, but I don't see a better way. Landlock already has an implicit
ptrace scoping but it's not (and should never be) optional.
See my proposal below about implicit
LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET. That would not change the kernel
checks you described though.
>
>
> > > AGAINST:
> > >
> > > (con1) It would work differently than the other scoped access rights
> > > that we already have.
> > >
> > > A speculative feature that could potentially be built with the
> > > scoped access rights is that we could add a rule to permit IPC
> > > to other Landlock scopes, e.g. introducing a new rule type
> > >
> > > struct landlock_scope_attr {
> > > __u64 allowed_access; /* for "scoped" bits */
> > > /* some way to identify domains */
> > > }
> > >
> > > so that we could make IPC access to other Landlock domains
> > > configurable.
> > >
> > > If the scoped bit and the FS RESOLVE_UNIX bit were both
> > > conflated in RESOLVE_UNIX, it would not be possible to make
> > > UNIX connections configurable in such a way.
> >
> > This exact API would no longer work, but if we give up the equivalence
> > between scope bits and the landlock_scope_attr struct, then we can do
> > something like:
> >
> > struct landlock_scope_attr {
> > __u64 ptrace:1; /* Note that this is not a (user controllable) scope bit! */
> > __u64 abstract_unix_socket:1;
> > __u64 pathname_unix_socket:1;
> > /* ... */
> >
> > __u64 allowed_signals;
> >
> > /*
> > * some way to identify domains, maybe we could use the audit domain
> > * ID, with 0 denoting "allow access to non-Landlocked processes?
> > */
> > }
>
> Yes, it would be possible to use such a struct for that scenario where
> IPC access gets allowed for other Landlock scopes. It would mean that
> we would not need to introduce a scoped flag for the pathname UNIX
> socket connections. But the relationship between that struct
> landlock_scope_attr and the flags and access rights in struct
> landlock_ruleset_attr would become less clear, which is a slight
> downside, and maybe error prone for users to work with.
>
> If we introduced an additional scoped flag, it would also be
> consistent though.
>
> (con1) was written under the assumption that we do not have an
> additional scoped flag. If that is lacking, it is not possible to
> express UNIX connect() access to other Landlock domains with that
> struct. But as outlined in the proposal below, if we *do* (later)
> introduce the additional scoped flag *in addition* to the FS access
> right, this *both* stays consistent in semantics with the signal and
> abstract UNIX support, *and* it starts working in a world where ICP
> access can be allowed to talk to other Landlock domains.
In this case, we need to assume the scoped flag is already there, and
because we have the implementation, to actually add it now.
As a side note, I don't really like this landlock_scope_attr interface.
A dedicated attr per kind of IPC would be much more flexible (e.g. the
handled_signal_number you described below). For instance, even if at
first we don't make it possible to describe different signals within the
attr struct, this struct will be able to grow. So it's better to have a
dedicated interface per IPC type than a generic scope interface that can
only express a subset of it.
Anyway, this proposal could work for pathname UNIX sockets, but a future
signal attr would be a bit inconsistent wrt to the pathname UNIX one.
>
> > > (con2) Consistent behaviour between scoped flags and their
> > > interactions with other access rights:
> > >
> > > The existing scoped access rights (signal, abstract sockets)
> > > could hypothetically be extended with a related access right of
> > > another type. For instance, there could be an access right type
> > >
> > > __u64 handled_signal_number;
> > >
> > > and then you could add a rule to permit the use of certain
> > > signal numbers. The interaction between the scoped flags and
> > > other access rights should work the same.
> > >
> > >
> > > Constructive Proposal for consideration: Why not both?
> > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > I will think about the following a bit more but I'm afraid that I feel
> > like it might get slightly confusing. With this, the only reason for
> > having LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET is for API consistency when we
> > later enable allowing access to other domains (if I understood correctly),
> > in which case I personally feel like the suggestion on landlock_scope_attr
> > above, where we essentially accept that it is decoupled with the scope
> > bits in the ruleset, might be simpler...?
>
> Mickaël expressed the opinion to me that he would like to APIs to stay
> consistent between signals, abstract UNIX sockets, named UNIX sockets
> and other future "scoped" operations, in scenarios where:
>
> * the "scoped" (IPC) operations can be configured to give access to
> other Landlock domains (and that should work for UNIX connections too)
> * the existing "scoped" operations also start having matching access rights
>
> I think with the way I proposed, that would be consistent.
What about always implicitly set LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET (in
the kernel) when LANDLOCK_ACCESS_FS_RESOLVE_UNIX is set?
*Requiring* users to set LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET would add a
new way to trigger an error, but setting
LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET *implicitly* looks safer. This
might also help for the documentation BTW.
If we agree on this, I could merge the scope_pathname_unix in 7.0, and
merge the resolve_unix in 7.1 (or later if something unexpected happen).
Otherwise, I'll merge both at the same time with the same ABI version
(with the risk to postpone again). WDYT?
>
>
> > > Why not do both what Tingmao proposed in [1] **and** reserve the
> > > option to add the matching "scoped flag" later?
> > >
> > > * Introduce LANDLOCK_ACCESS_FS_RESOLVE_UNIX.
> > >
> > > If it is handled, UNIX connections are allowed either:
> > >
> > > (1) if the connection is to a service in the same scope, or
> > > (2) if the path was allow-listed with a "path beneath" rule.
> > >
> > > * Add LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET later, if needed.
> > >
> > >
> > > Let's go through the arguments again:
> > >
> > > We have observed that it is harmless to allow connections to services
> > > in the same scope (1), and that if users absolutely don't want that,
> > > they can actually prohibit it through LANDLOCK_ACCESS_FS_MAKE_SOCK
> > > (pro1).
> > >
> > > (con1): Can we still implement the feature idea where we poke a hole
> > > to get UNIX-connect() access to other Landlock domains?
> > >
> > > I think the answer is yes. The implementation strategy is:
> > >
> > > * Add the scoped bit LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
> > > * The scoped bit can now be used to allow-list connections to
> > > other Landlock domains.
> > >
> > > For users, just setting the scoped bit on its own does the same as
> > > handling LANDLOCK_ACCESS_FS_RESOLVE_UNIX. That way, the kernel-side
> > > implementation can also stay simple. The only reason why the scoped
> > > bit is needed is because it makes it possible to allow-list
> > > connections to other Landlock domains, but at the same time, it is
> > > safe if libraries set the scoped bit once it exists, as it does not
> > > have any bad runtime impact either.
> > >
> > > (con2): Consistency: Do all the scoped flags interact with their
> > > corresponding access rights in the same way?
> > >
> > > The other scope flags do not have corresponding access rights, so
> > > far.
> > >
> > > If we were to add corresponding access rights for the other scope
> > > flags, I would argue that we could apply a consistent logic there,
> > > because IPC access within the same scope is always safe:
> > >
> > > - A hypothetical access right type for "signal numbers" would only
> > > restrict signals that go beyond the current scope.
> > >
> > > - A hypothetical access right type for "abstract UNIX domain socket
> > > names" would only restrict connections to abstract UNIX domain
> > > servers that go beyond the current scope.
> > >
> > > I can not come up with a scenario where this doesn't work.
> > >
> > >
> > > In conclusion, I think the approach has significant upsides:
> > >
> > > * Simpler UAPI: Users only have one access bit to deal with, in the
> > > near future. Once we do add a scope flag for UNIX connections, it
> > > does not interact in a surprising way with the corresponding FS
> > > access right, because with either of these, scoped access is
> > > allowed.
> > >
> > > If users absolutely need to restrict scoped access, they can
> > > restrict LANDLOCK_ACCESS_FS_MAKE_SOCK. It is a slightly obscure
> > > API, but in line with the "make easy things easy, make hard things
> > > possible" API philosophy. And needing this should be the
> > > exception rather than the norm, after all.
> > >
> > > * Consistent behaviour between scoped flags and regular access
> > > rights, also for speculative access rights affecting the existing
> > > scoped flags for signals and abstract UNIX domain sockets.
> > >
> > > [1] https://lore.kernel.org/all/f07fe41a-96c5-4d3a-9966-35b30b3a71f1@maowtm.org/
Thanks for this summary Günther!
^ permalink raw reply
* Re: [PATCH v2 0/6] Landlock: Implement scope control for pathname Unix sockets
From: Mickaël Salaün @ 2026-02-04 18:28 UTC (permalink / raw)
To: Justin Suess
Cc: Günther Noack, Tingmao Wang, Günther Noack,
Demi Marie Obenour, Alyssa Ross, Jann Horn, Tahera Fahimi,
linux-security-module, Matthieu Buffet
In-Reply-To: <bfcac905-5901-4cfb-8489-fb228cef0ee0@gmail.com>
On Wed, Feb 04, 2026 at 11:36:06AM -0500, Justin Suess wrote:
>
> On 2/4/26 06:44, Günther Noack wrote:
> > On Tue, Feb 03, 2026 at 09:53:11PM +0000, Tingmao Wang wrote:
> >> On 2/3/26 17:54, Günther Noack wrote:
> >>> BTW, regarding the implementation: To have *OR* semantics for "within
> >>> scope" and "allow-listed path", the implementation will be
> >>> non-trivial, and I suspect we won't hit the merge window if we try to
> >>> get them both in for 7.0. But in my mind, a simple UAPI is more
> >>> important than trying to make it in time for the next merge window.
> >>>
> >>> (The implementation is difficult because the path-based and
> >>> scope-based check currently happen in different LSM hooks, and none of
> >>> the two hooks has enough information to make the decision alone. The
> >>> second hook only gets called if the first returns 0. It'll require
> >>> some further discussion to make it work together.)
> >> Right. In that case, would it make sense to pass sk into the new
> >> security_unix_find() hook, perhaps with the new argument named `struct
> >> sock *other`? Then we can use this hook for the scope check as well by
> >> using landlock_cred(other->sk_socket->file->f_cred)->domain etc.
>
> Tingmao,
>
> This seems like the best way to do it. Alternatively, I considered passing in just the
> f_cred to the hook, but I'm leaning towards a more generic implementation like the one you
> have below.
>
> >>
> >> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> >> index 227467236930..db9d279b3883 100644
> >> --- a/net/unix/af_unix.c
> >> +++ b/net/unix/af_unix.c
> >> @@ -1223,24 +1223,24 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
> >>
> >> err = -ECONNREFUSED;
> >> inode = d_backing_inode(path.dentry);
> >> if (!S_ISSOCK(inode->i_mode))
> >> goto path_put;
> >>
> >> + err = -ECONNREFUSED;
> >> + sk = unix_find_socket_byinode(inode);
> >> + 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, type, flags);
> >> + err = security_unix_find(&path, sk, flags);
> >> if (err)
> >> - goto path_put;
> >> -
> >> - err = -ECONNREFUSED;
> >> - sk = unix_find_socket_byinode(inode);
> >> - if (!sk)
> >> - goto path_put;
> >> + goto sock_put;
> >>
> >> err = -EPROTOTYPE;
> >> if (sk->sk_type == type)
> >> touch_atime(&path);
> >> else
> >> goto sock_put;
> >>
> >> By doing this we won't even need to pass `type` separately anymore. The
> >> only change would be that now one can determine if a socket is bound or
> >> not even without being allowed RESOLVE_UNIX access. I'm not sure how much
> >> of an issue this is, but we could also call the hook anyway with a NULL in
> >> place of the new argument, if unix_find_socket_byinode() fails. Other
> >> LSMs can then decide what to do in that case (either return -ECONNREFUSED
> >> or -EPERM).
> I think landlock already allows checking of existence of files even when when the process
> doesn't have rights on them, so there is precedent for this so in my mind this would probably be OK.
I agree, there are a lot of different side channels in Linux. Landlock
is about access control, like most LSMs, so this should not be an issue.
And if an LSM wants to hide this information, it can also return
-ECONNREFUSED or something else.
> > Thank you for the suggestion.
> >
> > Small caveat is that the LSM interface is very central and we should
> > be careful. We have previously gotten the advice from Paul to design
> > the hooks in an LSM-independent way that ideally reflects the
> > arguments to the unix_find_bsd() function, and this would now deviate
> > (slightly) from that, but simplifying the implementation for us. In
> > my personal opinion, this might be worth doing the trade-off, if
> > AppArmor people also agree, but we should double check.
>
> Gunther:
>
> I'd be happy to send you an updated LSM hook patch with the sock parameter.
>
> Just let me know.
>
> >
> > To keep the discussion of implementation and interface separate, I
> > have raised this question in the pathname-restricted-UNIX patch set
> > thread in [1].
> >
> > [1] https://lore.kernel.org/all/aYMenaSmBkAsFowd@google.com/
> >
> >
> >
> >>>> Furthermore, an application / Landlock config etc can always opt to not
> >>>> use the scope bit at all, if it "knows" all the locations where the
> >>>> application's sockets would be placed, and just use RESOLVE_UNIX access
> >>>> right (or nothing if it is not supported).
> >>>>
> >>>> (The following is a bit of a side note, not terribly relevant if we're
> >>>> deciding to go with the patch as is.)
> >>>>
> >>>>>> [...]
> >>>>>> Another way to put it is that, if FS-based and scope-based controls
> >>>>>> interacts in the above proposed way, both mechanisms feel like "poking
> >>>>>> holes" in the other. But as Mickaël said, one can think of the two
> >>>>>> mechanisms not as independent controls, but rather as two interfaces for
> >>>>>> the same control. The socket access control is "enabled" if either the
> >>>>>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX access is handled, or the scope bit
> >>>>>> proposed in this patch is enabled.
> >>>>>>
> >>>>>> With that said, I can think of some alternative ways that might make this
> >>>>>> API look "better" (from a subjective point of view, feedback welcome),
> >>>>>> however it does mean more delays, and specifically, these will depend on
> >>>>>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX:
> >>>>>>
> >>>>>> One possibility is to simply always allow a Landlock domain to connect to
> >>>>>> its own sockets (in the case where LANDLOCK_ACCESS_FS_RESOLVE_UNIX is
> >>>>>> handled, otherwise all sockets are allowed). This might be reasonable, as
> >>>>>> one can only connect to a socket it creates if it has the permission to
> >>>>>> create it in the first place, which is already controlled by
> >>>>>> LANDLOCK_ACCESS_FS_MAKE_SOCK, so we don't really lose any policy
> >>>>>> flexibility here - if for some reason the sandboxer don't want to allow
> >>>>>> access to any (pathname) sockets, even the sandboxed app's own ones, it
> >>>>>> can just not allow LANDLOCK_ACCESS_FS_MAKE_SOCK anywhere.
> >>>>> LANDLOCK_ACCESS_FS_MAKE_SOCK is only required to bind/listen to a
> >>>>> socket, not to connect. I guess you was thinking about
> >>>>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX in this case?
> >>>> In this "allow same-scope connect unconditionally" proposal, the
> >>>> application would still be able to (bind to and) connect to its own
> >>>> sockets, even if LANDLOCK_ACCESS_FS_RESOLVE_UNIX is handled and nothing is
> >>>> allowed to have LANDLOCK_ACCESS_FS_RESOLVE_UNIX access. But a sandboxer
> >>>> which for whatever reason doesn't want this "allow same scope" default can
> >>>> still prevent the use of (pathname) sockets by restricting
> >>>> LANDLOCK_ACCESS_FS_MAKE_SOCK, because if an app can't connect to any
> >>>> sockets it doesn't own, and can't create any sockets itself either, then
> >>>> it effectively can't connect to any sockets at all.
> >>>>
> >>>> (Although on second thought, I guess there could be a case where an app
> >>>> first creates some socket files before doing landlock_restrict_self(),
> >>>> then it might still be able to bind to these even without
> >>>> LANDLOCK_ACCESS_FS_MAKE_SOCK?)
> >>> FWIW, I also really liked Tingmao's first of the two listed
> >>> possibilities in [1], where she proposed to introduce both rights
> >>> together. In my understanding, the arguments we have discussed so far
> >>> for that are:
> >>>
> >>> IN FAVOR:
> >>>
> >>> (pro1) Connecting to a UNIX socket in the same scope is always safe,
> >>> and it makes it possible to use named UNIX sockets between the
> >>> processes within a Landlock domains. (Mickaël convinced me in
> >>> discussion at FOSDEM that this is true.)
> >>>
> >>> If someone absolutely does not want that, they can restrict
> >>> LANDLOCK_ACCESS_FS_MAKE_SOCK and achieve the same effect (as
> >>> Tingmao said above).
> >>>
> >>> (pro2) The implementation of this is simpler.
> >>>
> >>> (I attempted to understand how the "or" semantics would be
> >>> implemented, and I found it non-trivial when you try to do it
> >>> for all layers at once. (Kernighan's Law applies, IMHO))
> >> I think the logic would basically be:
> >>
> >> 1. if any layers deny the access due to handled RESOLVE_UNIX but does not
> >> have the scope bit set, then we will deny rightaway, without calling
> >> domain_is_scoped().
> >>
> >> 2. Call domain_is_scoped() with a bitmask of "rules_covered" layers where
> >> there are RESOLVE_UNIX rules covering the socket being accessed, and
> >> essentially ignore those layers in the scope violation check.
>
> Tingmao:
>
> For connecting a pathname unix socket, the order of the hooks landlock sees is something like:
>
> 1. security_unix_find. (to look up the paths)
>
> 2. security_unix_may_send, security_unix_stream_connect (after the path is looked up)
>
> Which for is called in DGRAM:
>
> unix_dgram_connect OR unix_dgram_sendmsg
>
> and for STREAM:
>
> unix_stream_connect
>
> IIRC, the path lookup only occurs in this order always, so in the logic as you have it the domain_is_scoped()
> would be called twice, once from the security_unix_find when you call it in step two, and once from the
> domain scope hooks. (If access was allowed from security_unix_find)
>
> There are a couple of things to consider.
>
> ---
>
> Audit blockers need special handling:
>
> Here's an example:
>
> 1. Program A enforces a ruleset with RESOLVE_UNIX and the unix pathname scope bit, with no rules with that
> access bit (deny all for RESOLVE_UNIX).
>
> 2. Program A connects to /tmp/mysock.sock ran by program B, which is outside the domain.
>
> 2. security_unix_find is hit to lookup the path, and the RESOLVE_UNIX code doesn't grant access to
> /tmp/mysock.sock, so it calls domain_is_scoped
>
> 3. domain_is_scoped denies it as well, so now we must log an audit record.
>
> When logging the denial, we have to include both blockers "scope.unix_socket" and "fs.resolve_unix" for the
> denial, because it is the absence of both that caused the denial. I think the refer right has similar cases for auditing, so there is precedent for this (multiple blockers for an audit message).
That's a good point, and it would give more informations to diagnose
issues. However, being able to identify if both accesses are denied
would require to check both, whereas the first is enough to know that
Landlock denies the access. So, if we can return both records without
continuing the security checks, that's good, otherwise we should stop
ASAP and return the error.
Anyway, that might not be needed if we end up with my latest proposal
about always setting scope.unix_socket when fs.resolve_unix is set.
>
> ---
>
> Dual lookup for domain_is_scoped. Consider this case:
>
> 1. Program A enforces a ruleset with RESOLVE_UNIX and the unix pathname scope bit, with no rules with that
> access bit (deny all for RESOLVE_UNIX).
>
> 2. Program A connects to Program C's /tmp/foo.sock, which for the purposes of this example is in the domain of program A.
>
> 3. security_unix_find is hit to lookup the path, and the RESOLVE_UNIX code doesn't grant access to
> /tmp/mysock.sock, so it calls domain_is_scoped. Access is granted, and continues. (LSM hook complete)
>
> 4. The connection proceeds past the path lookup stage, and now security_unix_may_send, or security_unix_stream_connect gets called. This requires ANOTHER domain_is_scoped access check.
>
> While I don't THINK this introduces a TOCTOU, it is a little confusing.
>
> This does mean that we look up the domain twice, if this is implemented naively. I think we can then just
> skip the task credential checks then for security_unix_may_send and security_unix_stream_connect **for
> connecting to pathname sockets**, since the domain_is_scoped will already have been called in landlock's
> security_unix_find hook, eliminating the need for handling pathname socket domain checks layer on.
>
> >>
> >> I definitely agree that it is tricky, but making same-scope access be
> >> allowed (i.e. the suggested idea above) would only get rid of step 1,
> >> which I think is the "simpler" bit. The extra logic in step 2 is still
> >> needed.
> >>
> >> I definitely agree with pro1 tho.
> > Yes, you are describing the logic for the variant where
> > LANDLOCK_ACCESS_FS_RESOLVE_UNIX does not implicitly permit access from
> > within the same scope. In that variant, there can be situations where
> > the first hook can deny the action immediately.
> >
> > In the variant where LANDLOCK_ACCESS_FS_RESOLVE_UNIX *does* implicitly
> > allow access from within the same scope, that shortcutting is not
> > possible. On the upside however, there is no need to distinguish
> > whether the scope flag is set when we are in the security_unix_find()
> > hook, because access from within the same scope is always permitted.
> > (That is the simplification I meant.)
> >
> >
> >>> AGAINST:
> >>>
> >>> (con1) It would work differently than the other scoped access rights
> >>> that we already have.
> >>>
> >>> A speculative feature that could potentially be built with the
> >>> scoped access rights is that we could add a rule to permit IPC
> >>> to other Landlock scopes, e.g. introducing a new rule type
> >>>
> >>> struct landlock_scope_attr {
> >>> __u64 allowed_access; /* for "scoped" bits */
> >>> /* some way to identify domains */
> >>> }
> >>>
> >>> so that we could make IPC access to other Landlock domains
> >>> configurable.
> >>>
> >>> If the scoped bit and the FS RESOLVE_UNIX bit were both
> >>> conflated in RESOLVE_UNIX, it would not be possible to make
> >>> UNIX connections configurable in such a way.
> >> This exact API would no longer work, but if we give up the equivalence
> >> between scope bits and the landlock_scope_attr struct, then we can do
> >> something like:
> >>
> >> struct landlock_scope_attr {
> >> __u64 ptrace:1; /* Note that this is not a (user controllable) scope bit! */
> >> __u64 abstract_unix_socket:1;
> >> __u64 pathname_unix_socket:1;
> >> /* ... */
> >>
> >> __u64 allowed_signals;
> >>
> >> /*
> >> * some way to identify domains, maybe we could use the audit domain
> >> * ID, with 0 denoting "allow access to non-Landlocked processes?
> >> */
> >> }
> > Yes, it would be possible to use such a struct for that scenario where
> > IPC access gets allowed for other Landlock scopes. It would mean that
> > we would not need to introduce a scoped flag for the pathname UNIX
> > socket connections. But the relationship between that struct
> > landlock_scope_attr and the flags and access rights in struct
> > landlock_ruleset_attr would become less clear, which is a slight
> > downside, and maybe error prone for users to work with.
> >
> > If we introduced an additional scoped flag, it would also be
> > consistent though.
> >
> > (con1) was written under the assumption that we do not have an
> > additional scoped flag. If that is lacking, it is not possible to
> > express UNIX connect() access to other Landlock domains with that
> > struct. But as outlined in the proposal below, if we *do* (later)
> > introduce the additional scoped flag *in addition* to the FS access
> > right, this *both* stays consistent in semantics with the signal and
> > abstract UNIX support, *and* it starts working in a world where ICP
> > access can be allowed to talk to other Landlock domains.
> >
> >>> (con2) Consistent behaviour between scoped flags and their
> >>> interactions with other access rights:
> >>>
> >>> The existing scoped access rights (signal, abstract sockets)
> >>> could hypothetically be extended with a related access right of
> >>> another type. For instance, there could be an access right type
> >>>
> >>> __u64 handled_signal_number;
> >>>
> >>> and then you could add a rule to permit the use of certain
> >>> signal numbers. The interaction between the scoped flags and
> >>> other access rights should work the same.
> >>>
> >>>
> >>> Constructive Proposal for consideration: Why not both?
> >>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> I will think about the following a bit more but I'm afraid that I feel
> >> like it might get slightly confusing. With this, the only reason for
> >> having LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET is for API consistency when we
> >> later enable allowing access to other domains (if I understood correctly),
> >> in which case I personally feel like the suggestion on landlock_scope_attr
> >> above, where we essentially accept that it is decoupled with the scope
> >> bits in the ruleset, might be simpler...?
> > Mickaël expressed the opinion to me that he would like to APIs to stay
> > consistent between signals, abstract UNIX sockets, named UNIX sockets
> > and other future "scoped" operations, in scenarios where:
> >
> > * the "scoped" (IPC) operations can be configured to give access to
> > other Landlock domains (and that should work for UNIX connections too)
> > * the existing "scoped" operations also start having matching access rights
> >
> > I think with the way I proposed, that would be consistent.
> >
> >
> >>> Why not do both what Tingmao proposed in [1] **and** reserve the
> >>> option to add the matching "scoped flag" later?
> >>>
> >>> * Introduce LANDLOCK_ACCESS_FS_RESOLVE_UNIX.
> >>>
> >>> If it is handled, UNIX connections are allowed either:
> >>>
> >>> (1) if the connection is to a service in the same scope, or
> >>> (2) if the path was allow-listed with a "path beneath" rule.
> >>>
> >>> * Add LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET later, if needed.
> >>>
> >>>
> >>> Let's go through the arguments again:
> >>>
> >>> We have observed that it is harmless to allow connections to services
> >>> in the same scope (1), and that if users absolutely don't want that,
> >>> they can actually prohibit it through LANDLOCK_ACCESS_FS_MAKE_SOCK
> >>> (pro1).
> >>>
> >>> (con1): Can we still implement the feature idea where we poke a hole
> >>> to get UNIX-connect() access to other Landlock domains?
> >>>
> >>> I think the answer is yes. The implementation strategy is:
> >>>
> >>> * Add the scoped bit LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
> >>> * The scoped bit can now be used to allow-list connections to
> >>> other Landlock domains.
> >>>
> >>> For users, just setting the scoped bit on its own does the same as
> >>> handling LANDLOCK_ACCESS_FS_RESOLVE_UNIX. That way, the kernel-side
> >>> implementation can also stay simple. The only reason why the scoped
> >>> bit is needed is because it makes it possible to allow-list
> >>> connections to other Landlock domains, but at the same time, it is
> >>> safe if libraries set the scoped bit once it exists, as it does not
> >>> have any bad runtime impact either.
> >>>
> >>> (con2): Consistency: Do all the scoped flags interact with their
> >>> corresponding access rights in the same way?
> >>>
> >>> The other scope flags do not have corresponding access rights, so
> >>> far.
> >>>
> >>> If we were to add corresponding access rights for the other scope
> >>> flags, I would argue that we could apply a consistent logic there,
> >>> because IPC access within the same scope is always safe:
> >>>
> >>> - A hypothetical access right type for "signal numbers" would only
> >>> restrict signals that go beyond the current scope.
> >>>
> >>> - A hypothetical access right type for "abstract UNIX domain socket
> >>> names" would only restrict connections to abstract UNIX domain
> >>> servers that go beyond the current scope.
> >>>
> >>> I can not come up with a scenario where this doesn't work.
>
> Gunther / Tingmao / Mickaël:
>
> I have a potential idea to make this concept cleaner.
>
> The docs for landlock currently say:
>
>
> IPC scoping does not support exceptions via landlock_add_rule(2).
> If an operation is scoped within a domain, no rules can be added
> to allow access to resources or processes outside of the scope.
This part might indeed be confusing. The idea was to explain the
difference between scoped rights and handled access rights (which may
have rules).
>
> So if we go with the solution where we are now saying IPC scoping DOES support exceptions
> we will need to update the documentation, to say scoping for pathname unix sockets is an exception,
> and have to have the "exemptible scopes" (like this one) alongside "non-exemptible" scopes
> (ie the existing ones). This creates some friction for users.
The documentation will definitely require some updates. I think it can
be explained in a simple way.
>
> If we foresee other "exempt-able scopes" (which are scopes that also support creating exemptions w/ corresponding access rights) in the future, maybe we should consider separating the two in the ruleset
> attributes (I used scoped_fs as an example for the attribute name):
>
> structlandlock_ruleset_attrruleset_attr={
> .handled_access_fs=
> LANDLOCK_ACCESS_FS_EXECUTE|
> LANDLOCK_ACCESS_FS_WRITE_FILE|
> LANDLOCK_ACCESS_FS_READ_FILE|
> LANDLOCK_ACCESS_FS_READ_DIR|
> LANDLOCK_ACCESS_FS_REMOVE_DIR|
> LANDLOCK_ACCESS_FS_REMOVE_FILE|
> LANDLOCK_ACCESS_FS_MAKE_CHAR|
> LANDLOCK_ACCESS_FS_MAKE_DIR|
> LANDLOCK_ACCESS_FS_MAKE_REG|
> LANDLOCK_ACCESS_FS_MAKE_SOCK|
> LANDLOCK_ACCESS_FS_MAKE_FIFO|
> LANDLOCK_ACCESS_FS_MAKE_BLOCK|
> LANDLOCK_ACCESS_FS_MAKE_SYM|
> LANDLOCK_ACCESS_FS_REFER|
> LANDLOCK_ACCESS_FS_TRUNCATE|
> LANDLOCK_ACCESS_FS_IOCTL_DEV,
> .handled_access_net=
> LANDLOCK_ACCESS_NET_BIND_TCP|
> LANDLOCK_ACCESS_NET_CONNECT_TCP,
> .scoped=
> LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET|
> LANDLOCK_SCOPE_SIGNAL,
> .scoped_fs =
> LANDLOCK_SCOPE_FS_PATHNAME_UNIX_SOCKET
> };
>
> This more clearly distinguishes between scopes that have exceptions/corresponding fs rights,
> and ones that don't. Later we could add scoped_net, if needed. I feel like this would be more
> intuitive and better categorize future scoping rights. An obvious con is increasing the size of
> the ruleset attributes.
I see your point but I don't think it would be worth it to add
sub-scoped fields. Each field has a clear semantic, and the scoped one
is related to the domain wrt other domains.
>
> Of course this separation is only worth it if there are other "exempt-able" rights in the future.
> I can think of a few potential future rights which COULD be scoped and have corresponding rights
> (binder, sysv-ipc, pipes, tcp/udp between two local programs).
Yes, it would definitely be useful to add exception for other kind of
IPCs. The idea would be to be able to describe the peer, either with a
file path, or PID, or cgroups, or a Landlock domain... The inet case
is an interesting idea but that might be a challenging task to
implement, if even possible.
>
> >>>
> >>>
> >>> In conclusion, I think the approach has significant upsides:
> >>>
> >>> * Simpler UAPI: Users only have one access bit to deal with, in the
> >>> near future. Once we do add a scope flag for UNIX connections, it
> >>> does not interact in a surprising way with the corresponding FS
> >>> access right, because with either of these, scoped access is
> >>> allowed.
> >>>
> >>> If users absolutely need to restrict scoped access, they can
> >>> restrict LANDLOCK_ACCESS_FS_MAKE_SOCK. It is a slightly obscure
> >>> API, but in line with the "make easy things easy, make hard things
> >>> possible" API philosophy. And needing this should be the
> >>> exception rather than the norm, after all.
> >>>
> >>> * Consistent behaviour between scoped flags and regular access
> >>> rights, also for speculative access rights affecting the existing
> >>> scoped flags for signals and abstract UNIX domain sockets.
> >>>
> >>> [1] https://lore.kernel.org/all/f07fe41a-96c5-4d3a-9966-35b30b3a71f1@maowtm.org/
> > —Günther
>
^ permalink raw reply
* Re: [PATCH v2 0/6] Landlock: Implement scope control for pathname Unix sockets
From: Mickaël Salaün @ 2026-02-04 17:39 UTC (permalink / raw)
To: Tingmao Wang
Cc: Justin Suess, Günther Noack, Demi Marie Obenour,
Günther Noack, Alyssa Ross, Jann Horn, Tahera Fahimi,
linux-security-module, Matthieu Buffet
In-Reply-To: <16129d76-b6d3-4959-b241-dc79a32dd0cd@maowtm.org>
On Tue, Feb 03, 2026 at 01:26:31AM +0000, Tingmao Wang wrote:
> Hi Mickaël,
>
> Thanks for the feedback and explanations :)
>
> On 2/2/26 20:32, Mickaël Salaün wrote:
> > On Sat, Jan 31, 2026 at 05:41:14PM +0000, Tingmao Wang wrote:
> >> [...]
> >> What do folks think?
> >
> > I'd like to keep a clean API, with a "scoped" field handling IPC
> > scoping, and an "handled_access_fs" field handling filesystem-related
> > accesses.
> >
> > One thing to keep in mind is that we could add a new kind of "handled"
> > field that would enable to add rules identifying e.g. processes,
> > cgroups, or Landlock domains, and that could be used to add exceptions
> > to the current scopes. This means that we need to have a generic way to
> > handle this case.
> >
> > What is the issue with two complementary interfaces (scope and access)
> > used to express a policy about connecting to UNIX sockets? We just need
> > to make sure that scopes and handled_access_fs dealing with UNIX sockets
> > are like binary OR: if the scope is set, then the domain can communicate
> > with peers which are in the same domain, and if the handled_access_fs
> > right is set, then the domain can only communicate with matching sockets
> > (OR scoped ones if the scope is set).
>
> Right, I see what you're saying, especially with the "additional access
> rules for other scopes" example, and I think I'm happy with this. I guess
> my attempt at trying to make the API more "elegant" would introduce
> complexity and also create future inconsistency if other existing scope
> bits are combined with handled_access rules.
>
> > [...]
> > Anyway, we need to decide if this should be merged in Linux 7.0 (next
> > week) or not. I'd prefer to merge it now because I think it works well
> > and it's not a new concept wrt the abstract UNIX socket scoping.
> > However, if there are any concern, I'd like to hear them now and I can
> > delay this merge if needed. This patch series still need a new version
> > but that should only be about cosmetic fixes. WDYT?
>
> I ended up being pretty busy today but I can definitely send the next
> version tomorrow with your formatting changes and comments. I'm happy
> with it going into the next merge window if you are. Justin raises a
> point about having these two mechanisms in the same ABI version - see
> below for consideration.
>
> >> [...]
> >
> > My main concern is about user space libraries and users that may want to
> > have conditional enforcement for compatibility reasons e.g., only
> > enforce LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET (let's say ABI v8) if it can
> > also set LANDLOCK_ACCESS_FS_RESOLVE_UNIX (let's say ABI v9). I see two
> > ways to deal with this case (if needed):
> > - add synthetic access right to easily let users "combine" two access
> > rigths or none;
> > - have a more generic way to AND and OR access rights. I'm thinking
> > about updating the Rust library in this direction.
>
> I'm not sure I fully understand the complexity here, but I think, assuming
> these land in separate kernel versions, it will have to be that if both
> the scope bit and LANDLOCK_ACCESS_FS_RESOLVE_UNIX is requested (maybe if
> the user actually adds rules containing RESOLVE_UNIX access), but only the
> scope bit is supported, then it will have to skip enforcing pathname UNIX
> socket restrictions altogether by skipping both the scope bit and the
> RESOLVE_UNIX access (if in best effort mode), or fail (if in hard
> requirement mode).
Yeah, this should be OK in theory but it might be confusing to
developers.
>
> I don't immediately see how further customization ability (e.g. synthetic
> access rights or other AND/OR combination) could be used - if an app needs
> access to a privileged socket and can't pre-open it before
> landlock_restrict_self(), then the only realistic choice is to not use the
> scope bits if LANDLOCK_ACCESS_FS_RESOLVE_UNIX is not supported.
Yes, that's the main idea. The synthetic access right would just be
useful to avoid doing this check each time but let the library do it
instead. Anyway, that's mostly a (Rust) lib thing.
>
> On 2/2/26 22:03, Justin Suess wrote:
> > Regardless if you merge the patch series now in 7.0 or a later version, I think there is something to be said
> > about having the filesystem and scoped unix access right merged in the same ABI version / merge window.
> >
> > As you pointed out earlier, the combination of the two flags is much flexible and useful to userspace
> > consumers than one or the other, and if the features were merged separately, there would be an
> > awkward middle ABI where user space consumers may have to make compromises or changes to
> > sandbox between different versions or change application behavior.
> > [...]
>
> Given that the scope bit and RESOLVE_UNIX access right are in some sense
> part of the same system (they interact in an OR manner, after all), there
> is some positive for having them introduced in the same version, but on
> the other hand, with my above reasoning, I don't think these two
> mechanisms (scope bit and RESOLVE_UNIX access) being in different ABI
> versions would be too much of a problem. In either case, for applications
> which require access to more "privileged" sockets, when running on a
> kernel without the RESOLVE_UNIX access right support, no pathname socket
> restrictions can be applied (i.e. it won't use the scope bit either, there
> isn't much "compromise" it can make here). On the other hand, if
> RESOLVE_UNIX is supported, then it knows that the scope bit is also
> supported, and can just use it.
Yes
>
> Furthermore, an application / Landlock config etc can always opt to not
> use the scope bit at all, if it "knows" all the locations where the
> application's sockets would be placed, and just use RESOLVE_UNIX access
> right (or nothing if it is not supported).
>
> (The following is a bit of a side note, not terribly relevant if we're
> deciding to go with the patch as is.)
>
> >> [...]
> >> Another way to put it is that, if FS-based and scope-based controls
> >> interacts in the above proposed way, both mechanisms feel like "poking
> >> holes" in the other. But as Mickaël said, one can think of the two
> >> mechanisms not as independent controls, but rather as two interfaces for
> >> the same control. The socket access control is "enabled" if either the
> >> LANDLOCK_ACCESS_FS_RESOLVE_UNIX access is handled, or the scope bit
> >> proposed in this patch is enabled.
> >>
> >> With that said, I can think of some alternative ways that might make this
> >> API look "better" (from a subjective point of view, feedback welcome),
> >> however it does mean more delays, and specifically, these will depend on
> >> LANDLOCK_ACCESS_FS_RESOLVE_UNIX:
> >>
> >> One possibility is to simply always allow a Landlock domain to connect to
> >> its own sockets (in the case where LANDLOCK_ACCESS_FS_RESOLVE_UNIX is
> >> handled, otherwise all sockets are allowed). This might be reasonable, as
> >> one can only connect to a socket it creates if it has the permission to
> >> create it in the first place, which is already controlled by
> >> LANDLOCK_ACCESS_FS_MAKE_SOCK, so we don't really lose any policy
> >> flexibility here - if for some reason the sandboxer don't want to allow
> >> access to any (pathname) sockets, even the sandboxed app's own ones, it
> >> can just not allow LANDLOCK_ACCESS_FS_MAKE_SOCK anywhere.
> >
> > LANDLOCK_ACCESS_FS_MAKE_SOCK is only required to bind/listen to a
> > socket, not to connect. I guess you was thinking about
> > LANDLOCK_ACCESS_FS_RESOLVE_UNIX in this case?
>
> In this "allow same-scope connect unconditionally" proposal, the
> application would still be able to (bind to and) connect to its own
> sockets, even if LANDLOCK_ACCESS_FS_RESOLVE_UNIX is handled and nothing is
> allowed to have LANDLOCK_ACCESS_FS_RESOLVE_UNIX access. But a sandboxer
> which for whatever reason doesn't want this "allow same scope" default can
> still prevent the use of (pathname) sockets by restricting
> LANDLOCK_ACCESS_FS_MAKE_SOCK, because if an app can't connect to any
> sockets it doesn't own, and can't create any sockets itself either, then
> it effectively can't connect to any sockets at all.
>
> (Although on second thought, I guess there could be a case where an app
> first creates some socket files before doing landlock_restrict_self(),
> then it might still be able to bind to these even without
> LANDLOCK_ACCESS_FS_MAKE_SOCK?)
That's good to keep in mind. There might be weird cases but developers
should be encouraged to set all the scopes bits and then potentially
allow specific sockets with FS_RESOLVE_UNIX.
^ permalink raw reply
* Re: [PATCH v14 1/9] rust: types: Add Ownable/Owned types
From: Gary Guo @ 2026-02-04 16:56 UTC (permalink / raw)
To: Andreas Hindborg, Danilo Krummrich
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Asahi Lina
In-Reply-To: <87fr7gpk6d.fsf@t14s.mail-host-address-is-not-set>
On Wed Feb 4, 2026 at 4:06 PM GMT, Andreas Hindborg wrote:
> "Danilo Krummrich" <dakr@kernel.org> writes:
>
>> On Wed Feb 4, 2026 at 12:56 PM CET, Andreas Hindborg wrote:
>>> From: Asahi Lina <lina+kernel@asahilina.net>
>>>
>>> By analogy to `AlwaysRefCounted` and `ARef`, an `Ownable` type is a
>>> (typically C FFI) type that *may* be owned by Rust, but need not be. Unlike
>>> `AlwaysRefCounted`, this mechanism expects the reference to be unique
>>> within Rust, and does not allow cloning.
>>>
>>> Conceptually, this is similar to a `KBox<T>`, except that it delegates
>>> resource management to the `T` instead of using a generic allocator.
>>>
>>> This change is a derived work based on work by Asahi Lina
>>> <lina+kernel@asahilina.net> [1] and Oliver Mangold <oliver.mangold@pm.me>.
>>>
>>> Link: https://lore.kernel.org/rust-for-linux/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/ [1]
>>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>
>> Given the From: line above, this needs Lina's SoB.
>>
>> This patch was also originally posted by Abdiel and Boqun and I think we should
>> account for this. I mentioned this in a couple of previous versions already,
>> e.g. in [1]. I think we should account for this.
>>
>> [1] https://lore.kernel.org/all/cc28d048-5e0f-4f0e-b0f2-1b9e240f639b@kernel.org/
>
> I had a question about this in the cover letter.
>
> It is my understanding that the SoB needs confirmation from the author
> if the code was changed. I changed the code and did not want to bother
> the original author, because it is my understanding they do not wish to
> be contacted. I did not want to misrepresent the original author, and so
> I did not change the "From:" line.
Looks like your didn't make a very substantial change (just OwnableMut ->
Unpin)? So mentioning the change in commit message should be sufficient.
If it's very substantial I would change author to be you and add the original
author as Co-developed-by instead.
Best,
Gary
>
> I want to be clear that I want to submit this patch the correct way,
> whatever that is. I will happily take all the guidance I can get to find
> the correct way to represent the work of the original author(s).
>
> How would you prefer to account for the work by Abdiel and Boqun?
>
> Please advise about SoB and I will correct the series.
>
>
> Best regards,
> Andreas Hindborg
^ permalink raw reply
* Re: [PATCH v14 1/9] rust: types: Add Ownable/Owned types
From: Gary Guo @ 2026-02-04 16:55 UTC (permalink / raw)
To: Danilo Krummrich, Andreas Hindborg
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Asahi Lina
In-Reply-To: <DG6BWC5SOHUG.2K1ZXGYNVB69V@kernel.org>
On Wed Feb 4, 2026 at 4:46 PM GMT, Danilo Krummrich wrote:
> On Wed Feb 4, 2026 at 5:06 PM CET, Andreas Hindborg wrote:
>> It is my understanding that the SoB needs confirmation from the author
>> if the code was changed. I changed the code and did not want to bother
>> the original author, because it is my understanding they do not wish to
>> be contacted. I did not want to misrepresent the original author, and so
>> I did not change the "From:" line.
>
> Frankly, I don't know what's the correct thing to do in this case; maybe the
> correct thing is to just keep the SoB, but list all the changes that have been
> made.
>
> Technically, the same thing is common practice when maintainers (including
> myself) apply (minor) changes to a patch when applying them to their tree.
>
>> How would you prefer to account for the work by Abdiel and Boqun?
>
> I mean, I don't have a preference and if I would have one, it wouldn't be
> relevant. :) I just wanted to bring it up since the very first version was sent
> by the two of them. So, I think we should just ask.
It looks to me that they're independent works, there're very clear difference
between the two patches. Lina's patch closely follows the ARef design, and it
looks it also takes into account the dropck (by having `PhantomData`) and have
send/sync considerations.
Best,
Gary
^ permalink raw reply
* Re: [PATCH v14 1/9] rust: types: Add Ownable/Owned types
From: Danilo Krummrich @ 2026-02-04 16:46 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Asahi Lina
In-Reply-To: <87fr7gpk6d.fsf@t14s.mail-host-address-is-not-set>
On Wed Feb 4, 2026 at 5:06 PM CET, Andreas Hindborg wrote:
> It is my understanding that the SoB needs confirmation from the author
> if the code was changed. I changed the code and did not want to bother
> the original author, because it is my understanding they do not wish to
> be contacted. I did not want to misrepresent the original author, and so
> I did not change the "From:" line.
Frankly, I don't know what's the correct thing to do in this case; maybe the
correct thing is to just keep the SoB, but list all the changes that have been
made.
Technically, the same thing is common practice when maintainers (including
myself) apply (minor) changes to a patch when applying them to their tree.
> How would you prefer to account for the work by Abdiel and Boqun?
I mean, I don't have a preference and if I would have one, it wouldn't be
relevant. :) I just wanted to bring it up since the very first version was sent
by the two of them. So, I think we should just ask.
^ permalink raw reply
* Re: [PATCH v2 0/6] Landlock: Implement scope control for pathname Unix sockets
From: Justin Suess @ 2026-02-04 16:36 UTC (permalink / raw)
To: Günther Noack, Tingmao Wang, Mickaël Salaün
Cc: Günther Noack, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, linux-security-module
In-Reply-To: <aYMxHRwHCao30HfX@google.com>
On 2/4/26 06:44, Günther Noack wrote:
> On Tue, Feb 03, 2026 at 09:53:11PM +0000, Tingmao Wang wrote:
>> On 2/3/26 17:54, Günther Noack wrote:
>>> BTW, regarding the implementation: To have *OR* semantics for "within
>>> scope" and "allow-listed path", the implementation will be
>>> non-trivial, and I suspect we won't hit the merge window if we try to
>>> get them both in for 7.0. But in my mind, a simple UAPI is more
>>> important than trying to make it in time for the next merge window.
>>>
>>> (The implementation is difficult because the path-based and
>>> scope-based check currently happen in different LSM hooks, and none of
>>> the two hooks has enough information to make the decision alone. The
>>> second hook only gets called if the first returns 0. It'll require
>>> some further discussion to make it work together.)
>> Right. In that case, would it make sense to pass sk into the new
>> security_unix_find() hook, perhaps with the new argument named `struct
>> sock *other`? Then we can use this hook for the scope check as well by
>> using landlock_cred(other->sk_socket->file->f_cred)->domain etc.
Tingmao,
This seems like the best way to do it. Alternatively, I considered passing in just the
f_cred to the hook, but I'm leaning towards a more generic implementation like the one you
have below.
>>
>> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
>> index 227467236930..db9d279b3883 100644
>> --- a/net/unix/af_unix.c
>> +++ b/net/unix/af_unix.c
>> @@ -1223,24 +1223,24 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
>>
>> err = -ECONNREFUSED;
>> inode = d_backing_inode(path.dentry);
>> if (!S_ISSOCK(inode->i_mode))
>> goto path_put;
>>
>> + err = -ECONNREFUSED;
>> + sk = unix_find_socket_byinode(inode);
>> + 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, type, flags);
>> + err = security_unix_find(&path, sk, flags);
>> if (err)
>> - goto path_put;
>> -
>> - err = -ECONNREFUSED;
>> - sk = unix_find_socket_byinode(inode);
>> - if (!sk)
>> - goto path_put;
>> + goto sock_put;
>>
>> err = -EPROTOTYPE;
>> if (sk->sk_type == type)
>> touch_atime(&path);
>> else
>> goto sock_put;
>>
>> By doing this we won't even need to pass `type` separately anymore. The
>> only change would be that now one can determine if a socket is bound or
>> not even without being allowed RESOLVE_UNIX access. I'm not sure how much
>> of an issue this is, but we could also call the hook anyway with a NULL in
>> place of the new argument, if unix_find_socket_byinode() fails. Other
>> LSMs can then decide what to do in that case (either return -ECONNREFUSED
>> or -EPERM).
I think landlock already allows checking of existence of files even when when the process
doesn't have rights on them, so there is precedent for this so in my mind this would probably be OK.
> Thank you for the suggestion.
>
> Small caveat is that the LSM interface is very central and we should
> be careful. We have previously gotten the advice from Paul to design
> the hooks in an LSM-independent way that ideally reflects the
> arguments to the unix_find_bsd() function, and this would now deviate
> (slightly) from that, but simplifying the implementation for us. In
> my personal opinion, this might be worth doing the trade-off, if
> AppArmor people also agree, but we should double check.
Gunther:
I'd be happy to send you an updated LSM hook patch with the sock parameter.
Just let me know.
>
> To keep the discussion of implementation and interface separate, I
> have raised this question in the pathname-restricted-UNIX patch set
> thread in [1].
>
> [1] https://lore.kernel.org/all/aYMenaSmBkAsFowd@google.com/
>
>
>
>>>> Furthermore, an application / Landlock config etc can always opt to not
>>>> use the scope bit at all, if it "knows" all the locations where the
>>>> application's sockets would be placed, and just use RESOLVE_UNIX access
>>>> right (or nothing if it is not supported).
>>>>
>>>> (The following is a bit of a side note, not terribly relevant if we're
>>>> deciding to go with the patch as is.)
>>>>
>>>>>> [...]
>>>>>> Another way to put it is that, if FS-based and scope-based controls
>>>>>> interacts in the above proposed way, both mechanisms feel like "poking
>>>>>> holes" in the other. But as Mickaël said, one can think of the two
>>>>>> mechanisms not as independent controls, but rather as two interfaces for
>>>>>> the same control. The socket access control is "enabled" if either the
>>>>>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX access is handled, or the scope bit
>>>>>> proposed in this patch is enabled.
>>>>>>
>>>>>> With that said, I can think of some alternative ways that might make this
>>>>>> API look "better" (from a subjective point of view, feedback welcome),
>>>>>> however it does mean more delays, and specifically, these will depend on
>>>>>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX:
>>>>>>
>>>>>> One possibility is to simply always allow a Landlock domain to connect to
>>>>>> its own sockets (in the case where LANDLOCK_ACCESS_FS_RESOLVE_UNIX is
>>>>>> handled, otherwise all sockets are allowed). This might be reasonable, as
>>>>>> one can only connect to a socket it creates if it has the permission to
>>>>>> create it in the first place, which is already controlled by
>>>>>> LANDLOCK_ACCESS_FS_MAKE_SOCK, so we don't really lose any policy
>>>>>> flexibility here - if for some reason the sandboxer don't want to allow
>>>>>> access to any (pathname) sockets, even the sandboxed app's own ones, it
>>>>>> can just not allow LANDLOCK_ACCESS_FS_MAKE_SOCK anywhere.
>>>>> LANDLOCK_ACCESS_FS_MAKE_SOCK is only required to bind/listen to a
>>>>> socket, not to connect. I guess you was thinking about
>>>>> LANDLOCK_ACCESS_FS_RESOLVE_UNIX in this case?
>>>> In this "allow same-scope connect unconditionally" proposal, the
>>>> application would still be able to (bind to and) connect to its own
>>>> sockets, even if LANDLOCK_ACCESS_FS_RESOLVE_UNIX is handled and nothing is
>>>> allowed to have LANDLOCK_ACCESS_FS_RESOLVE_UNIX access. But a sandboxer
>>>> which for whatever reason doesn't want this "allow same scope" default can
>>>> still prevent the use of (pathname) sockets by restricting
>>>> LANDLOCK_ACCESS_FS_MAKE_SOCK, because if an app can't connect to any
>>>> sockets it doesn't own, and can't create any sockets itself either, then
>>>> it effectively can't connect to any sockets at all.
>>>>
>>>> (Although on second thought, I guess there could be a case where an app
>>>> first creates some socket files before doing landlock_restrict_self(),
>>>> then it might still be able to bind to these even without
>>>> LANDLOCK_ACCESS_FS_MAKE_SOCK?)
>>> FWIW, I also really liked Tingmao's first of the two listed
>>> possibilities in [1], where she proposed to introduce both rights
>>> together. In my understanding, the arguments we have discussed so far
>>> for that are:
>>>
>>> IN FAVOR:
>>>
>>> (pro1) Connecting to a UNIX socket in the same scope is always safe,
>>> and it makes it possible to use named UNIX sockets between the
>>> processes within a Landlock domains. (Mickaël convinced me in
>>> discussion at FOSDEM that this is true.)
>>>
>>> If someone absolutely does not want that, they can restrict
>>> LANDLOCK_ACCESS_FS_MAKE_SOCK and achieve the same effect (as
>>> Tingmao said above).
>>>
>>> (pro2) The implementation of this is simpler.
>>>
>>> (I attempted to understand how the "or" semantics would be
>>> implemented, and I found it non-trivial when you try to do it
>>> for all layers at once. (Kernighan's Law applies, IMHO))
>> I think the logic would basically be:
>>
>> 1. if any layers deny the access due to handled RESOLVE_UNIX but does not
>> have the scope bit set, then we will deny rightaway, without calling
>> domain_is_scoped().
>>
>> 2. Call domain_is_scoped() with a bitmask of "rules_covered" layers where
>> there are RESOLVE_UNIX rules covering the socket being accessed, and
>> essentially ignore those layers in the scope violation check.
Tingmao:
For connecting a pathname unix socket, the order of the hooks landlock sees is something like:
1. security_unix_find. (to look up the paths)
2. security_unix_may_send, security_unix_stream_connect (after the path is looked up)
Which for is called in DGRAM:
unix_dgram_connect OR unix_dgram_sendmsg
and for STREAM:
unix_stream_connect
IIRC, the path lookup only occurs in this order always, so in the logic as you have it the domain_is_scoped()
would be called twice, once from the security_unix_find when you call it in step two, and once from the
domain scope hooks. (If access was allowed from security_unix_find)
There are a couple of things to consider.
---
Audit blockers need special handling:
Here's an example:
1. Program A enforces a ruleset with RESOLVE_UNIX and the unix pathname scope bit, with no rules with that
access bit (deny all for RESOLVE_UNIX).
2. Program A connects to /tmp/mysock.sock ran by program B, which is outside the domain.
2. security_unix_find is hit to lookup the path, and the RESOLVE_UNIX code doesn't grant access to
/tmp/mysock.sock, so it calls domain_is_scoped
3. domain_is_scoped denies it as well, so now we must log an audit record.
When logging the denial, we have to include both blockers "scope.unix_socket" and "fs.resolve_unix" for the
denial, because it is the absence of both that caused the denial. I think the refer right has similar cases for auditing, so there is precedent for this (multiple blockers for an audit message).
---
Dual lookup for domain_is_scoped. Consider this case:
1. Program A enforces a ruleset with RESOLVE_UNIX and the unix pathname scope bit, with no rules with that
access bit (deny all for RESOLVE_UNIX).
2. Program A connects to Program C's /tmp/foo.sock, which for the purposes of this example is in the domain of program A.
3. security_unix_find is hit to lookup the path, and the RESOLVE_UNIX code doesn't grant access to
/tmp/mysock.sock, so it calls domain_is_scoped. Access is granted, and continues. (LSM hook complete)
4. The connection proceeds past the path lookup stage, and now security_unix_may_send, or security_unix_stream_connect gets called. This requires ANOTHER domain_is_scoped access check.
While I don't THINK this introduces a TOCTOU, it is a little confusing.
This does mean that we look up the domain twice, if this is implemented naively. I think we can then just
skip the task credential checks then for security_unix_may_send and security_unix_stream_connect **for
connecting to pathname sockets**, since the domain_is_scoped will already have been called in landlock's
security_unix_find hook, eliminating the need for handling pathname socket domain checks layer on.
>>
>> I definitely agree that it is tricky, but making same-scope access be
>> allowed (i.e. the suggested idea above) would only get rid of step 1,
>> which I think is the "simpler" bit. The extra logic in step 2 is still
>> needed.
>>
>> I definitely agree with pro1 tho.
> Yes, you are describing the logic for the variant where
> LANDLOCK_ACCESS_FS_RESOLVE_UNIX does not implicitly permit access from
> within the same scope. In that variant, there can be situations where
> the first hook can deny the action immediately.
>
> In the variant where LANDLOCK_ACCESS_FS_RESOLVE_UNIX *does* implicitly
> allow access from within the same scope, that shortcutting is not
> possible. On the upside however, there is no need to distinguish
> whether the scope flag is set when we are in the security_unix_find()
> hook, because access from within the same scope is always permitted.
> (That is the simplification I meant.)
>
>
>>> AGAINST:
>>>
>>> (con1) It would work differently than the other scoped access rights
>>> that we already have.
>>>
>>> A speculative feature that could potentially be built with the
>>> scoped access rights is that we could add a rule to permit IPC
>>> to other Landlock scopes, e.g. introducing a new rule type
>>>
>>> struct landlock_scope_attr {
>>> __u64 allowed_access; /* for "scoped" bits */
>>> /* some way to identify domains */
>>> }
>>>
>>> so that we could make IPC access to other Landlock domains
>>> configurable.
>>>
>>> If the scoped bit and the FS RESOLVE_UNIX bit were both
>>> conflated in RESOLVE_UNIX, it would not be possible to make
>>> UNIX connections configurable in such a way.
>> This exact API would no longer work, but if we give up the equivalence
>> between scope bits and the landlock_scope_attr struct, then we can do
>> something like:
>>
>> struct landlock_scope_attr {
>> __u64 ptrace:1; /* Note that this is not a (user controllable) scope bit! */
>> __u64 abstract_unix_socket:1;
>> __u64 pathname_unix_socket:1;
>> /* ... */
>>
>> __u64 allowed_signals;
>>
>> /*
>> * some way to identify domains, maybe we could use the audit domain
>> * ID, with 0 denoting "allow access to non-Landlocked processes?
>> */
>> }
> Yes, it would be possible to use such a struct for that scenario where
> IPC access gets allowed for other Landlock scopes. It would mean that
> we would not need to introduce a scoped flag for the pathname UNIX
> socket connections. But the relationship between that struct
> landlock_scope_attr and the flags and access rights in struct
> landlock_ruleset_attr would become less clear, which is a slight
> downside, and maybe error prone for users to work with.
>
> If we introduced an additional scoped flag, it would also be
> consistent though.
>
> (con1) was written under the assumption that we do not have an
> additional scoped flag. If that is lacking, it is not possible to
> express UNIX connect() access to other Landlock domains with that
> struct. But as outlined in the proposal below, if we *do* (later)
> introduce the additional scoped flag *in addition* to the FS access
> right, this *both* stays consistent in semantics with the signal and
> abstract UNIX support, *and* it starts working in a world where ICP
> access can be allowed to talk to other Landlock domains.
>
>>> (con2) Consistent behaviour between scoped flags and their
>>> interactions with other access rights:
>>>
>>> The existing scoped access rights (signal, abstract sockets)
>>> could hypothetically be extended with a related access right of
>>> another type. For instance, there could be an access right type
>>>
>>> __u64 handled_signal_number;
>>>
>>> and then you could add a rule to permit the use of certain
>>> signal numbers. The interaction between the scoped flags and
>>> other access rights should work the same.
>>>
>>>
>>> Constructive Proposal for consideration: Why not both?
>>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> I will think about the following a bit more but I'm afraid that I feel
>> like it might get slightly confusing. With this, the only reason for
>> having LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET is for API consistency when we
>> later enable allowing access to other domains (if I understood correctly),
>> in which case I personally feel like the suggestion on landlock_scope_attr
>> above, where we essentially accept that it is decoupled with the scope
>> bits in the ruleset, might be simpler...?
> Mickaël expressed the opinion to me that he would like to APIs to stay
> consistent between signals, abstract UNIX sockets, named UNIX sockets
> and other future "scoped" operations, in scenarios where:
>
> * the "scoped" (IPC) operations can be configured to give access to
> other Landlock domains (and that should work for UNIX connections too)
> * the existing "scoped" operations also start having matching access rights
>
> I think with the way I proposed, that would be consistent.
>
>
>>> Why not do both what Tingmao proposed in [1] **and** reserve the
>>> option to add the matching "scoped flag" later?
>>>
>>> * Introduce LANDLOCK_ACCESS_FS_RESOLVE_UNIX.
>>>
>>> If it is handled, UNIX connections are allowed either:
>>>
>>> (1) if the connection is to a service in the same scope, or
>>> (2) if the path was allow-listed with a "path beneath" rule.
>>>
>>> * Add LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET later, if needed.
>>>
>>>
>>> Let's go through the arguments again:
>>>
>>> We have observed that it is harmless to allow connections to services
>>> in the same scope (1), and that if users absolutely don't want that,
>>> they can actually prohibit it through LANDLOCK_ACCESS_FS_MAKE_SOCK
>>> (pro1).
>>>
>>> (con1): Can we still implement the feature idea where we poke a hole
>>> to get UNIX-connect() access to other Landlock domains?
>>>
>>> I think the answer is yes. The implementation strategy is:
>>>
>>> * Add the scoped bit LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET
>>> * The scoped bit can now be used to allow-list connections to
>>> other Landlock domains.
>>>
>>> For users, just setting the scoped bit on its own does the same as
>>> handling LANDLOCK_ACCESS_FS_RESOLVE_UNIX. That way, the kernel-side
>>> implementation can also stay simple. The only reason why the scoped
>>> bit is needed is because it makes it possible to allow-list
>>> connections to other Landlock domains, but at the same time, it is
>>> safe if libraries set the scoped bit once it exists, as it does not
>>> have any bad runtime impact either.
>>>
>>> (con2): Consistency: Do all the scoped flags interact with their
>>> corresponding access rights in the same way?
>>>
>>> The other scope flags do not have corresponding access rights, so
>>> far.
>>>
>>> If we were to add corresponding access rights for the other scope
>>> flags, I would argue that we could apply a consistent logic there,
>>> because IPC access within the same scope is always safe:
>>>
>>> - A hypothetical access right type for "signal numbers" would only
>>> restrict signals that go beyond the current scope.
>>>
>>> - A hypothetical access right type for "abstract UNIX domain socket
>>> names" would only restrict connections to abstract UNIX domain
>>> servers that go beyond the current scope.
>>>
>>> I can not come up with a scenario where this doesn't work.
Gunther / Tingmao / Mickaël:
I have a potential idea to make this concept cleaner.
The docs for landlock currently say:
IPC scoping does not support exceptions via landlock_add_rule(2).
If an operation is scoped within a domain, no rules can be added
to allow access to resources or processes outside of the scope.
So if we go with the solution where we are now saying IPC scoping DOES support exceptions
we will need to update the documentation, to say scoping for pathname unix sockets is an exception,
and have to have the "exemptible scopes" (like this one) alongside "non-exemptible" scopes
(ie the existing ones). This creates some friction for users.
If we foresee other "exempt-able scopes" (which are scopes that also support creating exemptions w/ corresponding access rights) in the future, maybe we should consider separating the two in the ruleset
attributes (I used scoped_fs as an example for the attribute name):
structlandlock_ruleset_attrruleset_attr={
.handled_access_fs=
LANDLOCK_ACCESS_FS_EXECUTE|
LANDLOCK_ACCESS_FS_WRITE_FILE|
LANDLOCK_ACCESS_FS_READ_FILE|
LANDLOCK_ACCESS_FS_READ_DIR|
LANDLOCK_ACCESS_FS_REMOVE_DIR|
LANDLOCK_ACCESS_FS_REMOVE_FILE|
LANDLOCK_ACCESS_FS_MAKE_CHAR|
LANDLOCK_ACCESS_FS_MAKE_DIR|
LANDLOCK_ACCESS_FS_MAKE_REG|
LANDLOCK_ACCESS_FS_MAKE_SOCK|
LANDLOCK_ACCESS_FS_MAKE_FIFO|
LANDLOCK_ACCESS_FS_MAKE_BLOCK|
LANDLOCK_ACCESS_FS_MAKE_SYM|
LANDLOCK_ACCESS_FS_REFER|
LANDLOCK_ACCESS_FS_TRUNCATE|
LANDLOCK_ACCESS_FS_IOCTL_DEV,
.handled_access_net=
LANDLOCK_ACCESS_NET_BIND_TCP|
LANDLOCK_ACCESS_NET_CONNECT_TCP,
.scoped=
LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET|
LANDLOCK_SCOPE_SIGNAL,
.scoped_fs =
LANDLOCK_SCOPE_FS_PATHNAME_UNIX_SOCKET
};
This more clearly distinguishes between scopes that have exceptions/corresponding fs rights,
and ones that don't. Later we could add scoped_net, if needed. I feel like this would be more
intuitive and better categorize future scoping rights. An obvious con is increasing the size of
the ruleset attributes.
Of course this separation is only worth it if there are other "exempt-able" rights in the future.
I can think of a few potential future rights which COULD be scoped and have corresponding rights
(binder, sysv-ipc, pipes, tcp/udp between two local programs).
>>>
>>>
>>> In conclusion, I think the approach has significant upsides:
>>>
>>> * Simpler UAPI: Users only have one access bit to deal with, in the
>>> near future. Once we do add a scope flag for UNIX connections, it
>>> does not interact in a surprising way with the corresponding FS
>>> access right, because with either of these, scoped access is
>>> allowed.
>>>
>>> If users absolutely need to restrict scoped access, they can
>>> restrict LANDLOCK_ACCESS_FS_MAKE_SOCK. It is a slightly obscure
>>> API, but in line with the "make easy things easy, make hard things
>>> possible" API philosophy. And needing this should be the
>>> exception rather than the norm, after all.
>>>
>>> * Consistent behaviour between scoped flags and regular access
>>> rights, also for speculative access rights affecting the existing
>>> scoped flags for signals and abstract UNIX domain sockets.
>>>
>>> [1] https://lore.kernel.org/all/f07fe41a-96c5-4d3a-9966-35b30b3a71f1@maowtm.org/
> —Günther
^ permalink raw reply
* Re: [PATCH v14 1/9] rust: types: Add Ownable/Owned types
From: Andreas Hindborg @ 2026-02-04 16:06 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Asahi Lina
In-Reply-To: <DG6AIA0QK77C.EKG7X4NBEJ00@kernel.org>
"Danilo Krummrich" <dakr@kernel.org> writes:
> On Wed Feb 4, 2026 at 12:56 PM CET, Andreas Hindborg wrote:
>> From: Asahi Lina <lina+kernel@asahilina.net>
>>
>> By analogy to `AlwaysRefCounted` and `ARef`, an `Ownable` type is a
>> (typically C FFI) type that *may* be owned by Rust, but need not be. Unlike
>> `AlwaysRefCounted`, this mechanism expects the reference to be unique
>> within Rust, and does not allow cloning.
>>
>> Conceptually, this is similar to a `KBox<T>`, except that it delegates
>> resource management to the `T` instead of using a generic allocator.
>>
>> This change is a derived work based on work by Asahi Lina
>> <lina+kernel@asahilina.net> [1] and Oliver Mangold <oliver.mangold@pm.me>.
>>
>> Link: https://lore.kernel.org/rust-for-linux/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/ [1]
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> Given the From: line above, this needs Lina's SoB.
>
> This patch was also originally posted by Abdiel and Boqun and I think we should
> account for this. I mentioned this in a couple of previous versions already,
> e.g. in [1]. I think we should account for this.
>
> [1] https://lore.kernel.org/all/cc28d048-5e0f-4f0e-b0f2-1b9e240f639b@kernel.org/
I had a question about this in the cover letter.
It is my understanding that the SoB needs confirmation from the author
if the code was changed. I changed the code and did not want to bother
the original author, because it is my understanding they do not wish to
be contacted. I did not want to misrepresent the original author, and so
I did not change the "From:" line.
I want to be clear that I want to submit this patch the correct way,
whatever that is. I will happily take all the guidance I can get to find
the correct way to represent the work of the original author(s).
How would you prefer to account for the work by Abdiel and Boqun?
Please advise about SoB and I will correct the series.
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH v14 2/9] rust: rename `AlwaysRefCounted` to `RefCounted`.
From: Danilo Krummrich @ 2026-02-04 15:44 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Oliver Mangold
In-Reply-To: <20260204-unique-ref-v14-2-17cb29ebacbb@kernel.org>
On Wed Feb 4, 2026 at 12:56 PM CET, Andreas Hindborg wrote:
> rust/kernel/auxiliary.rs | 7 +++++-
> rust/kernel/device.rs | 10 ++++++---
> rust/kernel/device/property.rs | 7 +++++-
> rust/kernel/drm/device.rs | 10 ++++++---
> rust/kernel/drm/gem/mod.rs | 8 ++++---
> rust/kernel/pci.rs | 10 ++++++++-
> rust/kernel/platform.rs | 7 +++++-
If you apply the kernel import style, those are
Acked-by: Danilo Krummrich <dakr@kernel.org>
However, I wonder if we want aref::* in the prelude instead.
^ permalink raw reply
* Re: [PATCH v14 1/9] rust: types: Add Ownable/Owned types
From: Danilo Krummrich @ 2026-02-04 15:41 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Asahi Lina
In-Reply-To: <20260204-unique-ref-v14-1-17cb29ebacbb@kernel.org>
On Wed Feb 4, 2026 at 12:56 PM CET, Andreas Hindborg wrote:
> From: Asahi Lina <lina+kernel@asahilina.net>
>
> By analogy to `AlwaysRefCounted` and `ARef`, an `Ownable` type is a
> (typically C FFI) type that *may* be owned by Rust, but need not be. Unlike
> `AlwaysRefCounted`, this mechanism expects the reference to be unique
> within Rust, and does not allow cloning.
>
> Conceptually, this is similar to a `KBox<T>`, except that it delegates
> resource management to the `T` instead of using a generic allocator.
>
> This change is a derived work based on work by Asahi Lina
> <lina+kernel@asahilina.net> [1] and Oliver Mangold <oliver.mangold@pm.me>.
>
> Link: https://lore.kernel.org/rust-for-linux/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/ [1]
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
Given the From: line above, this needs Lina's SoB.
This patch was also originally posted by Abdiel and Boqun and I think we should
account for this. I mentioned this in a couple of previous versions already,
e.g. in [1]. I think we should account for this.
[1] https://lore.kernel.org/all/cc28d048-5e0f-4f0e-b0f2-1b9e240f639b@kernel.org/
^ permalink raw reply
* Re: [PATCH v14 2/9] rust: rename `AlwaysRefCounted` to `RefCounted`.
From: Igor Korotin @ 2026-02-04 15:08 UTC (permalink / raw)
To: Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman, Ira Weiny,
Leon Romanovsky, Paul Moore, Serge Hallyn, Rafael J. Wysocki,
David Airlie, Simona Vetter, Alexander Viro, Christian Brauner,
Jan Kara, Igor Korotin, Daniel Almeida, Lorenzo Stoakes,
Liam R. Howlett, Viresh Kumar, Nishanth Menon, Stephen Boyd,
Bjorn Helgaas, Krzysztof Wilczyński
Cc: linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Oliver Mangold
In-Reply-To: <20260204-unique-ref-v14-2-17cb29ebacbb@kernel.org>
Hello Andreas
On 2/4/2026 11:56 AM, Andreas Hindborg wrote:
> From: Oliver Mangold <oliver.mangold@pm.me>
>
> There are types where it may both be reference counted in some cases and
> owned in others. In such cases, obtaining `ARef<T>` from `&T` would be
> unsound as it allows creation of `ARef<T>` copy from `&Owned<T>`.
>
> Therefore, we split `AlwaysRefCounted` into `RefCounted` (which `ARef<T>`
> would require) and a marker trait to indicate that the type is always
> reference counted (and not `Ownable`) so the `&T` -> `ARef<T>` conversion
> is possible.
>
> - Rename `AlwaysRefCounted` to `RefCounted`.
> - Add a new unsafe trait `AlwaysRefCounted`.
> - Implement the new trait `AlwaysRefCounted` for the newly renamed
> `RefCounted` implementations. This leaves functionality of existing
> implementers of `AlwaysRefCounted` intact.
>
> Original patch by Oliver Mangold <oliver.mangold@pm.me> [1].
>
> Link: https://lore.kernel.org/r/20251117-unique-ref-v13-2-b5b243df1250@pm.me [1]
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/auxiliary.rs | 7 +++++-
> rust/kernel/block/mq/request.rs | 15 +++++++------
> rust/kernel/cred.rs | 13 ++++++++++--
> rust/kernel/device.rs | 10 ++++++---
> rust/kernel/device/property.rs | 7 +++++-
> rust/kernel/drm/device.rs | 10 ++++++---
> rust/kernel/drm/gem/mod.rs | 8 ++++---
> rust/kernel/fs/file.rs | 16 ++++++++++----
> rust/kernel/i2c.rs | 16 +++++++++-----
> rust/kernel/mm.rs | 15 +++++++++----
> rust/kernel/mm/mmput_async.rs | 9 ++++++--
> rust/kernel/opp.rs | 10 ++++++---
> rust/kernel/owned.rs | 2 +-
> rust/kernel/pci.rs | 10 ++++++++-
> rust/kernel/pid_namespace.rs | 12 +++++++++--
> rust/kernel/platform.rs | 7 +++++-
> rust/kernel/sync/aref.rs | 47 ++++++++++++++++++++++++++---------------
> rust/kernel/task.rs | 10 ++++++---
> rust/kernel/types.rs | 3 ++-
> 19 files changed, 164 insertions(+), 63 deletions(-)
>
> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
> index be76f11aecb7e..c410dcfc7b6f7 100644
> --- a/rust/kernel/auxiliary.rs
> +++ b/rust/kernel/auxiliary.rs
> @@ -11,6 +11,7 @@
> driver,
> error::{from_result, to_result, Result},
> prelude::*,
> + sync::aref::{AlwaysRefCounted, RefCounted},
> types::Opaque,
> ThisModule,
> };
> @@ -283,7 +284,7 @@ unsafe impl<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for Device<Ctx>
> kernel::impl_device_context_into_aref!(Device);
>
> // SAFETY: Instances of `Device` are always reference-counted.
> -unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
> +unsafe impl RefCounted for Device {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::get_device(self.as_ref().as_raw()) };
> @@ -302,6 +303,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
> +// `&Device`.
> +unsafe impl AlwaysRefCounted for Device {}
> +
> impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
> fn as_ref(&self) -> &device::Device<Ctx> {
> // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
> index ce3e30c81cb5e..cf013b9e2cacf 100644
> --- a/rust/kernel/block/mq/request.rs
> +++ b/rust/kernel/block/mq/request.rs
> @@ -9,7 +9,7 @@
> block::mq::Operations,
> error::Result,
> sync::{
> - aref::{ARef, AlwaysRefCounted},
> + aref::{ARef, AlwaysRefCounted, RefCounted},
> atomic::Relaxed,
> Refcount,
> },
> @@ -229,11 +229,10 @@ unsafe impl<T: Operations> Send for Request<T> {}
> // mutate `self` are internally synchronized`
> unsafe impl<T: Operations> Sync for Request<T> {}
>
> -// SAFETY: All instances of `Request<T>` are reference counted. This
> -// implementation of `AlwaysRefCounted` ensure that increments to the ref count
> -// keeps the object alive in memory at least until a matching reference count
> -// decrement is executed.
> -unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {
> +// SAFETY: All instances of `Request<T>` are reference counted. This implementation of `RefCounted`
> +// ensure that increments to the ref count keeps the object alive in memory at least until a
> +// matching reference count decrement is executed.
> +unsafe impl<T: Operations> RefCounted for Request<T> {
> fn inc_ref(&self) {
> self.wrapper_ref().refcount().inc();
> }
> @@ -255,3 +254,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
> }
> }
> }
> +
> +// SAFETY: We currently do not implement `Ownable`, thus it is okay to obtain an `ARef<Request>`
> +// from a `&Request` (but this will change in the future).
> +unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {}
> diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs
> index ffa156b9df377..20ef0144094be 100644
> --- a/rust/kernel/cred.rs
> +++ b/rust/kernel/cred.rs
> @@ -8,7 +8,12 @@
> //!
> //! Reference: <https://www.kernel.org/doc/html/latest/security/credentials.html>
>
> -use crate::{bindings, sync::aref::AlwaysRefCounted, task::Kuid, types::Opaque};
> +use crate::{
> + bindings,
> + sync::aref::RefCounted,
> + task::Kuid,
> + types::{AlwaysRefCounted, Opaque},
> +};
>
> /// Wraps the kernel's `struct cred`.
> ///
> @@ -76,7 +81,7 @@ pub fn euid(&self) -> Kuid {
> }
>
> // SAFETY: The type invariants guarantee that `Credential` is always ref-counted.
> -unsafe impl AlwaysRefCounted for Credential {
> +unsafe impl RefCounted for Credential {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> @@ -90,3 +95,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Credential>) {
> unsafe { bindings::put_cred(obj.cast().as_ptr()) };
> }
> }
> +
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Credential>` from a
> +// `&Credential`.
> +unsafe impl AlwaysRefCounted for Credential {}
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 031720bf5d8ca..e09dad5f9afea 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -7,8 +7,8 @@
> use crate::{
> bindings, fmt,
> prelude::*,
> - sync::aref::ARef,
> - types::{ForeignOwnable, Opaque},
> + sync::aref::{ARef, RefCounted},
> + types::{AlwaysRefCounted, ForeignOwnable, Opaque},
> };
> use core::{any::TypeId, marker::PhantomData, ptr};
>
> @@ -492,7 +492,7 @@ pub fn fwnode(&self) -> Option<&property::FwNode> {
> kernel::impl_device_context_into_aref!(Device);
>
> // SAFETY: Instances of `Device` are always reference-counted.
> -unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
> +unsafe impl RefCounted for Device {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::get_device(self.as_raw()) };
> @@ -504,6 +504,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
> +// `&Device`.
> +unsafe impl AlwaysRefCounted for Device {}
> +
> // SAFETY: As by the type invariant `Device` can be sent to any thread.
> unsafe impl Send for Device {}
>
> diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs
> index 3a332a8c53a9e..a8bb824ad0ec1 100644
> --- a/rust/kernel/device/property.rs
> +++ b/rust/kernel/device/property.rs
> @@ -14,6 +14,7 @@
> fmt,
> prelude::*,
> str::{CStr, CString},
> + sync::aref::{AlwaysRefCounted, RefCounted},
> types::{ARef, Opaque},
> };
>
> @@ -359,7 +360,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> }
>
> // SAFETY: Instances of `FwNode` are always reference-counted.
> -unsafe impl crate::types::AlwaysRefCounted for FwNode {
> +unsafe impl RefCounted for FwNode {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the
> // refcount is non-zero.
> @@ -373,6 +374,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<FwNode>` from a
> +// `&FwNode`.
> +unsafe impl AlwaysRefCounted for FwNode {}
> +
> enum Node<'a> {
> Borrowed(&'a FwNode),
> Owned(ARef<FwNode>),
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index 3ce8f62a00569..38ce7f389ed00 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -11,8 +11,8 @@
> error::from_err_ptr,
> error::Result,
> prelude::*,
> - sync::aref::{ARef, AlwaysRefCounted},
> - types::Opaque,
> + sync::aref::{AlwaysRefCounted, RefCounted},
> + types::{ARef, Opaque},
> };
> use core::{alloc::Layout, mem, ops::Deref, ptr, ptr::NonNull};
>
> @@ -198,7 +198,7 @@ fn deref(&self) -> &Self::Target {
>
> // SAFETY: DRM device objects are always reference counted and the get/put functions
> // satisfy the requirements.
> -unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {
> +unsafe impl<T: drm::Driver> RefCounted for Device<T> {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::drm_dev_get(self.as_raw()) };
> @@ -213,6 +213,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
> +// `&Device`.
> +unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {}
> +
> impl<T: drm::Driver> AsRef<device::Device> for Device<T> {
> fn as_ref(&self) -> &device::Device {
> // SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid,
> diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
> index a7f682e95c018..ad6840a440165 100644
> --- a/rust/kernel/drm/gem/mod.rs
> +++ b/rust/kernel/drm/gem/mod.rs
> @@ -10,8 +10,7 @@
> drm::driver::{AllocImpl, AllocOps},
> error::{to_result, Result},
> prelude::*,
> - sync::aref::{ARef, AlwaysRefCounted},
> - types::Opaque,
> + types::{ARef, AlwaysRefCounted, Opaque},
> };
> use core::{ops::Deref, ptr::NonNull};
>
> @@ -253,7 +252,7 @@ extern "C" fn free_callback(obj: *mut bindings::drm_gem_object) {
> }
>
> // SAFETY: Instances of `Object<T>` are always reference-counted.
> -unsafe impl<T: DriverObject> crate::types::AlwaysRefCounted for Object<T> {
> +unsafe impl<T: DriverObject> crate::types::RefCounted for Object<T> {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::drm_gem_object_get(self.as_raw()) };
> @@ -267,6 +266,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> unsafe { bindings::drm_gem_object_put(obj.as_raw()) }
> }
> }
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
> +// `&Object`.
> +unsafe impl<T: DriverObject> crate::types::AlwaysRefCounted for Object<T> {}
>
> impl<T: DriverObject> super::private::Sealed for Object<T> {}
>
> diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
> index 23ee689bd2400..06e457d62a939 100644
> --- a/rust/kernel/fs/file.rs
> +++ b/rust/kernel/fs/file.rs
> @@ -12,8 +12,8 @@
> cred::Credential,
> error::{code::*, to_result, Error, Result},
> fmt,
> - sync::aref::{ARef, AlwaysRefCounted},
> - types::{NotThreadSafe, Opaque},
> + sync::aref::RefCounted,
> + types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
> };
> use core::ptr;
>
> @@ -197,7 +197,7 @@ unsafe impl Sync for File {}
>
> // SAFETY: The type invariants guarantee that `File` is always ref-counted. This implementation
> // makes `ARef<File>` own a normal refcount.
> -unsafe impl AlwaysRefCounted for File {
> +unsafe impl RefCounted for File {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> @@ -212,6 +212,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<File>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<File>` from a
> +// `&File`.
> +unsafe impl AlwaysRefCounted for File {}
> +
> /// Wraps the kernel's `struct file`. Not thread safe.
> ///
> /// This type represents a file that is not known to be safe to transfer across thread boundaries.
> @@ -233,7 +237,7 @@ pub struct LocalFile {
>
> // SAFETY: The type invariants guarantee that `LocalFile` is always ref-counted. This implementation
> // makes `ARef<LocalFile>` own a normal refcount.
> -unsafe impl AlwaysRefCounted for LocalFile {
> +unsafe impl RefCounted for LocalFile {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> @@ -249,6 +253,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<LocalFile>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<LocalFile>` from a
> +// `&LocalFile`.
> +unsafe impl AlwaysRefCounted for LocalFile {}
> +
> impl LocalFile {
> /// Constructs a new `struct file` wrapper from a file descriptor.
> ///
> diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
> index 39b0a9a207fda..b5e3c236a5c16 100644
> --- a/rust/kernel/i2c.rs
> +++ b/rust/kernel/i2c.rs
> @@ -17,8 +17,10 @@
> of,
> prelude::*,
> types::{
> + ARef,
> AlwaysRefCounted,
> - Opaque, //
> + Opaque,
> + RefCounted, //
> }, //
> };
>
> @@ -31,8 +33,6 @@
> }, //
> };
>
> -use kernel::types::ARef;
> -
> /// An I2C device id table.
> #[repr(transparent)]
> #[derive(Clone, Copy)]
> @@ -416,7 +416,7 @@ pub fn get(index: i32) -> Result<ARef<Self>> {
> kernel::impl_device_context_into_aref!(I2cAdapter);
>
> // SAFETY: Instances of `I2cAdapter` are always reference-counted.
> -unsafe impl crate::types::AlwaysRefCounted for I2cAdapter {
> +unsafe impl crate::types::RefCounted for I2cAdapter {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::i2c_get_adapter(self.index()) };
> @@ -427,6 +427,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> unsafe { bindings::i2c_put_adapter(obj.as_ref().as_raw()) }
> }
> }
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from an
> +// `&I2cAdapter`.
> +unsafe impl AlwaysRefCounted for I2cAdapter {}
>
> /// The i2c board info representation
> ///
> @@ -492,7 +495,7 @@ unsafe impl<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for I2cClient<C
> kernel::impl_device_context_into_aref!(I2cClient);
>
> // SAFETY: Instances of `I2cClient` are always reference-counted.
> -unsafe impl AlwaysRefCounted for I2cClient {
> +unsafe impl RefCounted for I2cClient {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::get_device(self.as_ref().as_raw()) };
> @@ -503,6 +506,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> unsafe { bindings::put_device(&raw mut (*obj.as_ref().as_raw()).dev) }
> }
> }
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from an
> +// `&I2cClient`.
> +unsafe impl AlwaysRefCounted for I2cClient {}
>
> impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for I2cClient<Ctx> {
> fn as_ref(&self) -> &device::Device<Ctx> {
> diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs
> index 4764d7b68f2a7..dd9e3969e7206 100644
> --- a/rust/kernel/mm.rs
> +++ b/rust/kernel/mm.rs
> @@ -13,8 +13,8 @@
>
> use crate::{
> bindings,
> - sync::aref::{ARef, AlwaysRefCounted},
> - types::{NotThreadSafe, Opaque},
> + sync::aref::RefCounted,
> + types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
> };
> use core::{ops::Deref, ptr::NonNull};
>
> @@ -55,7 +55,7 @@ unsafe impl Send for Mm {}
> unsafe impl Sync for Mm {}
>
> // SAFETY: By the type invariants, this type is always refcounted.
> -unsafe impl AlwaysRefCounted for Mm {
> +unsafe impl RefCounted for Mm {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The pointer is valid since self is a reference.
> @@ -69,6 +69,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Mm>` from a `&Mm`.
> +unsafe impl AlwaysRefCounted for Mm {}
> +
> /// A wrapper for the kernel's `struct mm_struct`.
> ///
> /// This type is like [`Mm`], but with non-zero `mm_users`. It can only be used when `mm_users` can
> @@ -91,7 +94,7 @@ unsafe impl Send for MmWithUser {}
> unsafe impl Sync for MmWithUser {}
>
> // SAFETY: By the type invariants, this type is always refcounted.
> -unsafe impl AlwaysRefCounted for MmWithUser {
> +unsafe impl RefCounted for MmWithUser {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The pointer is valid since self is a reference.
> @@ -105,6 +108,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<MmWithUser>` from a
> +// `&MmWithUser`.
> +unsafe impl AlwaysRefCounted for MmWithUser {}
> +
> // Make all `Mm` methods available on `MmWithUser`.
> impl Deref for MmWithUser {
> type Target = Mm;
> diff --git a/rust/kernel/mm/mmput_async.rs b/rust/kernel/mm/mmput_async.rs
> index b8d2f051225c7..aba4ce675c860 100644
> --- a/rust/kernel/mm/mmput_async.rs
> +++ b/rust/kernel/mm/mmput_async.rs
> @@ -10,7 +10,8 @@
> use crate::{
> bindings,
> mm::MmWithUser,
> - sync::aref::{ARef, AlwaysRefCounted},
> + sync::aref::RefCounted,
> + types::{ARef, AlwaysRefCounted},
> };
> use core::{ops::Deref, ptr::NonNull};
>
> @@ -34,7 +35,7 @@ unsafe impl Send for MmWithUserAsync {}
> unsafe impl Sync for MmWithUserAsync {}
>
> // SAFETY: By the type invariants, this type is always refcounted.
> -unsafe impl AlwaysRefCounted for MmWithUserAsync {
> +unsafe impl RefCounted for MmWithUserAsync {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The pointer is valid since self is a reference.
> @@ -48,6 +49,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<MmWithUserAsync>`
> +// from a `&MmWithUserAsync`.
> +unsafe impl AlwaysRefCounted for MmWithUserAsync {}
> +
> // Make all `MmWithUser` methods available on `MmWithUserAsync`.
> impl Deref for MmWithUserAsync {
> type Target = MmWithUser;
> diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
> index a760fac287655..06fe2ca776a4f 100644
> --- a/rust/kernel/opp.rs
> +++ b/rust/kernel/opp.rs
> @@ -16,8 +16,8 @@
> ffi::{c_char, c_ulong},
> prelude::*,
> str::CString,
> - sync::aref::{ARef, AlwaysRefCounted},
> - types::Opaque,
> + sync::aref::RefCounted,
> + types::{ARef, AlwaysRefCounted, Opaque},
> };
>
> #[cfg(CONFIG_CPU_FREQ)]
> @@ -1041,7 +1041,7 @@ unsafe impl Send for OPP {}
> unsafe impl Sync for OPP {}
>
> /// SAFETY: The type invariants guarantee that [`OPP`] is always refcounted.
> -unsafe impl AlwaysRefCounted for OPP {
> +unsafe impl RefCounted for OPP {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> unsafe { bindings::dev_pm_opp_get(self.0.get()) };
> @@ -1053,6 +1053,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<OPP>` from an
> +// `&OPP`.
> +unsafe impl AlwaysRefCounted for OPP {}
> +
> impl OPP {
> /// Creates an owned reference to a [`OPP`] from a valid pointer.
> ///
> diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
> index fe30580331df9..b02edda11fcf6 100644
> --- a/rust/kernel/owned.rs
> +++ b/rust/kernel/owned.rs
> @@ -25,7 +25,7 @@
> ///
> /// Note: The underlying object is not required to provide internal reference counting, because it
> /// represents a unique, owned reference. If reference counting (on the Rust side) is required,
> -/// [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented.
> +/// [`RefCounted`](crate::types::RefCounted) should be implemented.
> ///
> /// # Safety
> ///
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index bea76ca9c3da5..9ee8f2bc6db9e 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -19,6 +19,10 @@
> },
> prelude::*,
> str::CStr,
> + sync::aref::{
> + AlwaysRefCounted,
> + RefCounted, //
> + },
> types::Opaque,
> ThisModule, //
> };
> @@ -467,7 +471,7 @@ unsafe impl<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for Device<Ctx>
> impl crate::dma::Device for Device<device::Core> {}
>
> // SAFETY: Instances of `Device` are always reference-counted.
> -unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
> +unsafe impl RefCounted for Device {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::pci_dev_get(self.as_raw()) };
> @@ -479,6 +483,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
> +// `&Device`.
> +unsafe impl AlwaysRefCounted for Device {}
> +
> impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
> fn as_ref(&self) -> &device::Device<Ctx> {
> // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
> diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
> index 979a9718f153d..4f6a94540e33d 100644
> --- a/rust/kernel/pid_namespace.rs
> +++ b/rust/kernel/pid_namespace.rs
> @@ -7,7 +7,11 @@
> //! C header: [`include/linux/pid_namespace.h`](srctree/include/linux/pid_namespace.h) and
> //! [`include/linux/pid.h`](srctree/include/linux/pid.h)
>
> -use crate::{bindings, sync::aref::AlwaysRefCounted, types::Opaque};
> +use crate::{
> + bindings,
> + sync::aref::RefCounted,
> + types::{AlwaysRefCounted, Opaque},
> +};
> use core::ptr;
>
> /// Wraps the kernel's `struct pid_namespace`. Thread safe.
> @@ -41,7 +45,7 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::pid_namespace) -> &'a Self {
> }
>
> // SAFETY: Instances of `PidNamespace` are always reference-counted.
> -unsafe impl AlwaysRefCounted for PidNamespace {
> +unsafe impl RefCounted for PidNamespace {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> @@ -55,6 +59,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<PidNamespace>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<PidNamespace>` from
> +// a `&PidNamespace`.
> +unsafe impl AlwaysRefCounted for PidNamespace {}
> +
> // SAFETY:
> // - `PidNamespace::dec_ref` can be called from any thread.
> // - It is okay to send ownership of `PidNamespace` across thread boundaries.
> diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
> index 35a5813ffb33f..139517c21961e 100644
> --- a/rust/kernel/platform.rs
> +++ b/rust/kernel/platform.rs
> @@ -13,6 +13,7 @@
> irq::{self, IrqRequest},
> of,
> prelude::*,
> + sync::aref::{AlwaysRefCounted, RefCounted},
> types::Opaque,
> ThisModule,
> };
> @@ -490,7 +491,7 @@ pub fn optional_irq_by_name(&self, name: &CStr) -> Result<IrqRequest<'_>> {
> impl crate::dma::Device for Device<device::Core> {}
>
> // SAFETY: Instances of `Device` are always reference-counted.
> -unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
> +unsafe impl RefCounted for Device {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::get_device(self.as_ref().as_raw()) };
> @@ -502,6 +503,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
> +// `&Device`.
> +unsafe impl AlwaysRefCounted for Device {}
> +
> impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
> fn as_ref(&self) -> &device::Device<Ctx> {
> // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
> diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
> index e175aefe86151..61caddfd89619 100644
> --- a/rust/kernel/sync/aref.rs
> +++ b/rust/kernel/sync/aref.rs
> @@ -19,11 +19,9 @@
>
> use core::{marker::PhantomData, mem::ManuallyDrop, ops::Deref, ptr::NonNull};
>
> -/// Types that are _always_ reference counted.
> +/// Types that are internally reference counted.
> ///
> /// It allows such types to define their own custom ref increment and decrement functions.
> -/// Additionally, it allows users to convert from a shared reference `&T` to an owned reference
> -/// [`ARef<T>`].
> ///
> /// This is usually implemented by wrappers to existing structures on the C side of the code. For
> /// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted
> @@ -40,9 +38,8 @@
> /// at least until matching decrements are performed.
> ///
> /// Implementers must also ensure that all instances are reference-counted. (Otherwise they
> -/// won't be able to honour the requirement that [`AlwaysRefCounted::inc_ref`] keep the object
> -/// alive.)
> -pub unsafe trait AlwaysRefCounted {
> +/// won't be able to honour the requirement that [`RefCounted::inc_ref`] keep the object alive.)
> +pub unsafe trait RefCounted {
> /// Increments the reference count on the object.
> fn inc_ref(&self);
>
> @@ -55,11 +52,27 @@ pub unsafe trait AlwaysRefCounted {
> /// Callers must ensure that there was a previous matching increment to the reference count,
> /// and that the object is no longer used after its reference count is decremented (as it may
> /// result in the object being freed), unless the caller owns another increment on the refcount
> - /// (e.g., it calls [`AlwaysRefCounted::inc_ref`] twice, then calls
> - /// [`AlwaysRefCounted::dec_ref`] once).
> + /// (e.g., it calls [`RefCounted::inc_ref`] twice, then calls [`RefCounted::dec_ref`] once).
> unsafe fn dec_ref(obj: NonNull<Self>);
> }
>
> +/// Always reference-counted type.
> +///
> +/// It allows deriving a counted reference [`ARef<T>`] from a `&T`.
> +///
> +/// This provides some convenience, but it allows "escaping" borrow checks on `&T`. As it
> +/// complicates attempts to ensure that a reference to T is unique, it is optional to provide for
> +/// [`RefCounted`] types. See *Safety* below.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that no safety invariants are violated by upgrading an `&T` to an
> +/// [`ARef<T>`]. In particular that implies [`AlwaysRefCounted`] and [`crate::types::Ownable`]
> +/// cannot be implemented for the same type, as this would allow violating the uniqueness guarantee
> +/// of [`crate::types::Owned<T>`] by dereferencing it into an `&T` and obtaining an [`ARef`] from
> +/// that.
> +pub unsafe trait AlwaysRefCounted: RefCounted {}
> +
> /// An owned reference to an always-reference-counted object.
> ///
> /// The object's reference count is automatically decremented when an instance of [`ARef`] is
> @@ -70,7 +83,7 @@ pub unsafe trait AlwaysRefCounted {
> ///
> /// The pointer stored in `ptr` is non-null and valid for the lifetime of the [`ARef`] instance. In
> /// particular, the [`ARef`] instance owns an increment on the underlying object's reference count.
> -pub struct ARef<T: AlwaysRefCounted> {
> +pub struct ARef<T: RefCounted> {
> ptr: NonNull<T>,
> _p: PhantomData<T>,
> }
> @@ -79,16 +92,16 @@ pub struct ARef<T: AlwaysRefCounted> {
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `ARef<T>` may ultimately access `T` using a
> // mutable reference, for example, when the reference count reaches zero and `T` is dropped.
> -unsafe impl<T: AlwaysRefCounted + Sync + Send> Send for ARef<T> {}
> +unsafe impl<T: RefCounted + Sync + Send> Send for ARef<T> {}
>
> // SAFETY: It is safe to send `&ARef<T>` to another thread when the underlying `T` is `Sync`
> // because it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally,
> // it needs `T` to be `Send` because any thread that has a `&ARef<T>` may clone it and get an
> // `ARef<T>` on that thread, so the thread may ultimately access `T` using a mutable reference, for
> // example, when the reference count reaches zero and `T` is dropped.
> -unsafe impl<T: AlwaysRefCounted + Sync + Send> Sync for ARef<T> {}
> +unsafe impl<T: RefCounted + Sync + Send> Sync for ARef<T> {}
>
> -impl<T: AlwaysRefCounted> ARef<T> {
> +impl<T: RefCounted> ARef<T> {
> /// Creates a new instance of [`ARef`].
> ///
> /// It takes over an increment of the reference count on the underlying object.
> @@ -117,12 +130,12 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
> ///
> /// ```
> /// use core::ptr::NonNull;
> - /// use kernel::sync::aref::{ARef, AlwaysRefCounted};
> + /// use kernel::sync::aref::{ARef, RefCounted};
> ///
> /// struct Empty {}
> ///
> /// # // SAFETY: TODO.
> - /// unsafe impl AlwaysRefCounted for Empty {
> + /// unsafe impl RefCounted for Empty {
> /// fn inc_ref(&self) {}
> /// unsafe fn dec_ref(_obj: NonNull<Self>) {}
> /// }
> @@ -140,7 +153,7 @@ pub fn into_raw(me: Self) -> NonNull<T> {
> }
> }
>
> -impl<T: AlwaysRefCounted> Clone for ARef<T> {
> +impl<T: RefCounted> Clone for ARef<T> {
> fn clone(&self) -> Self {
> self.inc_ref();
> // SAFETY: We just incremented the refcount above.
> @@ -148,7 +161,7 @@ fn clone(&self) -> Self {
> }
> }
>
> -impl<T: AlwaysRefCounted> Deref for ARef<T> {
> +impl<T: RefCounted> Deref for ARef<T> {
> type Target = T;
>
> fn deref(&self) -> &Self::Target {
> @@ -165,7 +178,7 @@ fn from(b: &T) -> Self {
> }
> }
>
> -impl<T: AlwaysRefCounted> Drop for ARef<T> {
> +impl<T: RefCounted> Drop for ARef<T> {
> fn drop(&mut self) {
> // SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to
> // decrement.
> diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
> index 49fad6de06740..0a6e38d984560 100644
> --- a/rust/kernel/task.rs
> +++ b/rust/kernel/task.rs
> @@ -9,8 +9,8 @@
> ffi::{c_int, c_long, c_uint},
> mm::MmWithUser,
> pid_namespace::PidNamespace,
> - sync::aref::ARef,
> - types::{NotThreadSafe, Opaque},
> + sync::aref::{AlwaysRefCounted, RefCounted},
> + types::{ARef, NotThreadSafe, Opaque},
> };
> use core::{
> cmp::{Eq, PartialEq},
> @@ -348,7 +348,7 @@ pub fn active_pid_ns(&self) -> Option<&PidNamespace> {
> }
>
> // SAFETY: The type invariants guarantee that `Task` is always refcounted.
> -unsafe impl crate::sync::aref::AlwaysRefCounted for Task {
> +unsafe impl RefCounted for Task {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> @@ -362,6 +362,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Task>` from a
> +// `&Task`.
> +unsafe impl AlwaysRefCounted for Task {}
> +
> impl Kuid {
> /// Get the current euid.
> #[inline]
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 4aec7b699269a..9b96aa2ebdb7e 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -18,7 +18,8 @@
> },
> sync::aref::{
> ARef,
> - AlwaysRefCounted, //
> + AlwaysRefCounted,
> + RefCounted, //
> }, //
> };
>
>
For Rust I2C subsystem:
Acked-by: Igor Korotin <igor.korotin.linux@gmail.com>
Thanks
Igor
^ permalink raw reply
* Re: [PATCH] xfrm: kill xfrm_dev_{state,policy}_flush_secctx_check()
From: Tetsuo Handa @ 2026-02-04 13:57 UTC (permalink / raw)
To: Paul Moore, SELinux, linux-security-module
Cc: Steffen Klassert, Herbert Xu, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Network Development
In-Reply-To: <74a70504-8ff8-4d97-b35f-774364779889@I-love.SAKURA.ne.jp>
On 2026/02/04 19:15, Tetsuo Handa wrote:
> On 2026/02/04 7:40, Paul Moore wrote:
>> This is not an unusual request for such a proposed change, and
>> is something that I would expect a LSM maintainer to do without much
>> hesitation. If you are unwilling to investigate this, can you explain
>> why?
>
> Because I'm not familiar with how XFRM works; I'm not a user of LSM XFRM hooks.
>
> I can't judge whether the current code is COMPREHENSIVELY GATING;
> I can't imagine what the state you call COMPREHENSIVELY GATING is.
Steffen Klassert worried that killing xfrm_dev_state_flush_secctx_check() and
xfrm_dev_policy_flush_secctx_check() might violate a LSM policy and you agreed
( https://lkml.kernel.org/r/CAHC9VhQ54LRD7k_x6tUju2kPVBEHcdgBh46_hBN8btG0vhfy_w@mail.gmail.com ),
but the reality is that nobody in the world has enforced an LSM policy for almost 9 years
that makes xfrm_dev_{state,policy}_flush() no-op. That is, xfrm_dev_state_flush_secctx_check()
and xfrm_dev_policy_flush_secctx_check() had been effectively unused.
Killing xfrm_dev_state_flush_secctx_check() and xfrm_dev_policy_flush_secctx_check()
increases "system's stability" without sacrificing "authorization".
It is up to SELinux developers to discuss what actions to take as a compensation for
killing xfrm_dev_state_flush_secctx_check() and xfrm_dev_policy_flush_secctx_check().
The compensation might be to add LSM hooks to immediately before the point of no return.
But I wonder why you want to authorize deleting resources which are going to be "deleted by
cascade" due to deleting a dependent resource...
>
>
>
> P.S. For your investigation, I attach a new report that syzbot found today, and
> I'll drop "xfrm: always fail xfrm_dev_{state,policy}_flush_secctx_check()"
> because these three reports will be sufficient for people to understand that
> we need to kill xfrm_dev_{state,policy}_flush_secctx_check() calls.
^ 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