* [PATCH v7 00/11] landlock: UNIX connect() control by pathname and scope
From: Günther Noack @ 2026-03-23 16:56 UTC (permalink / raw)
To: Mickaël Salaün, John Johansen, Paul Moore, James Morris,
Serge E . Hallyn
Cc: Günther Noack, linux-security-module, Tingmao Wang,
Justin Suess, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Jann Horn, Tahera Fahimi, Sebastian Andrzej Siewior,
Kuniyuki Iwashima, Simon Horman, netdev, Alexander Viro,
Christian Brauner
Hello!
This patch set introduces a filesystem-based Landlock restriction
mechanism for connecting to UNIX domain sockets (or addressing them
with sendmsg(2)). It introduces the filesystem access right
LANDLOCK_ACCESS_FS_RESOLVE_UNIX.
For the connection-oriented SOCK_STREAM and SOCK_SEQPACKET type
sockets, the access right makes the connect(2) operation fail with
EACCES, if denied.
SOCK_DGRAM-type UNIX sockets can be used both with connect(2), or by
passing an explicit recipient address with every sendmsg(2)
invocation. In the latter case, the Landlock check is done when an
explicit recipient address is passed to sendmsg(2) and can make
sendmsg(2) return EACCES. When UNIX datagram sockets are connected
with connect(2), a fixed recipient address is associated with the
socket and the check happens during connect(2) and may return EACCES.
When LANDLOCK_ACCESS_FS_RESOLVE_UNIX is handled within a Landlock
domain, this domain will only allow connect(2) and sendmsg(2) to
server sockets that were created within the same domain. Or, to
phrase it the other way around: Unless it is allow-listed with a
LANDLOCK_PATH_BENEATH rule, the newly created domain denies connect(2)
and sendmsg(2) actions that are directed *outwards* of that domain.
In that regard, LANDLOCK_ACCESS_FS_RESOLVE_UNIX has the same semantics
as one of the "scoped" access rights.
== Motivation
Currently, landlocked processes can connect to named UNIX sockets
through the BSD socket API described in unix(7), by invoking socket(2)
followed by connect(2) with a suitable struct sockname_un holding the
socket's filename. This is a surprising gap in Landlock's sandboxing
capabilities for users (e.g. in [1]) and it can be used to escape a
sandbox when a Unix service offers command execution (various such
scenarios were listed by Tingmao Wang in [2]).
The original feature request is at [4].
== Alternatives and Related Work
=== Alternative: Use existing LSM hooks
We have carefully and seriously considered the use of existing LSM
hooks, but still came to the conclusion that a new LSM hook is better
suited in this case:
The existing hooks security_unix_stream_connect(),
security_unix_may_send() and security_socket_connect() do not give
access to the resolved filesystem path.
* Resolving the filesystem path in the struct sockaddr_un again within
a Landlock would produce a TOCTOU race, so this is not an option.
* We would therefore need to wire through the resolved struct path
from unix_find_bsd() to one of the existing LSM hooks which get
called later. This would be a more substantial change to af_unix.c.
The struct path that is available in the listening-side struct sock is
can be read through the existing hooks, but it is not an option to use
this information: As the listening socket may have been bound from
within a different namespace, the path that was used for that can is
in the general case not meaningful for a sandboxed process. In
particular, it is not possible to use this path (or prefixes thereof)
when constructing a sandbox policy in the client-side process.
Paul Moore also chimed in in support of adding a new hook, with the
rationale that the simplest change to the LSM hook interface has
traditionally proven to be the most robust. [11]
More details are on the Github issue at [6] and on the LKML at [9].
In a the discussion of the V2 review, started by Christian Brauner
[10], we have further explored the approach of reusing the existing
LSM hooks but still ended up leaning on the side of introducing a new
hook, with Paul Moore and me (gnoack) arguing for that option.
Further insights about the LSM hook were shared in the V3 review by
Tingmao Wang [12], who spotted additional requirements due to the two
approaches being merged into one patch set. The summary of that
discussion is in [13].
=== Related work: Scope Control for Pathname Unix Sockets
The motivation for this patch is the same as in Tingmao Wang's patch
set for "scoped" control for pathname Unix sockets [2], originally
proposed in the Github feature request [5].
In [14], we have settled on the decision to merge the two patch sets
into this one, whose primary way of controlling connect(2) is
LANDLOCK_ACCESS_FS_RESOLVE_UNIX, but where this flag additionally has
the semantics of only restricting this unix(7) IPC *outwards* of the
created Landlock domain, in line with the logic that exists for the
existing "scoped" flags already.
By having LANDLOCK_ACCESS_FS_RESOLVE_UNIX implement "scoping"
semantics, we can avoid introducing two separate interacting flags for
now, but we retain the option of introducing
LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET at a later point in time, should
such a flag be needed to express additional rules.
== Credits
The feature was originally suggested by Jann Horn in [7].
Tingmao Wang and Demi Marie Obenour have taken the initiative to
revive this discussion again in [1], [4] and [5].
Tingmao Wang has sent the patch set for the scoped access control for
pathname Unix sockets [2] and has contributed substantial insights
during the code review, shaping the form of the LSM hook and agreeing
to merge the pathname and scoped-flag patch sets.
Justin Suess has sent the patch for the LSM hook in [8] and
subsequently through this patch set.
Christian Brauner and Paul Moore have contributed to the design of the
new LSM hook, discussing the tradeoffs in [10].
Sebastian Andrzej Siewior and Kuniyuki Iwashima have helped with
locking questions in the networking subsystem. [15] [16]
Ryan Sullivan has started on an initial implementation and has brought
up relevant discussion points on the Github issue at [4].
As maintainer of Landlock, Mickaël Salaün has done the main review so
far and particularly pointed out ways in which the UNIX connect()
patch sets interact with each other and what we need to look for with
regards to UAPI consistency as Landlock evolves.
[1] https://lore.kernel.org/landlock/515ff0f4-2ab3-46de-8d1e-5c66a93c6ede@gmail.com/
[2] Tingmao Wang's "Implement scope control for pathname Unix sockets"
https://lore.kernel.org/all/cover.1767115163.git.m@maowtm.org/
[3] https://lore.kernel.org/all/20251230.bcae69888454@gnoack.org/
[4] Github issue for FS-based control for named Unix sockets:
https://github.com/landlock-lsm/linux/issues/36
[5] Github issue for scope-based restriction of named Unix sockets:
https://github.com/landlock-lsm/linux/issues/51
[6] https://github.com/landlock-lsm/linux/issues/36#issuecomment-2950632277
[7] https://lore.kernel.org/linux-security-module/CAG48ez3NvVnonOqKH4oRwRqbSOLO0p9djBqgvxVwn6gtGQBPcw@mail.gmail.com/
[8] Patch for the LSM hook:
https://lore.kernel.org/all/20251231213314.2979118-1-utilityemal77@gmail.com/
[9] https://lore.kernel.org/all/20260108.64bd7391e1ae@gnoack.org/
[10] https://lore.kernel.org/all/20260113-kerngesund-etage-86de4a21da24@brauner/
[11] https://lore.kernel.org/all/CAHC9VhQHZCe0LMx4xzSo-h1SWY489U4frKYnxu4YVrcJN3x7nA@mail.gmail.com/
[12] https://lore.kernel.org/all/e6b6b069-384c-4c45-a56b-fa54b26bc72a@maowtm.org/
[13] https://lore.kernel.org/all/aYMenaSmBkAsFowd@google.com/
[14] https://lore.kernel.org/all/20260205.Kiech3gupee1@digikod.net/
[15] https://lore.kernel.org/all/20260310151907.VYySCtJp@linutronix.de/
[16] https://lore.kernel.org/all/CAAVpQUC95mSjX1vRK===pubHofcYqbkNE7goYKiu6vha5GYAFw@mail.gmail.com/
---
== Patch set history
V1: https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
V2: https://lore.kernel.org/all/20260110143300.71048-2-gnoack3000@gmail.com/
V3: https://lore.kernel.org/all/20260119203457.97676-2-gnoack3000@gmail.com/
V4: https://lore.kernel.org/all/20260208231017.114343-1-gnoack3000@gmail.com/
V5: https://lore.kernel.org/all/20260215105158.28132-1-gnoack3000@gmail.com/
V6: https://lore.kernel.org/all/20260315222150.121952-1-gnoack3000@gmail.com/
Changes in V7:
* Implementation:
* LSM hook: Small header file layout restructurings, typos (Justin)
* Hold unix_state_lock across the usage of the other Landlock domain
This prevents a UAF; spotted by Mickaël and Sebastian in code review 🏆
* Ignore the return value from landlock_init_layer_masks() -
it is not needed in this specific case
* Add a BUILD_BUG_ON to unmask_scoped_access() like in
domain_is_scoped(). Revise the BUILD_BUG_ON logic and bring both
implementations in line.
* Documentation and commentary:
* Mention access_mask_t change from u16 to u32 in commit message
* Improve documentation for unmask_scoped_access()
* Various typos and smaller documentation fixes, caught in code review
* Add ABI version remark to landlock.h
* Add comment to explain why returning 0 on SOCK_DEAD is correct -
This is not obvious in the code and requires understanding the callers
* In kernel docs, use "case 6 ... 8" in a place
Changes in V6:
* Implementation:
* Move the LSM hook call after the check that checks for the other
end's matching socket type. (Justin Suess)
* Lock with unix_state_lock() and check SOCK_DEAD.
* Remove unnecessary layer_access_masks_empty() call (and its
implementation).
* Documentation:
* Squash docs with design rationale into main implementation commit,
and cross-referece it from the header docs.
* Clarify that denials result in EACCES and that this is consistent
with other filesystem access rights.
* Minor:
* Use mem_is_zero() in is_layer_masks_allowed() (minor cleanup)
* Omit unnecessary __attribute__((fallthrough)) usages
* Remove comment at the end of a line in a place.
* Selftests:
* sun_path population fixes
* coredump test: Set EUID to 0 (needed for UML-based selftests)
Link[1]: https://lore.kernel.org/all/20260218.ohth8theu8Yi@digikod.net/
Changes in V5:
This change primarily adds tests, changing the testing approach for
the main test to use the scoped_domains fixture as in Tingmao's patch
set [2], and adding tests for the audit and coredump cases.
* Selftests:
* Replaced the main selftest with one based on scoped_domains
* Added audit test
* Added test for the coredump case
* Added a follow-up commit that simplifies ruleset enforcement
* Kernel code:
* Mark coredump check as unlikely (per Justin's review)
* Drop check for socket type (per Mickaël's review)
Changes in V4:
Since this version, this patch set subsumes the scoping semantics from
Tingmao Wang's "Scope Control" patch set [2], per discussion with
Tingmao Wang and Mickaël Salaün in [14] and in the thread leading up
to it.
Now, LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET only restricts connect(2) and
sendmsg(2) *outwards* of the domain where it is restricted, *with the
same semantics as a "scoped" flag*.
* Implement a layer-mask based version of domain_is_scoped():
unmask_scoped_access(). Rationale: domain_is_scoped() returns
early, which we can't do in the layer masks based variant. The two
variants are similar enough.
* LSM hook: Replace 'type' argument with 'sk' argument,
per discussion in [12] and [13].
* Bump ABI version to 9 (pessimistically assuming that we won't make
it for 7.0)
* Documentation fixes in header file and in Documentation/
* selftests: more test variants, now also parameterizing whether the
server socket gets created within the Landlock domain or before that
* selftests: use EXPECT_EQ() for test cleanup
Changes in V3:
* LSM hook: rename it to security_unix_find() (Justin Suess)
(resolving the previously open question about the LSM hook name)
Related discussions:
https://lore.kernel.org/all/20260112.Wufar9coosoo@digikod.net/
https://lore.kernel.org/all/CAHC9VhSRiHwLEWfFkQdPEwgB4AXKbXzw_+3u=9hPpvUTnu02Bg@mail.gmail.com/
* Reunite the three UNIX resolving access rights back into one
(resolving the previously open question about the access right
structuring) Related discussion:
https://lore.kernel.org/all/20260112.Wufar9coosoo@digikod.net/)
* Sample tool: Add new UNIX lookup access rights to ACCESS_FILE
Changes in V2:
* Send Justin Suess's LSM hook patch together with the Landlock
implementation
* LSM hook: Pass type and flags parameters to the hook, to make the
access right more generally usable across LSMs, per suggestion from
Paul Moore (Implemented by Justin)
* Split the access right into the three types of UNIX domain sockets:
SOCK_STREAM, SOCK_DGRAM and SOCK_SEQPACKET.
* selftests: More exhaustive tests.
* Removed a minor commit from V1 which adds a missing close(fd) to a
test (it is already in the mic-next branch)
Günther Noack (10):
landlock: Use mem_is_zero() in is_layer_masks_allowed()
landlock: Control pathname UNIX domain socket resolution by path
samples/landlock: Add support for named UNIX domain socket
restrictions
landlock: Clarify BUILD_BUG_ON check in scoping logic
selftests/landlock: Replace access_fs_16 with ACCESS_ALL in fs_test
selftests/landlock: Test LANDLOCK_ACCESS_FS_RESOLVE_UNIX
selftests/landlock: Audit test for LANDLOCK_ACCESS_FS_RESOLVE_UNIX
selftests/landlock: Check that coredump sockets stay unrestricted
selftests/landlock: fs_test: Simplify ruleset creation and enforcement
landlock: Document FS access right for pathname UNIX sockets
Justin Suess (1):
lsm: Add LSM hook security_unix_find
Documentation/security/landlock.rst | 40 +
Documentation/userspace-api/landlock.rst | 14 +-
include/linux/lsm_hook_defs.h | 5 +
include/linux/security.h | 11 +
include/uapi/linux/landlock.h | 21 +
net/unix/af_unix.c | 10 +-
samples/landlock/sandboxer.c | 12 +-
security/landlock/access.h | 2 +-
security/landlock/audit.c | 1 +
security/landlock/fs.c | 135 +-
security/landlock/limits.h | 2 +-
security/landlock/syscalls.c | 2 +-
security/landlock/task.c | 9 +-
security/security.c | 20 +
tools/testing/selftests/landlock/base_test.c | 2 +-
tools/testing/selftests/landlock/fs_test.c | 1345 ++++++++++--------
16 files changed, 1003 insertions(+), 628 deletions(-)
Range-diff against v6:
1: b2235eddb099 ! 1: dec515e7d7bf lsm: Add LSM hook security_unix_find
@@ Metadata
## Commit message ##
lsm: Add LSM hook security_unix_find
- Add a LSM hook security_unix_find.
+ Add an LSM hook security_unix_find.
- This hook is called to check the path of a named unix socket before a
+ This hook is called to check the path of a named UNIX socket before a
connection is initiated. The peer socket may be inspected as well.
Why existing hooks are unsuitable:
@@ Commit message
may be bound to a path in a different namespace than the caller,
resulting in a path that cannot be referenced at policy creation time.
+ Consumers of the hook wishing to reference @other are responsible
+ for acquiring the unix_state_lock and checking for the SOCK_DEAD flag
+ therein, ensuring the socket hasn't died since lookup.
+
Cc: Günther Noack <gnoack3000@gmail.com>
Cc: Tingmao Wang <m@maowtm.org>
+ Cc: Mickaël Salaün <mic@digikod.net>
+ Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
+ Signed-off-by: Günther Noack <gnoack3000@gmail.com>
## include/linux/lsm_hook_defs.h ##
@@ include/linux/lsm_hook_defs.h: LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
@@ net/unix/af_unix.c: static struct sock *unix_find_bsd(struct sockaddr_un *sunadd
+ if (sk->sk_type != type)
goto sock_put;
-+ /*
-+ * We call the hook because we know that the inode is a socket and we
-+ * hold a valid reference to it via the path.
-+ */
+ err = security_unix_find(&path, sk, flags);
+ if (err)
+ goto sock_put;
++
+ touch_atime(&path);
+
path_put(&path);
@@ security/security.c: int security_mptcp_add_subflow(struct sock *sk, struct sock
+ * @flags: flags associated with the socket
+ *
+ * This hook is called to check permissions before connecting to a named
-+ * AF_UNIX socket.
++ * AF_UNIX socket. The caller does not hold any locks on @other.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
2: 0e25c15eea62 ! 2: aaa334660a52 landlock: use mem_is_zero() in is_layer_masks_allowed()
@@ Metadata
Author: Günther Noack <gnoack3000@gmail.com>
## Commit message ##
- landlock: use mem_is_zero() in is_layer_masks_allowed()
+ landlock: Use mem_is_zero() in is_layer_masks_allowed()
This is equivalent, but expresses the intent a bit clearer.
3: e50515788eba ! 3: 4d455134c5d9 landlock: Control pathname UNIX domain socket resolution by path
@@ Commit message
landlock: Control pathname UNIX domain socket resolution by path
* Add a new access right LANDLOCK_ACCESS_FS_RESOLVE_UNIX, which
- controls the look up operations for named UNIX domain sockets. The
+ controls the lookup operations for named UNIX domain sockets. The
resolution happens during connect() and sendmsg() (depending on
socket type).
+ * Change access_mask_t from u16 to u32 (see below)
* Hook into the path lookup in unix_find_bsd() in af_unix.c, using a
LSM hook. Make policy decisions based on the new access rights
* Increment the Landlock ABI version.
- * Minor test adaptions to keep the tests working.
+ * Minor test adaptations to keep the tests working.
* Document the design rationale for scoped access rights,
and cross-reference it from the header documentation.
@@ Commit message
In case of a denial, connect() and sendmsg() return EACCES, which is
the same error as it is returned if the user does not have the write
- bit in the traditional Unix file system permissions of that file.
+ bit in the traditional UNIX file system permissions of that file.
+
+ The access_mask_t type grows from u16 to u32 to make space for the new
+ access right. This also doubles the size of struct layer_access_masks
+ from 32 byte to 64 byte.
Document the (possible future) interaction between scoped flags and
other access rights in struct landlock_ruleset_attr, and summarize the
@@ include/uapi/linux/landlock.h: struct landlock_net_port_attr {
+ * were created outside of the newly created Landlock domain (e.g. from within
+ * a parent domain or from an unrestricted process). Newly created UNIX
+ * servers within the same Landlock domain continue to be accessible. In this
-+ * regard, %LANDLOCK_ACCESS_RESOLVE_UNIX has the same semantics as the
++ * regard, %LANDLOCK_ACCESS_FS_RESOLVE_UNIX has the same semantics as the
+ * ``LANDLOCK_SCOPE_*`` flags.
+ *
+ * If a resolve attempt is denied, the operation returns an ``EACCES`` error,
+ * in line with other filesystem access rights (but different to denials for
+ * abstract UNIX domain sockets).
+ *
++ * This access right is available since the ninth version of the Landlock ABI.
++ *
+ * The rationale for this design is described in
+ * :ref:`Documentation/security/landlock.rst <scoped-flags-interaction>`.
*
@@ security/landlock/fs.c: static int hook_path_truncate(const struct path *const p
+ * This does the same as domain_is_scoped(), but unmasks bits in @masks.
+ * It can not return early as domain_is_scoped() does.
+ *
++ * A scoped access for a given access right bit is allowed iff, for all layer
++ * depths where the access bit is set, the client and server domain are the
++ * same. This function clears the access rights @access in @masks at all layer
++ * depths where the client and server domain are the same, so that, when they
++ * are all cleared, the access is allowed.
++ *
+ * @client: Client domain
+ * @server: Server domain
+ * @masks: Layer access masks to unmask
-+ * @access: Access bit that controls scoping
++ * @access: Access bits that control scoping
+ */
+static void unmask_scoped_access(const struct landlock_ruleset *const client,
+ const struct landlock_ruleset *const server,
@@ security/landlock/fs.c: static int hook_path_truncate(const struct path *const p
+ if (!server)
+ return;
+
++ /*
++ * client_layer must be a signed integer with greater capacity
++ * than client->num_layers to ensure the following loop stops.
++ */
++ BUILD_BUG_ON(sizeof(client_layer) > sizeof(client->num_layers));
++
+ client_layer = client->num_layers - 1;
+ client_walker = client->hierarchy;
+ server_layer = server->num_layers - 1;
@@ security/landlock/fs.c: static int hook_path_truncate(const struct path *const p
+ if (unlikely(flags & SOCK_COREDUMP))
+ return 0;
+
-+ /* Access to the same (or a lower) domain is always allowed. */
+ subject = landlock_get_applicable_subject(current_cred(),
+ fs_resolve_unix, NULL);
+
+ if (!subject)
+ return 0;
+
-+ if (!landlock_init_layer_masks(subject->domain, fs_resolve_unix.fs,
-+ &layer_masks, LANDLOCK_KEY_INODE))
-+ return 0;
++ /*
++ * Ignoring return value: that the domains apply was already checked in
++ * landlock_get_applicable_subject() above.
++ */
++ landlock_init_layer_masks(subject->domain, fs_resolve_unix.fs,
++ &layer_masks, LANDLOCK_KEY_INODE);
+
+ /* Checks the layers in which we are connecting within the same domain. */
+ unix_state_lock(other);
+ if (unlikely(sock_flag(other, SOCK_DEAD) || !other->sk_socket ||
+ !other->sk_socket->file)) {
+ unix_state_unlock(other);
++ /*
++ * We rely on the caller to catch the (non-reversible) SOCK_DEAD
++ * condition and retry the lookup. If we returned an error
++ * here, the lookup would not get retried.
++ */
+ return 0;
+ }
+ dom_other = landlock_cred(other->sk_socket->file->f_cred)->domain;
-+ unix_state_unlock(other);
+
++ /* Access to the same (or a lower) domain is always allowed. */
+ unmask_scoped_access(subject->domain, dom_other, &layer_masks,
+ fs_resolve_unix.fs);
++ unix_state_unlock(other);
+
+ /* Checks the connections to allow-listed paths. */
+ if (is_access_to_paths_allowed(subject->domain, path,
4: 8c1a813b9831 = 4: 70354dfcefcb samples/landlock: Add support for named UNIX domain socket restrictions
-: ------------ > 5: 00c90045f470 landlock: Clarify BUILD_BUG_ON check in scoping logic
9: 43c93c9af805 ! 6: b8e6477b2f9a landlock: Document FS access right for pathname UNIX sockets
@@ Metadata
Author: Günther Noack <gnoack3000@gmail.com>
## Commit message ##
- landlock: Document FS access right for pathname UNIX sockets
+ selftests/landlock: Replace access_fs_16 with ACCESS_ALL in fs_test
- Cc: Justin Suess <utilityemal77@gmail.com>
- Cc: Mickaël Salaün <mic@digikod.net>
+ The access_fs_16 variable was originally intended to stay frozen at 16
+ access rights so that audit tests would not need updating when new
+ access rights are added. Now that we have 17 access rights, the name
+ is confusing.
+
+ Replace all uses of access_fs_16 with ACCESS_ALL and delete the
+ variable.
+
+ Suggested-by: Mickaël Salaün <mic@digikod.net>
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
- ## Documentation/userspace-api/landlock.rst ##
-@@ Documentation/userspace-api/landlock.rst: to be explicit about the denied-by-default access rights.
- LANDLOCK_ACCESS_FS_MAKE_SYM |
- LANDLOCK_ACCESS_FS_REFER |
- LANDLOCK_ACCESS_FS_TRUNCATE |
-- LANDLOCK_ACCESS_FS_IOCTL_DEV,
-+ LANDLOCK_ACCESS_FS_IOCTL_DEV |
-+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
- .handled_access_net =
- LANDLOCK_ACCESS_NET_BIND_TCP |
- LANDLOCK_ACCESS_NET_CONNECT_TCP,
-@@ Documentation/userspace-api/landlock.rst: version, and only use the available subset of access rights:
- /* Removes LANDLOCK_SCOPE_* for ABI < 6 */
- ruleset_attr.scoped &= ~(LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
- LANDLOCK_SCOPE_SIGNAL);
-+ __attribute__((fallthrough));
-+ case 7:
-+ case 8:
-+ /* Removes LANDLOCK_ACCESS_FS_RESOLVE_UNIX for ABI < 9 */
-+ ruleset_attr.handled_access_fs &= ~LANDLOCK_ACCESS_FS_RESOLVE_UNIX;
- }
+ ## tools/testing/selftests/landlock/fs_test.c ##
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, execute_make)
+ * only the blocked ones are logged.
+ */
- This enables the creation of an inclusive ruleset that will contain our rules.
-@@ Documentation/userspace-api/landlock.rst: enforce Landlock rulesets across all threads of the calling process
- using the ``LANDLOCK_RESTRICT_SELF_TSYNC`` flag passed to
- sys_landlock_restrict_self().
+-/* clang-format off */
+-static const __u64 access_fs_16 =
+- 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;
+-/* clang-format on */
+-
+ TEST_F(audit_layout1, execute_read)
+ {
+ struct audit_records records;
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, execute_read)
+ test_check_exec(_metadata, 0, file1_s1d1);
-+Pathname UNIX sockets (ABI < 9)
-+-------------------------------
-+
-+Starting with the Landlock ABI version 9, it is possible to restrict
-+connections to pathname UNIX domain sockets (:manpage:`unix(7)`) using
-+the new ``LANDLOCK_ACCESS_FS_RESOLVE_UNIX`` right.
-+
- .. _kernel_support:
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ /*
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, write_file)
+ struct audit_records records;
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, read_file)
+ struct audit_records records;
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(EACCES, test_open(file1_s1d1, O_RDONLY));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, read_dir)
+ struct audit_records records;
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(EACCES, test_open(dir_s1d1, O_DIRECTORY));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, remove_dir)
+ EXPECT_EQ(0, unlink(file2_s1d3));
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(-1, rmdir(dir_s1d3));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, remove_file)
+ struct audit_records records;
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(-1, unlink(file1_s1d3));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_char)
+ EXPECT_EQ(0, unlink(file1_s1d3));
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(-1, mknod(file1_s1d3, S_IFCHR | 0644, 0));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_dir)
+ EXPECT_EQ(0, unlink(file1_s1d3));
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(-1, mkdir(file1_s1d3, 0755));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_reg)
+ EXPECT_EQ(0, unlink(file1_s1d3));
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(-1, mknod(file1_s1d3, S_IFREG | 0644, 0));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_sock)
+ EXPECT_EQ(0, unlink(file1_s1d3));
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(-1, mknod(file1_s1d3, S_IFSOCK | 0644, 0));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_fifo)
+ EXPECT_EQ(0, unlink(file1_s1d3));
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(-1, mknod(file1_s1d3, S_IFIFO | 0644, 0));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_block)
+ EXPECT_EQ(0, unlink(file1_s1d3));
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(-1, mknod(file1_s1d3, S_IFBLK | 0644, 0));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_sym)
+ EXPECT_EQ(0, unlink(file1_s1d3));
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(-1, symlink("target", file1_s1d3));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, refer_rename)
+ EXPECT_EQ(0, unlink(file1_s1d3));
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(EACCES, test_rename(file1_s1d2, file1_s2d3));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, refer_exchange)
+ EXPECT_EQ(0, unlink(file1_s1d3));
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ /*
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, truncate)
+ struct audit_records records;
+
+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
+- .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ EXPECT_EQ(-1, truncate(file1_s1d3, 0));
+@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, ioctl_dev)
+ drop_access_rights(_metadata,
+ &(struct landlock_ruleset_attr){
+ .handled_access_fs =
+- access_fs_16 &
++ ACCESS_ALL &
+ ~LANDLOCK_ACCESS_FS_READ_FILE,
+ });
- Kernel support
5: 7c360d3c293f ! 7: 66554c5b5b9c landlock/selftests: Test LANDLOCK_ACCESS_FS_RESOLVE_UNIX
@@ Metadata
Author: Günther Noack <gnoack3000@gmail.com>
## Commit message ##
- landlock/selftests: Test LANDLOCK_ACCESS_FS_RESOLVE_UNIX
+ selftests/landlock: Test LANDLOCK_ACCESS_FS_RESOLVE_UNIX
* Extract common helpers from an existing IOCTL test that
also uses pathname unix(7) sockets.
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F_FORK(ioctl, handle_file_acces
+ enforce_ruleset(_metadata, fd);
+ EXPECT_EQ(0, close(fd));
+ } else {
-+ drop_access_rights(
-+ _metadata,
-+ &(struct landlock_ruleset_attr){
-+ .handled_access_fs =
-+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
-+ });
++ struct landlock_ruleset_attr attr = {
++ .handled_access_fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
++ };
++ drop_access_rights(_metadata, &attr);
+ }
+}
+
6: 6ea15ea91990 ! 8: fab3d3d71215 landlock/selftests: Audit test for LANDLOCK_ACCESS_FS_RESOLVE_UNIX
@@ Metadata
Author: Günther Noack <gnoack3000@gmail.com>
## Commit message ##
- landlock/selftests: Audit test for LANDLOCK_ACCESS_FS_RESOLVE_UNIX
+ selftests/landlock: Audit test for LANDLOCK_ACCESS_FS_RESOLVE_UNIX
Add an audit test to check that Landlock denials from
LANDLOCK_ACCESS_FS_RESOLVE_UNIX result in audit logs in the expected
@@ Commit message
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
## tools/testing/selftests/landlock/fs_test.c ##
-@@ tools/testing/selftests/landlock/fs_test.c: static const __u64 access_fs_16 =
- LANDLOCK_ACCESS_FS_MAKE_SYM |
- LANDLOCK_ACCESS_FS_REFER |
- LANDLOCK_ACCESS_FS_TRUNCATE |
-- LANDLOCK_ACCESS_FS_IOCTL_DEV;
-+ LANDLOCK_ACCESS_FS_IOCTL_DEV |
-+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX;
- /* clang-format on */
-
- TEST_F(audit_layout1, execute_read)
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, ioctl_dev)
EXPECT_EQ(1, records.domain);
}
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, ioctl_dev)
+ if (!child_pid) {
+ drop_access_rights(_metadata,
+ &(struct landlock_ruleset_attr){
-+ .handled_access_fs = access_fs_16,
++ .handled_access_fs = ACCESS_ALL,
+ });
+
+ cli_fd = socket(AF_UNIX, SOCK_STREAM, 0);
7: 0ada3dd8e1a1 ! 9: 4e8a07344278 landlock/selftests: Check that coredump sockets stay unrestricted
@@ Metadata
Author: Günther Noack <gnoack3000@gmail.com>
## Commit message ##
- landlock/selftests: Check that coredump sockets stay unrestricted
+ selftests/landlock: Check that coredump sockets stay unrestricted
Even when a process is restricted with the new
- LANDLOCK_ACCESS_FS_RESOLVE_SOCKET right, the kernel can continue
- writing its coredump to the configured coredump socket.
+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX right, the kernel can continue writing
+ its coredump to the configured coredump socket.
In the test, we create a local server and rewire the system to write
coredumps into it. We then create a child process within a Landlock
- domain where LANDLOCK_ACCESS_FS_RESOLVE_SOCKET is restricted and make
+ domain where LANDLOCK_ACCESS_FS_RESOLVE_UNIX is restricted and make
the process crash. The test uses SO_PEERCRED to check that the
connecting client process is the expected one.
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(scoped_domains, unix_seqpacke
+ set_core_pattern(_metadata, core_pattern);
+
+ /* Restrict LANDLOCK_ACCESS_FS_RESOLVE_UNIX. */
-+ drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-+ .handled_access_fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
-+ });
++ drop_access_rights(_metadata,
++ &(struct landlock_ruleset_attr){
++ .handled_access_fs =
++ LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
++ });
+
+ /* Fork a child that crashes. */
+ child_pid = fork();
8: e9f591ec527f ! 10: fcaef2882dbd landlock/selftests: fs_test: Simplify ruleset creation and enforcement
@@ Metadata
Author: Günther Noack <gnoack3000@gmail.com>
## Commit message ##
- landlock/selftests: fs_test: Simplify ruleset creation and enforcement
+ selftests/landlock: fs_test: Simplify ruleset creation and enforcement
* Add enforce_fs() for defining and enforcing a ruleset in one step
* In some places, dropped "ASSERT_LE(0, fd)" checks after
@@ tools/testing/selftests/landlock/fs_test.c: FIXTURE_TEARDOWN(scoped_domains)
- enforce_ruleset(_metadata, fd);
- EXPECT_EQ(0, close(fd));
- } else {
-- drop_access_rights(
-- _metadata,
-- &(struct landlock_ruleset_attr){
-- .handled_access_fs =
-- LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
-- });
+- struct landlock_ruleset_attr attr = {
+- .handled_access_fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
+- };
+- drop_access_rights(_metadata, &attr);
- }
-}
-
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F_FORK(coredump, socket_not_res
set_core_pattern(_metadata, core_pattern);
/* Restrict LANDLOCK_ACCESS_FS_RESOLVE_UNIX. */
-- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
-- });
+- drop_access_rights(_metadata,
+- &(struct landlock_ruleset_attr){
+- .handled_access_fs =
+- LANDLOCK_ACCESS_FS_RESOLVE_UNIX,
+- });
+ enforce_fs(_metadata, LANDLOCK_ACCESS_FS_RESOLVE_UNIX, NULL);
/* Fork a child that crashes. */
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, execute_read)
test_check_exec(_metadata, 0, file1_s1d1);
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
/*
* The only difference with the previous audit_layout1.execute_read test is
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, write_file)
struct audit_records records;
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY));
EXPECT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, read_file)
struct audit_records records;
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(EACCES, test_open(file1_s1d1, O_RDONLY));
EXPECT_EQ(0, matches_log_fs(_metadata, self->audit_fd, "fs\\.read_file",
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, read_dir)
struct audit_records records;
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(EACCES, test_open(dir_s1d1, O_DIRECTORY));
EXPECT_EQ(0, matches_log_fs(_metadata, self->audit_fd, "fs\\.read_dir",
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, remove_dir)
EXPECT_EQ(0, unlink(file2_s1d3));
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(-1, rmdir(dir_s1d3));
EXPECT_EQ(EACCES, errno);
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, remove_file)
struct audit_records records;
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(-1, unlink(file1_s1d3));
EXPECT_EQ(EACCES, errno);
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_char)
EXPECT_EQ(0, unlink(file1_s1d3));
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(-1, mknod(file1_s1d3, S_IFCHR | 0644, 0));
EXPECT_EQ(EACCES, errno);
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_dir)
EXPECT_EQ(0, unlink(file1_s1d3));
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(-1, mkdir(file1_s1d3, 0755));
EXPECT_EQ(EACCES, errno);
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_reg)
EXPECT_EQ(0, unlink(file1_s1d3));
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(-1, mknod(file1_s1d3, S_IFREG | 0644, 0));
EXPECT_EQ(EACCES, errno);
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_sock)
EXPECT_EQ(0, unlink(file1_s1d3));
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(-1, mknod(file1_s1d3, S_IFSOCK | 0644, 0));
EXPECT_EQ(EACCES, errno);
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_fifo)
EXPECT_EQ(0, unlink(file1_s1d3));
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(-1, mknod(file1_s1d3, S_IFIFO | 0644, 0));
EXPECT_EQ(EACCES, errno);
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_block)
EXPECT_EQ(0, unlink(file1_s1d3));
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(-1, mknod(file1_s1d3, S_IFBLK | 0644, 0));
EXPECT_EQ(EACCES, errno);
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, make_sym)
EXPECT_EQ(0, unlink(file1_s1d3));
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(-1, symlink("target", file1_s1d3));
EXPECT_EQ(EACCES, errno);
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, refer_rename)
EXPECT_EQ(0, unlink(file1_s1d3));
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(EACCES, test_rename(file1_s1d2, file1_s2d3));
EXPECT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, refer_exchange
EXPECT_EQ(0, unlink(file1_s1d3));
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
/*
* The only difference with the previous audit_layout1.refer_rename test is
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, truncate)
struct audit_records records;
- drop_access_rights(_metadata, &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
EXPECT_EQ(-1, truncate(file1_s1d3, 0));
EXPECT_EQ(EACCES, errno);
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, ioctl_dev)
- drop_access_rights(_metadata,
- &(struct landlock_ruleset_attr){
- .handled_access_fs =
-- access_fs_16 &
+- ACCESS_ALL &
- ~LANDLOCK_ACCESS_FS_READ_FILE,
- });
-+ enforce_fs(_metadata, access_fs_16 & ~LANDLOCK_ACCESS_FS_READ_FILE,
-+ NULL);
++ enforce_fs(_metadata, ACCESS_ALL & ~LANDLOCK_ACCESS_FS_READ_FILE, NULL);
fd = open("/dev/null", O_RDONLY | O_CLOEXEC);
ASSERT_LE(0, fd);
@@ tools/testing/selftests/landlock/fs_test.c: TEST_F(audit_layout1, resolve_unix)
if (!child_pid) {
- drop_access_rights(_metadata,
- &(struct landlock_ruleset_attr){
-- .handled_access_fs = access_fs_16,
+- .handled_access_fs = ACCESS_ALL,
- });
-+ enforce_fs(_metadata, access_fs_16, NULL);
++ enforce_fs(_metadata, ACCESS_ALL, NULL);
cli_fd = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, cli_fd);
-: ------------ > 11: 362a0e8f84a0 landlock: Document FS access right for pathname UNIX sockets
--
2.53.0
^ permalink raw reply
* Re: [PATCH v6 3/9] landlock: Control pathname UNIX domain socket resolution by path
From: Günther Noack @ 2026-03-23 15:31 UTC (permalink / raw)
To: Mickaël Salaün
Cc: John Johansen, Tingmao Wang, Justin Suess,
Sebastian Andrzej Siewior, Kuniyuki Iwashima, Jann Horn,
linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Tahera Fahimi, Kees Cook
In-Reply-To: <20260321.Aiquah5gaedo@digikod.net>
Hello!
On Sat, Mar 21, 2026 at 10:09:35AM +0100, Mickaël Salaün wrote:
> On Fri, Mar 20, 2026 at 11:25:04PM +0100, Günther Noack wrote:
> > On Fri, Mar 20, 2026 at 06:51:13PM +0100, Mickaël Salaün wrote:
> > > On Fri, Mar 20, 2026 at 05:15:40PM +0100, Günther Noack wrote:
> > > > On Wed, Mar 18, 2026 at 05:52:48PM +0100, Mickaël Salaün wrote:
> > > > > On Sun, Mar 15, 2026 at 11:21:44PM +0100, Günther Noack wrote:
> > > > > > + * @client: Client domain
> > > > > > + * @server: Server domain
> > > > > > + * @masks: Layer access masks to unmask
> > > > > > + * @access: Access bit that controls scoping
> > > > > > + */
> > > > > > +static void unmask_scoped_access(const struct landlock_ruleset *const client,
> > > > > > + const struct landlock_ruleset *const server,
> > > > > > + struct layer_access_masks *const masks,
> > > > > > + const access_mask_t access)
> > > > > > +{
> > > > > > + int client_layer, server_layer;
> > > > > > + const struct landlock_hierarchy *client_walker, *server_walker;
> > > > > > +
> > > > > > + /* This should not happen. */
> > > > > > + if (WARN_ON_ONCE(!client))
> > > > > > + return;
> > > > > > +
> > > > > > + /* Server has no Landlock domain; nothing to clear. */
> > > > > > + if (!server)
> > > > > > + return;
> > > > > > +
> > > > >
> > > > > Please also copy the BUILD_BUG_ON() from domain_is_scoped().
> > > >
> > > > I don't understand what this check is good for. It says:
> > > >
> > > > /*
> > > > * client_layer must be a signed integer with greater capacity
> > > > * than client->num_layers to ensure the following loop stops.
> > > > */
> > > > BUILD_BUG_ON(sizeof(client_layer) > sizeof(client->num_layers));
> > > >
> > > > For the loop to terminate, in my understanding, client_layer must be
> > > > able to store client->num_layers - 1 down to - 1, but that is anyway a
> > > > given since num_layers can't exceed 16 and client_layer is signed. It
> > > > seems that the termination of this would anyway be caught in our tests
> > > > as well?
> > > >
> > > > Could you please clarify what this BUILD_BUG_ON() is trying to assert?
> > >
> > > The intent was to make sure client_layer is indeed an int and not an
> > > unsigned int for instance. Hopefully tests catch that but using a
> > > build-time assert catch the potential issue and document it. Also, it
> > > would be weird to not have the same checks in both copies of code.
> >
> > It isn't clear to me why the BUILD_BUG_ON is checking based on the
> > sizeof() of these variables then. The number of bytes in an integer
> > type is independent of its signedness, after all. Should we rather do
> > this maybe?
> >
> > /*
> > * client_layer must be a signed integer to ensure the following
> > * loop stops.
> > */
> > BUILD_BUG_ON(!is_signed_type(typeof(client_layer)));
>
> I didn't know about this macro, looks good.
>
> >
> > (Although that is also a bit of a triviality given that the same
> > variable is being defined as a signed integer just a few lines above,
> > but at least it is very explicit about it.)
>
> Yeah, I know, but I added this canary check because I was bitten by a
> bug. Even if it might be trivial now, it might help when working on
> other parts, and it's just a build-time check that also serves as doc.
> If in doubt, let's keep build-time checks that were most likely added
> for a good reason. I prefer to have build-time errors than run-time
> ones.
>
> >
> > I'd drop the remark about the capacity, as even a signed 8-bit integer
> > is large enough to hold the layer indices and the -1.
>
> Large enough for now... All these checks are useful to easily spot
> issues if/when the current invariant change (e.g. if one day we extend
> the number of max layers). Integer C types can silently cast to other
> integer types (with different capacity or sign/unsigned), which might be
> the source of bugs. So yeah, please keep the current capacity check and
> add the sign one, it won't do any harm.
I feel that this is still not fully addressed. I know it is a
triviality to go in circles over this BUILD_BUG, but the current form:
BUILD_BUG_ON(sizeof(client_layer) > sizeof(client->num_layers));
checks that the size of client_layer is smaller(!) or equal than the
size of client->num_layers, so it seems that the check is accidentally
done the wrong way around. It does not fire because the two integers
happen to have the same size, but it feels a bit misleading to leave
it like that?
To keep things moving, and because it feels like a minor (any maybe
optional) issue, I will send V7 now and optimistically do the
following:
* Copy the BUILD_BUG_ON and matching comment from domain_is_checked()
as you initially requested.
* Make a constructive proposal that rewrites the check in both
functions, to check that all numbers from -1 to
LANDLOCK_MAX_NUM_LAYERS are representable (and the loop therefore
terminates). In my understanding that was the intention here.
–Günther
^ permalink raw reply
* Re: [PATCH v6 1/9] lsm: Add LSM hook security_unix_find
From: Georgia Garcia @ 2026-03-23 14:37 UTC (permalink / raw)
To: Mickaël Salaün, John Johansen
Cc: Paul Moore, Günther Noack, James Morris, Serge E . Hallyn,
Tingmao Wang, Justin Suess, linux-security-module,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Sebastian Andrzej Siewior, Kuniyuki Iwashima,
Simon Horman, netdev, Alexander Viro, Christian Brauner
In-Reply-To: <20260318.In1aekohyivu@digikod.net>
Hello,
On Wed, 2026-03-18 at 09:48 +0100, Mickaël Salaün wrote:
> On Tue, Mar 17, 2026 at 05:34:57PM -0400, Paul Moore wrote:
> > On Mar 15, 2026 =?UTF-8?q?G=C3=BCnther=20Noack?= <gnoack3000@gmail.com> wrote:
> > >
> > > Add a LSM hook security_unix_find.
> > >
> > > This hook is called to check the path of a named unix socket before a
> > > connection is initiated. The peer socket may be inspected as well.
> > >
> > > Why existing hooks are unsuitable:
> > >
> > > Existing socket hooks, security_unix_stream_connect(),
> > > security_unix_may_send(), and security_socket_connect() don't provide
> > > TOCTOU-free / namespace independent access to the paths of sockets.
> > >
> > > (1) We cannot resolve the path from the struct sockaddr in existing hooks.
> > > This requires another path lookup. A change in the path between the
> > > two lookups will cause a TOCTOU bug.
> > >
> > > (2) We cannot use the struct path from the listening socket, because it
> > > may be bound to a path in a different namespace than the caller,
> > > resulting in a path that cannot be referenced at policy creation time.
> > >
> > > Cc: Günther Noack <gnoack3000@gmail.com>
> > > Cc: Tingmao Wang <m@maowtm.org>
> > > Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> > > ---
> > > include/linux/lsm_hook_defs.h | 5 +++++
> > > include/linux/security.h | 11 +++++++++++
> > > net/unix/af_unix.c | 13 ++++++++++---
> > > security/security.c | 20 ++++++++++++++++++++
> > > 4 files changed, 46 insertions(+), 3 deletions(-)
> >
> > Some really minor nitpicky things (below), but nothing critical.
> > However, as we discussed, I would like to see the AppArmor folks comment
> > on the new hook before we merge anything as I know they have an interest
> > here.
>
> John, Georgia, we've been discussing this new hook for a few months now
> but didn't hear from you yet. We plan to merge this patch series with
> the 7.1 merge window (in a few weeks), so before that I'd like to merge
> it in -next in a few days to get a broader coverage. I'm pretty sure
> this hook will work well with AppArmor too, but could you please take
> look to confirm?
Apologies for the long delay replying. I have looked it over and I have
no objections on the hook, it looks good to me. I would prefer if we
got a reply from John as well since I'm not 100% confident but he
should be out this week. In any case,
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Thanks and sorry again for the long time to reply.
^ permalink raw reply
* Re: [PATCH 6/7] tomoyo: Convert from sb_mount to granular mount hooks
From: Tetsuo Handa @ 2026-03-23 10:32 UTC (permalink / raw)
To: Christian Brauner, Song Liu
Cc: viro@zeniv.linux.org.uk, Song Liu, paul@paul-moore.com,
jmorris@namei.org, serge@hallyn.com, jack@suse.cz,
john.johansen@canonical.com, stephen.smalley.work@gmail.com,
omosnace@redhat.com, mic@digikod.net, gnoack@google.com,
takedakn@nttdata.co.jp, herton@canonical.com, Kernel Team,
selinux@vger.kernel.org, apparmor@lists.ubuntu.com,
linux-fsdevel@vger.kernel.org,
linux-security-module@vger.kernel.org
In-Reply-To: <20260323-klappen-atemschutz-7a0af8c6b087@brauner>
On 2026/03/23 19:16, Christian Brauner wrote:
>>> Since not all filesystems need to resolve dev_name argument, conversion from dev_name
>>> to "struct path" is up to individual filesystem. If we can use a flag like FS_REQUIRES_DEV
>>> that tells whether that filesystem will resolve dev_name argument, I think we can resolve
>>> the dev_name argument before security_mount_new() is called (and can avoid TOCTOU).
>>
I was expecting that "struct file_system_type"->fs_flags containing FS_REQUIRES_DEV
is a sign that the dev_name argument is a pathname. But it seems that such assumption
no longer holds true. For example, cramfs started treating dev_name like "mtd$num"
as if /dev/mtdblock$num since 4.15. So, current TOMOYO logic causes mount request of
cramfs with dev_name like "mtd0" to fail.
>> I guess we can add dev_path to fs_context?
>
> No, when and how the path is resolved is entirely up to the individual
> filesystem and we're not hoisting the block-based specific path lookup
> up into the VFS while leaving the other stuff per-filesystem. And it's
> not as trivial as you want it to be either.
Then, how can LSM modules know that how the requested filesystem resolves
the dev_name argument, without embedding filesystem specific resolution
logic into individual LSM module?
I want some flag like FS_REQUIRES_DEV that tells individual LSM module
whether the dev_name argument should be interpreted as a pathname.
^ permalink raw reply
* Re: [PATCH 6/7] tomoyo: Convert from sb_mount to granular mount hooks
From: Christian Brauner @ 2026-03-23 10:16 UTC (permalink / raw)
To: Song Liu
Cc: Tetsuo Handa, viro@zeniv.linux.org.uk, Song Liu,
paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com,
jack@suse.cz, john.johansen@canonical.com,
stephen.smalley.work@gmail.com, omosnace@redhat.com,
mic@digikod.net, gnoack@google.com, takedakn@nttdata.co.jp,
herton@canonical.com, Kernel Team, selinux@vger.kernel.org,
apparmor@lists.ubuntu.com, linux-fsdevel@vger.kernel.org,
linux-security-module@vger.kernel.org
In-Reply-To: <F0A0D13E-8208-49A4-9AC6-89AC4BF3F4FB@meta.com>
On Mon, Mar 23, 2026 at 03:32:14AM +0000, Song Liu wrote:
>
>
> > On Mar 22, 2026, at 3:46 AM, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
> >
> > On 2026/03/22 10:06, Song Liu wrote:
> >>>> @@ -70,6 +70,7 @@ static bool tomoyo_check_mount_acl(struct tomoyo_request_info *r,
> >>>> * @dir: Pointer to "struct path".
> >>>> * @type: Name of filesystem type.
> >>>> * @flags: Mount options.
> >>>> + * @dev_path: Pre-resolved device/source path. Maybe NULL.
> >>>
> >>> I guess that we can avoid passing maybe-NULL dev_name if Al Viro can accept
> >>> resolving maybe-NULL dev_path argument before calling LSM hooks.
> >>
> >> If I understand the code correctly, tomoyo records requested_dev_name for
> >> new mount. namespace.c, OTOH, does NOT do kern_path() for new mount. This
> >> is why we keep the maybe-NULL dev_name here. I personally think this is
> >> the best approach without changing tomoyo behavior.
> >
> > Well, namespace.c does kern_path() for new mount, but it happens a bit later after
> > security_mount_new() was called.
> >
> > do_new_mount_fc() => fc_mount() => vfs_get_tree() => fc->ops->get_tree() == e.g. ext4_get_tree()
> > => get_tree_bdev() => get_tree_bdev_flags() => lookup_bdev() => kern_path()
> >
> > @@ -3835,6 +3855,9 @@ static int do_new_mount(const struct path *path, const char *fstype,
> > err = parse_monolithic_mount_data(fc, data);
> > if (!err && !mount_capable(fc))
> > err = -EPERM;
> > +
> > + if (!err)
> > + err = security_mount_new(fc, path, mnt_flags, flags, data);
> > if (!err)
> > err = do_new_mount_fc(fc, path, mnt_flags);
> >
> >
> > Since not all filesystems need to resolve dev_name argument, conversion from dev_name
> > to "struct path" is up to individual filesystem. If we can use a flag like FS_REQUIRES_DEV
> > that tells whether that filesystem will resolve dev_name argument, I think we can resolve
> > the dev_name argument before security_mount_new() is called (and can avoid TOCTOU).
>
> I guess we can add dev_path to fs_context?
No, when and how the path is resolved is entirely up to the individual
filesystem and we're not hoisting the block-based specific path lookup
up into the VFS while leaving the other stuff per-filesystem. And it's
not as trivial as you want it to be either.
^ permalink raw reply
* [PATCH] KEYS: trusted: Protocol debugging as a feature
From: Jarkko Sakkinen @ 2026-03-23 9:00 UTC (permalink / raw)
To: linux-integrity
Cc: keyrings, Jarkko Sakkinen, Srish Srinivasan, Nayna Jain,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM,
open list
TPM_DEBUG is a non-standard way to specify a feature in Linux kernel.
Introduce CONFIG_TRUSTED_KEYS_DEBUG, and use it to replace TPM_DEBUG in
TPM 1.x trusted keys.
Given that protocol bus could contain sensitive data, harden the feature as
follows:
1. In the Kconfig description postulate that pr_debug() statements must be
used.
2. Use pr_debug() statements in TPM 1.x driver to print the protocol dump.
Traces can be enabled e.g., by providing trusted.dyndbg='+p' for the kernel
command-line.
Cc: Srish Srinivasan <ssrish@linux.ibm.com>
Reported-by: Nayna Jain <nayna@linux.ibm.com>
Closes: https://lore.kernel.org/all/7f8b8478-5cd8-4d97-bfd0-341fd5cf10f9@linux.ibm.com/
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
security/keys/trusted-keys/Kconfig | 10 +++++++
security/keys/trusted-keys/trusted_tpm1.c | 36 +++++++++++------------
2 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/security/keys/trusted-keys/Kconfig b/security/keys/trusted-keys/Kconfig
index 9e00482d886a..0e53bef1343d 100644
--- a/security/keys/trusted-keys/Kconfig
+++ b/security/keys/trusted-keys/Kconfig
@@ -1,6 +1,16 @@
config HAVE_TRUSTED_KEYS
bool
+config TRUSTED_KEYS_DEBUG
+ bool "Debug trusted keys protocol"
+ depends on HAVE_TRUSTED_KEYS
+ default n
+ help
+ Drivers that support debugging the protocol dump, can opt-in that
+ feature here. Protocol dump must only use DEBUG level output, as
+ sensitive data may pass by. In the kernel-command line traces can
+ be enabled via trusted.dyndbg='+p'.
+
config TRUSTED_KEYS_TPM
bool "TPM-based trusted keys"
depends on TCG_TPM >= TRUSTED_KEYS
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index c865c97aa1b4..8fe889c7cdd1 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -46,38 +46,36 @@ enum {
SRK_keytype = 4
};
-#define TPM_DEBUG 0
-
-#if TPM_DEBUG
+#ifdef CONFIG_TRUSTED_KEYS_DEBUG
static inline void dump_options(struct trusted_key_options *o)
{
- pr_info("sealing key type %d\n", o->keytype);
- pr_info("sealing key handle %0X\n", o->keyhandle);
- pr_info("pcrlock %d\n", o->pcrlock);
- pr_info("pcrinfo %d\n", o->pcrinfo_len);
- print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
- 16, 1, o->pcrinfo, o->pcrinfo_len, 0);
+ pr_debug("sealing key type %d\n", o->keytype);
+ pr_debug("sealing key handle %0X\n", o->keyhandle);
+ pr_debug("pcrlock %d\n", o->pcrlock);
+ pr_debug("pcrinfo %d\n", o->pcrinfo_len);
+ print_hex_dump_debug("pcrinfo ", DUMP_PREFIX_NONE,
+ 16, 1, o->pcrinfo, o->pcrinfo_len, 0);
}
static inline void dump_sess(struct osapsess *s)
{
- print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
- 16, 1, &s->handle, 4, 0);
- pr_info("secret:\n");
- print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
- 16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
- pr_info("trusted-key: enonce:\n");
- print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
- 16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
+ print_hex_dump_debug("trusted-key: handle ", DUMP_PREFIX_NONE,
+ 16, 1, &s->handle, 4, 0);
+ pr_debug("secret:\n");
+ print_hex_dump_debug("", DUMP_PREFIX_NONE,
+ 16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
+ pr_debug("trusted-key: enonce:\n");
+ print_hex_dump_debug("", DUMP_PREFIX_NONE,
+ 16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
}
static inline void dump_tpm_buf(unsigned char *buf)
{
int len;
- pr_info("\ntpm buffer\n");
+ pr_debug("\ntpm buffer\n");
len = LOAD32(buf, TPM_SIZE_OFFSET);
- print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
+ print_hex_dump_debug("", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
}
#else
static inline void dump_options(struct trusted_key_options *o)
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2026-03-23 5:46 UTC (permalink / raw)
To: Mimi Zohar
Cc: Chris Fenner, linux-integrity, Jonathan McDowell, Eric Biggers,
James Bottomley, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list, Roberto Sassu
In-Reply-To: <acDQ7_lXNhsvp8Nb@kernel.org>
On Mon, Mar 23, 2026 at 07:34:39AM +0200, Jarkko Sakkinen wrote:
> On Mon, Mar 23, 2026 at 07:26:38AM +0200, Jarkko Sakkinen wrote:
> > On Thu, Mar 19, 2026 at 10:28:03AM -0400, Mimi Zohar wrote:
> > > On Wed, 2026-03-18 at 10:36 -0700, Chris Fenner wrote:
> > > > Apologies if my long message derailed this discussion. I meant to
> > > > support Mimi's concern here and project a future vision where
> > > > TCG_TPM2_HMAC doesn't conflict with other features.
> > > >
> > > > More concisely, I think that:
> > > >
> > > > > tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled
> > > >
> > > > is not a compelling argument for removing TPM as an RNG source,
> > > > because TCG_TPM2_HMAC is known to have poor performance already
> > > > anyway.
> > >
> > > Agreed. Thanks, Chris! FYI, we raised concerns about IMA performance with the
> > > TPM HMAC and encrypted feature while it was being developed. James had some
> > > ideas, at the time, as to how to resolve the performance issue for IMA. Yet it
> > > was upstreamed without those changes and with CONFIG_TCG_TPM2_HMAC enabled by
> > > default on x86 systems.
> > >
> > > Jarkko has queued this patch in the "queue" branch, without indicating whether
> > > it will eventually be upstreamed or not.
> >
> > Yes and there's been multiple months of time to comment this and I
> > backed up the patch set there, which is not same as applying it.
>
> There's quite many other patches in that patch set also in the queue
> branch. This was largeriy past life for me when these comments came.
> Really don't understand what is suddenly going on tnh and for one
> not that interesting patch.
Underlined: not a queue to anywhere. I can rename it something else,
did not really think about the name when I created the branch.
BR, Jarkkko
^ permalink raw reply
* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2026-03-23 5:34 UTC (permalink / raw)
To: Mimi Zohar
Cc: Chris Fenner, linux-integrity, Jonathan McDowell, Eric Biggers,
James Bottomley, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list, Roberto Sassu
In-Reply-To: <acDPCRVx_xO_RhZK@kernel.org>
On Mon, Mar 23, 2026 at 07:26:38AM +0200, Jarkko Sakkinen wrote:
> On Thu, Mar 19, 2026 at 10:28:03AM -0400, Mimi Zohar wrote:
> > On Wed, 2026-03-18 at 10:36 -0700, Chris Fenner wrote:
> > > Apologies if my long message derailed this discussion. I meant to
> > > support Mimi's concern here and project a future vision where
> > > TCG_TPM2_HMAC doesn't conflict with other features.
> > >
> > > More concisely, I think that:
> > >
> > > > tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled
> > >
> > > is not a compelling argument for removing TPM as an RNG source,
> > > because TCG_TPM2_HMAC is known to have poor performance already
> > > anyway.
> >
> > Agreed. Thanks, Chris! FYI, we raised concerns about IMA performance with the
> > TPM HMAC and encrypted feature while it was being developed. James had some
> > ideas, at the time, as to how to resolve the performance issue for IMA. Yet it
> > was upstreamed without those changes and with CONFIG_TCG_TPM2_HMAC enabled by
> > default on x86 systems.
> >
> > Jarkko has queued this patch in the "queue" branch, without indicating whether
> > it will eventually be upstreamed or not.
>
> Yes and there's been multiple months of time to comment this and I
> backed up the patch set there, which is not same as applying it.
There's quite many other patches in that patch set also in the queue
branch. This was largeriy past life for me when these comments came.
Really don't understand what is suddenly going on tnh and for one
not that interesting patch.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend
From: Jarkko Sakkinen @ 2026-03-23 5:28 UTC (permalink / raw)
To: Srish Srinivasan
Cc: Nayna Jain, linux-integrity, keyrings, James.Bottomley, zohar,
stefanb, linux-kernel, linux-security-module
In-Reply-To: <21cc6859-1750-4c22-91bb-116620764ca9@linux.ibm.com>
On Tue, Mar 17, 2026 at 08:44:03AM +0530, Srish Srinivasan wrote:
>
> On 3/10/26 4:15 AM, Nayna Jain wrote:
> >
> > On 2/20/26 1:34 PM, Srish Srinivasan wrote:
> > > The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info()
> > > for logging debug information.
> > >
> > > Replace pr_info() with pr_debug(), and use KERN_DEBUG for
> > > print_hex_dump().
> > > Remove TPM_DEBUG.
> > >
> > > No functional change intended.
> > There is functional change here. This change allows secret and nonce in
> > the function dump_sess() to be logged to kernel logs when dynamic debug
> > is enabled. Previously, it was possible only in the debug builds and not
> > the production builds at runtime. With this change, it is always there
> > in production build. This can result in possible attack.
>
>
> Hi Jarkko,
> Could you please let us know your thoughts on this one?
>
> And Nayna,
> thanks for bringing this up.
Nayna is absolutely right so I dropped it.
Solution is debatable.
>
> thanks,
> Srish.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2026-03-23 5:26 UTC (permalink / raw)
To: Mimi Zohar
Cc: Chris Fenner, linux-integrity, Jonathan McDowell, Eric Biggers,
James Bottomley, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list, Roberto Sassu
In-Reply-To: <df0a54666d890d5223877e0a0f90ac9082b2d038.camel@linux.ibm.com>
On Thu, Mar 19, 2026 at 10:28:03AM -0400, Mimi Zohar wrote:
> On Wed, 2026-03-18 at 10:36 -0700, Chris Fenner wrote:
> > Apologies if my long message derailed this discussion. I meant to
> > support Mimi's concern here and project a future vision where
> > TCG_TPM2_HMAC doesn't conflict with other features.
> >
> > More concisely, I think that:
> >
> > > tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled
> >
> > is not a compelling argument for removing TPM as an RNG source,
> > because TCG_TPM2_HMAC is known to have poor performance already
> > anyway.
>
> Agreed. Thanks, Chris! FYI, we raised concerns about IMA performance with the
> TPM HMAC and encrypted feature while it was being developed. James had some
> ideas, at the time, as to how to resolve the performance issue for IMA. Yet it
> was upstreamed without those changes and with CONFIG_TCG_TPM2_HMAC enabled by
> default on x86 systems.
>
> Jarkko has queued this patch in the "queue" branch, without indicating whether
> it will eventually be upstreamed or not.
Yes and there's been multiple months of time to comment this and I
backed up the patch set there, which is not same as applying it.
>
> Mimi
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2026-03-23 5:24 UTC (permalink / raw)
To: Mimi Zohar
Cc: Chris Fenner, linux-integrity, Jonathan McDowell, Eric Biggers,
James Bottomley, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list, Roberto Sassu
In-Reply-To: <dca994cd0ed11a262d4022c4984984460ba06a78.camel@linux.ibm.com>
On Thu, Mar 05, 2026 at 10:37:08AM -0500, Mimi Zohar wrote:
> On Tue, 2026-03-03 at 23:32 +0200, Jarkko Sakkinen wrote:
> > On Fri, Feb 20, 2026 at 10:30:26AM -0800, Chris Fenner wrote:
> > > My conclusion about TCG_TPM2_HMAC after [1] and [2] was that
> > > TPM2_TCG_HMAC doesn't (or didn't at the time) actually solve the
> > > threat model it claims to (active interposer adversaries), while
> > > dramatically increasing the cost of many kernel TPM activities beyond
> > > the amount that would have been required to just solve for
> > > passive/bus-sniffer interposer adversaries. The added symmetric crypto
> > > required to secure a TPM transaction is almost not noticeable; the big
> > > performance problem is the re-bootstrapping of the session with ECDH
> > > for every command.
> > >
> > > My primary concern at that time was, essentially, that TCG_TPM2_HMAC
> > > punts on checking that the key that was used to secure the session was
> > > actually resident in a real TPM and not just an interposer adversary.
> > > I wrote up my understanding at
> > > https://www.dlp.rip/decorative-cryptography, for anyone who wants a
> > > long-form opinionated take :).
> > >
> > > Unless I'm wrong, or TCG_TPM2_HMAC has changed dramatically since
> > > August, I don't think "TPM2_TCG_HMAC makes this too costly" is a
> > > compelling reason to make a security decision. (There could be other
> > > reasons to make choices about whether to use the TPM as a source of
> > > randomness in the kernel! This just isn't one IMHO.)
> > >
> > > The version of TCG_TPM2_HMAC that I'd like to see someday would be one
> > > that fully admits that its threat model is only passive interposers,
> > > and sets up one session upon startup and ContextSaves/ContextLoads it
> > > back into the TPM as needed in order to secure parameter encryption
> > > for e.g., GetRandom() and Unseal() calls.
> >
> > Neither agreeing nor disagreeing but this patch set clearly does not
> > move forward and I spent already enough energy for this. For better
> > ideas the patches are available in queue branch.
>
> Jarkko, you totally ignored my comments below. I object to your removing the
> TPM trusted-keys RNG support.
It has not been removed but I can keep the patches still backed
up in a branch, can't I?
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend
From: Jarkko Sakkinen @ 2026-03-23 5:21 UTC (permalink / raw)
To: Nayna Jain
Cc: Srish Srinivasan, linux-integrity, keyrings, James.Bottomley,
zohar, stefanb, linux-kernel, linux-security-module
In-Reply-To: <acDM-hNRThVPRYhq@kernel.org>
On Mon, Mar 23, 2026 at 07:17:51AM +0200, Jarkko Sakkinen wrote:
> On Mon, Mar 09, 2026 at 06:45:23PM -0400, Nayna Jain wrote:
> >
> > On 2/20/26 1:34 PM, Srish Srinivasan wrote:
> > > The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info()
> > > for logging debug information.
> > >
> > > Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump().
> > > Remove TPM_DEBUG.
> > >
> > > No functional change intended.
> > There is functional change here. This change allows secret and nonce in the
> > function dump_sess() to be logged to kernel logs when dynamic debug is
> > enabled. Previously, it was possible only in the debug builds and not the
> > production builds at runtime. With this change, it is always there in
> > production build. This can result in possible attack.
>
> Good catch, thank you. It's in my master branch still (not in -next).
>
> TPM_DEBUG should be removed in all cases. If you really want to read
> a secret, use tracing tools.
>
> This only proves that the print should exist or should be a constant
> value, or overwritten same length value.
I dropped the current patches but yeah, a comment "do not touch this,
could be poisonous" won't be an acceptable way to address this.
If you want "some" debug information you can always put F-string or
0-string of same length, so there's options.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2 1/2] keys/trusted_keys: clean up debug message logging in the tpm backend
From: Jarkko Sakkinen @ 2026-03-23 5:17 UTC (permalink / raw)
To: Nayna Jain
Cc: Srish Srinivasan, linux-integrity, keyrings, James.Bottomley,
zohar, stefanb, linux-kernel, linux-security-module
In-Reply-To: <7f8b8478-5cd8-4d97-bfd0-341fd5cf10f9@linux.ibm.com>
On Mon, Mar 09, 2026 at 06:45:23PM -0400, Nayna Jain wrote:
>
> On 2/20/26 1:34 PM, Srish Srinivasan wrote:
> > The TPM trusted-keys backend uses a local TPM_DEBUG guard and pr_info()
> > for logging debug information.
> >
> > Replace pr_info() with pr_debug(), and use KERN_DEBUG for print_hex_dump().
> > Remove TPM_DEBUG.
> >
> > No functional change intended.
> There is functional change here. This change allows secret and nonce in the
> function dump_sess() to be logged to kernel logs when dynamic debug is
> enabled. Previously, it was possible only in the debug builds and not the
> production builds at runtime. With this change, it is always there in
> production build. This can result in possible attack.
Good catch, thank you. It's in my master branch still (not in -next).
TPM_DEBUG should be removed in all cases. If you really want to read
a secret, use tracing tools.
This only proves that the print should exist or should be a constant
value, or overwritten same length value.
> Instead of doing this change, I think add a comment to prevent this sort of
> change in the future.
>
> Thanks & Regards,
>
> - Nayna
>
> >
> > Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> > ---
> > security/keys/trusted-keys/trusted_tpm1.c | 40 +++++++----------------
> > 1 file changed, 12 insertions(+), 28 deletions(-)
> >
> > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > index c865c97aa1b4..216caef97ffc 100644
> > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > @@ -46,28 +46,25 @@ enum {
> > SRK_keytype = 4
> > };
> > -#define TPM_DEBUG 0
> > -
> > -#if TPM_DEBUG
> > static inline void dump_options(struct trusted_key_options *o)
> > {
> > - pr_info("sealing key type %d\n", o->keytype);
> > - pr_info("sealing key handle %0X\n", o->keyhandle);
> > - pr_info("pcrlock %d\n", o->pcrlock);
> > - pr_info("pcrinfo %d\n", o->pcrinfo_len);
> > - print_hex_dump(KERN_INFO, "pcrinfo ", DUMP_PREFIX_NONE,
> > + pr_debug("sealing key type %d\n", o->keytype);
> > + pr_debug("sealing key handle %0X\n", o->keyhandle);
> > + pr_debug("pcrlock %d\n", o->pcrlock);
> > + pr_debug("pcrinfo %d\n", o->pcrinfo_len);
> > + print_hex_dump(KERN_DEBUG, "pcrinfo ", DUMP_PREFIX_NONE,
> > 16, 1, o->pcrinfo, o->pcrinfo_len, 0);
> > }
> > static inline void dump_sess(struct osapsess *s)
> > {
> > - print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE,
> > + print_hex_dump(KERN_DEBUG, "trusted-key: handle ", DUMP_PREFIX_NONE,
> > 16, 1, &s->handle, 4, 0);
> > - pr_info("secret:\n");
> > - print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> > + pr_debug("secret:\n");
> > + print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
> > 16, 1, &s->secret, SHA1_DIGEST_SIZE, 0);
> > - pr_info("trusted-key: enonce:\n");
> > - print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE,
> > + pr_debug("trusted-key: enonce:\n");
> > + print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE,
> > 16, 1, &s->enonce, SHA1_DIGEST_SIZE, 0);
> > }
> > @@ -75,23 +72,10 @@ static inline void dump_tpm_buf(unsigned char *buf)
> > {
> > int len;
> > - pr_info("\ntpm buffer\n");
> > + pr_debug("\ntpm buffer\n");
> > len = LOAD32(buf, TPM_SIZE_OFFSET);
> > - print_hex_dump(KERN_INFO, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> > -}
> > -#else
> > -static inline void dump_options(struct trusted_key_options *o)
> > -{
> > -}
> > -
> > -static inline void dump_sess(struct osapsess *s)
> > -{
> > -}
> > -
> > -static inline void dump_tpm_buf(unsigned char *buf)
> > -{
> > + print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_NONE, 16, 1, buf, len, 0);
> > }
> > -#endif
> > static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
> > unsigned int keylen, ...)
BR, Jarkko
^ permalink raw reply
* [RFC PATCH v2 2/2] selinux: fix overlayfs mmap() and mprotect() access checks
From: Paul Moore @ 2026-03-23 4:24 UTC (permalink / raw)
To: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs
Cc: Amir Goldstein, Gao Xiang
In-Reply-To: <20260323042510.3331778-4-paul@paul-moore.com>
The existing SELinux security model for overlayfs is to allow access if
the current task is able to access the top level file (the "user" file)
and the mounter's credentials are sufficient to access the lower
level file (the "backing" file). Unfortunately, the current code does
not properly enforce these access controls for both mmap() and mprotect()
operations on overlayfs filesystems.
This patch makes use of the newly created security_mmap_backing_file()
LSM hook to provide the missing backing file enforcement for mmap()
operations, and leverages the backing file API and new LSM blob to
provide the necessary information to properly enforce the mprotect()
access controls.
Cc: stable@vger.kernel.org
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
security/selinux/hooks.c | 252 ++++++++++++++++++++++--------
security/selinux/include/objsec.h | 17 ++
2 files changed, 200 insertions(+), 69 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index d8224ea113d1..2a3d524dce24 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1745,6 +1745,60 @@ static inline int file_path_has_perm(const struct cred *cred,
static int bpf_fd_pass(const struct file *file, u32 sid);
#endif
+static int __file_has_perm(bool bf_user_file, const struct cred *cred,
+ const struct file *file, u32 av)
+
+{
+ struct common_audit_data ad;
+ struct inode *inode;
+ u32 ssid = cred_sid(cred);
+ u32 tsid_fd;
+ int rc;
+
+ if (bf_user_file) {
+ struct backing_file_security_struct *bfsec;
+ const struct path *path;
+
+ if (WARN_ON(!(file->f_mode & FMODE_BACKING)))
+ return -EPERM;
+
+ bfsec = selinux_backing_file(file);
+ path = backing_file_user_path(file);
+ tsid_fd = bfsec->uf_sid;
+ inode = d_inode(path->dentry);
+
+ ad.type = LSM_AUDIT_DATA_PATH;
+ ad.u.path = *path;
+ } else {
+ struct file_security_struct *fsec = selinux_file(file);
+
+ tsid_fd = fsec->sid;
+ inode = file_inode(file);
+
+ ad.type = LSM_AUDIT_DATA_FILE;
+ ad.u.file = file;
+ }
+
+ if (ssid != tsid_fd) {
+ rc = avc_has_perm(ssid, tsid_fd, SECCLASS_FD, FD__USE, &ad);
+ if (rc)
+ return rc;
+ }
+
+#ifdef CONFIG_BPF_SYSCALL
+ /* regardless of backing vs user file, use the underlying file here */
+ rc = bpf_fd_pass(file, ssid);
+ if (rc)
+ return rc;
+#endif
+
+ /* av is zero if only checking access to the descriptor. */
+ if (av)
+ return inode_has_perm(cred, inode, av, &ad);
+
+ return 0;
+}
+
/* Check whether a task can use an open file descriptor to
access an inode in a given way. Check access to the
descriptor itself, and then use dentry_has_perm to
@@ -1753,41 +1807,10 @@ static int bpf_fd_pass(const struct file *file, u32 sid);
has the same SID as the process. If av is zero, then
access to the file is not checked, e.g. for cases
where only the descriptor is affected like seek. */
-static int file_has_perm(const struct cred *cred,
- struct file *file,
- u32 av)
+static inline int file_has_perm(const struct cred *cred,
+ const struct file *file, u32 av)
{
- struct file_security_struct *fsec = selinux_file(file);
- struct inode *inode = file_inode(file);
- struct common_audit_data ad;
- u32 sid = cred_sid(cred);
- int rc;
-
- ad.type = LSM_AUDIT_DATA_FILE;
- ad.u.file = file;
-
- if (sid != fsec->sid) {
- rc = avc_has_perm(sid, fsec->sid,
- SECCLASS_FD,
- FD__USE,
- &ad);
- if (rc)
- goto out;
- }
-
-#ifdef CONFIG_BPF_SYSCALL
- rc = bpf_fd_pass(file, cred_sid(cred));
- if (rc)
- return rc;
-#endif
-
- /* av is zero if only checking access to the descriptor. */
- rc = 0;
- if (av)
- rc = inode_has_perm(cred, inode, av, &ad);
-
-out:
- return rc;
+ return __file_has_perm(false, cred, file, av);
}
/*
@@ -3825,6 +3848,17 @@ static int selinux_file_alloc_security(struct file *file)
return 0;
}
+static int selinux_backing_file_alloc(void *backing_file_blob,
+ const struct file *user_file)
+{
+ struct backing_file_security_struct *bfsec;
+
+ bfsec = selinux_backing_file_raw(backing_file_blob);
+ bfsec->uf_sid = selinux_file(user_file)->sid;
+
+ return 0;
+}
+
/*
* Check whether a task has the ioctl permission and cmd
* operation to an inode.
@@ -3942,42 +3976,53 @@ static int selinux_file_ioctl_compat(struct file *file, unsigned int cmd,
static int default_noexec __ro_after_init;
-static int file_map_prot_check(struct file *file, unsigned long prot, int shared)
+static int __file_map_prot_check(bool bf_user_file, const struct cred *cred,
+ const struct file *file, unsigned long prot,
+ bool shared)
{
- const struct cred *cred = current_cred();
- u32 sid = cred_sid(cred);
- int rc = 0;
+ struct inode *inode = NULL;
+
+ if (file) {
+ if (bf_user_file)
+ inode = d_inode(backing_file_user_path(file)->dentry);
+ else
+ inode = file_inode(file);
+ }
+
+ if (default_noexec && (prot & PROT_EXEC) &&
+ (!file || IS_PRIVATE(inode) || (!shared && (prot & PROT_WRITE)))) {
+ int rc;
+ u32 sid = cred_sid(cred);
- if (default_noexec &&
- (prot & PROT_EXEC) && (!file || IS_PRIVATE(file_inode(file)) ||
- (!shared && (prot & PROT_WRITE)))) {
/*
- * We are making executable an anonymous mapping or a
- * private file mapping that will also be writable.
- * This has an additional check.
+ * We are making executable an anonymous mapping or a private
+ * file mapping that will also be writable.
*/
- rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
- PROCESS__EXECMEM, NULL);
+ rc = avc_has_perm(sid, sid, SECCLASS_PROCESS, PROCESS__EXECMEM,
+ NULL);
if (rc)
- goto error;
+ return rc;
}
if (file) {
- /* read access is always possible with a mapping */
+ /* "read" always possible, "write" only if shared */
u32 av = FILE__READ;
-
- /* write access only matters if the mapping is shared */
if (shared && (prot & PROT_WRITE))
av |= FILE__WRITE;
-
if (prot & PROT_EXEC)
av |= FILE__EXECUTE;
- return file_has_perm(cred, file, av);
+ return __file_has_perm(bf_user_file, cred, file, av);
}
-error:
- return rc;
+ return 0;
+}
+
+static inline int file_map_prot_check(const struct cred *cred,
+ const struct file *file,
+ unsigned long prot, bool shared)
+{
+ return __file_map_prot_check(false, cred, file, prot, shared);
}
static int selinux_mmap_addr(unsigned long addr)
@@ -3993,36 +4038,80 @@ static int selinux_mmap_addr(unsigned long addr)
return rc;
}
-static int selinux_mmap_file(struct file *file,
- unsigned long reqprot __always_unused,
- unsigned long prot, unsigned long flags)
+static int selinux_mmap_file_common(const struct cred *cred, struct file *file,
+ unsigned long prot, bool shared)
{
- struct common_audit_data ad;
- int rc;
-
if (file) {
+ int rc;
+ struct common_audit_data ad;
+
ad.type = LSM_AUDIT_DATA_FILE;
ad.u.file = file;
- rc = inode_has_perm(current_cred(), file_inode(file),
- FILE__MAP, &ad);
+ rc = inode_has_perm(cred, file_inode(file), FILE__MAP, &ad);
if (rc)
return rc;
}
- return file_map_prot_check(file, prot,
- (flags & MAP_TYPE) == MAP_SHARED);
+ return file_map_prot_check(cred, file, prot, shared);
+}
+
+static int selinux_mmap_file(struct file *file,
+ unsigned long reqprot __always_unused,
+ unsigned long prot, unsigned long flags)
+{
+ return selinux_mmap_file_common(current_cred(), file, prot,
+ (flags & MAP_TYPE) == MAP_SHARED);
+}
+
+/**
+ * selinux_mmap_backing_file - Check mmap permissions on a backing file
+ * @vma: memory region
+ * @backing_file: stacked filesystem backing file
+ * @user_file: user visible file
+ *
+ * This is called after selinux_mmap_file() on stacked filesystems, and it
+ * is this function's responsibility to verify access to @backing_file and
+ * setup the SELinux state for possible later use in the mprotect() code path.
+ *
+ * By the time this function is called, mmap() access to @user_file has already
+ * been authorized and @vma->vm_file has been set to point to @backing_file.
+ *
+ * Return zero on success, negative values otherwise.
+ */
+static int selinux_mmap_backing_file(struct vm_area_struct *vma,
+ struct file *backing_file,
+ struct file *user_file __always_unused)
+{
+ unsigned long prot = 0;
+
+ /* translate vma->vm_flags perms into PROT perms */
+ if (vma->vm_flags & VM_READ)
+ prot |= PROT_READ;
+ if (vma->vm_flags & VM_WRITE)
+ prot |= PROT_WRITE;
+ if (vma->vm_flags & VM_EXEC)
+ prot |= PROT_EXEC;
+
+ return selinux_mmap_file_common(backing_file->f_cred, backing_file,
+ prot, vma->vm_flags & VM_SHARED);
}
static int selinux_file_mprotect(struct vm_area_struct *vma,
unsigned long reqprot __always_unused,
unsigned long prot)
{
+ int rc;
const struct cred *cred = current_cred();
u32 sid = cred_sid(cred);
+ const struct file *file = vma->vm_file;
+ bool backing_file = false;
+
+ /* check if we need to trigger the "backing files are awful" mode */
+ if (file && (file->f_mode & FMODE_BACKING))
+ backing_file = true;
if (default_noexec &&
(prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
- int rc = 0;
/*
* We don't use the vma_is_initial_heap() helper as it has
* a history of problems and is currently broken on systems
@@ -4036,11 +4125,15 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
vma->vm_end <= vma->vm_mm->brk) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECHEAP, NULL);
- } else if (!vma->vm_file && (vma_is_initial_stack(vma) ||
+ if (rc)
+ return rc;
+ } else if (!file && (vma_is_initial_stack(vma) ||
vma_is_stack_for_current(vma))) {
rc = avc_has_perm(sid, sid, SECCLASS_PROCESS,
PROCESS__EXECSTACK, NULL);
- } else if (vma->vm_file && vma->anon_vma) {
+ if (rc)
+ return rc;
+ } else if (file && vma->anon_vma) {
/*
* We are making executable a file mapping that has
* had some COW done. Since pages might have been
@@ -4048,13 +4141,31 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
* modified content. This typically should only
* occur for text relocations.
*/
- rc = file_has_perm(cred, vma->vm_file, FILE__EXECMOD);
+ rc = __file_has_perm(backing_file, cred, file,
+ FILE__EXECMOD);
+ if (rc)
+ return rc;
+ if (backing_file) {
+ rc = file_has_perm(file->f_cred, file,
+ FILE__EXECMOD);
+ if (rc)
+ return rc;
+ }
}
+ }
+
+ rc = __file_map_prot_check(backing_file, cred, file, prot,
+ vma->vm_flags & VM_SHARED);
+ if (rc)
+ return rc;
+ if (backing_file) {
+ rc = file_map_prot_check(file->f_cred, file, prot,
+ vma->vm_flags & VM_SHARED);
if (rc)
return rc;
}
- return file_map_prot_check(vma->vm_file, prot, vma->vm_flags&VM_SHARED);
+ return 0;
}
static int selinux_file_lock(struct file *file, unsigned int cmd)
@@ -7393,6 +7504,7 @@ struct lsm_blob_sizes selinux_blob_sizes __ro_after_init = {
.lbs_cred = sizeof(struct cred_security_struct),
.lbs_task = sizeof(struct task_security_struct),
.lbs_file = sizeof(struct file_security_struct),
+ .lbs_backing_file = sizeof(struct backing_file_security_struct),
.lbs_inode = sizeof(struct inode_security_struct),
.lbs_ipc = sizeof(struct ipc_security_struct),
.lbs_key = sizeof(struct key_security_struct),
@@ -7498,9 +7610,11 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
LSM_HOOK_INIT(file_permission, selinux_file_permission),
LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
+ LSM_HOOK_INIT(backing_file_alloc, selinux_backing_file_alloc),
LSM_HOOK_INIT(file_ioctl, selinux_file_ioctl),
LSM_HOOK_INIT(file_ioctl_compat, selinux_file_ioctl_compat),
LSM_HOOK_INIT(mmap_file, selinux_mmap_file),
+ LSM_HOOK_INIT(mmap_backing_file, selinux_mmap_backing_file),
LSM_HOOK_INIT(mmap_addr, selinux_mmap_addr),
LSM_HOOK_INIT(file_mprotect, selinux_file_mprotect),
LSM_HOOK_INIT(file_lock, selinux_file_lock),
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 5bddd28ea5cb..8ec493064aa2 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -88,6 +88,10 @@ struct file_security_struct {
u32 pseqno; /* Policy seqno at the time of file open */
};
+struct backing_file_security_struct {
+ u32 uf_sid; /* associated user file fsec->sid */
+};
+
struct superblock_security_struct {
u32 sid; /* SID of file system superblock */
u32 def_sid; /* default SID for labeling */
@@ -195,6 +199,19 @@ static inline struct file_security_struct *selinux_file(const struct file *file)
return file->f_security + selinux_blob_sizes.lbs_file;
}
+static inline struct backing_file_security_struct *
+selinux_backing_file_raw(void *blob)
+{
+ return blob + selinux_blob_sizes.lbs_backing_file;
+}
+
+static inline struct backing_file_security_struct *
+selinux_backing_file(const struct file *backing_file)
+{
+ void *blob = backing_file_security(backing_file);
+ return selinux_backing_file_raw(blob);
+}
+
static inline struct inode_security_struct *
selinux_inode(const struct inode *inode)
{
--
2.53.0
^ permalink raw reply related
* [RFC PATCH v2 1/2] lsm: add backing_file LSM hooks
From: Paul Moore @ 2026-03-23 4:24 UTC (permalink / raw)
To: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs
Cc: Amir Goldstein, Gao Xiang
In-Reply-To: <20260323042510.3331778-4-paul@paul-moore.com>
Stacked filesystems such as overlayfs do not currently provide the
necessary mechanisms for LSMs to properly enforce access controls on the
mmap() and mprotect() operations. In order to resolve this gap, a LSM
security blob is being added to the backing_file struct and the following
new LSM hooks are being created:
security_backing_file_alloc()
security_backing_file_free()
security_mmap_backing_file()
The first two hooks are to manage the lifecycle of the LSM security blob
in the backing_file struct, while the third provides a new mmap() access
control point for the underlying backing file. It is also expected that
LSMs will likely want to update their security_file_mprotect() callback
to address issues with their mprotect() controls, but that does not
require a change to the security_file_mprotect() LSM hook.
There are a two other small changes to support these new LSM hooks. We
pass the user file associated with a backing file down to
alloc_empty_backing_file() so it can be included in the
security_backing_file_alloc() hook, and we constify the file struct field
in the LSM common_audit_data struct to better support LSMs that need to
pass a const file struct pointer into the common LSM audit code.
Thanks to Arnd Bergmann for identifying the missing EXPORT_SYMBOL_GPL()
and supplying a fixup.
Cc: stable@vger.kernel.org
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
fs/backing-file.c | 18 ++++--
fs/erofs/ishare.c | 10 +++-
fs/file_table.c | 21 ++++++-
fs/fuse/passthrough.c | 2 +-
fs/internal.h | 3 +-
fs/overlayfs/dir.c | 2 +-
fs/overlayfs/file.c | 2 +-
include/linux/backing-file.h | 4 +-
include/linux/fs.h | 1 +
include/linux/lsm_audit.h | 2 +-
include/linux/lsm_hook_defs.h | 5 ++
include/linux/lsm_hooks.h | 1 +
include/linux/security.h | 22 ++++++++
security/lsm.h | 1 +
security/lsm_init.c | 9 +++
security/security.c | 100 ++++++++++++++++++++++++++++++++++
16 files changed, 187 insertions(+), 16 deletions(-)
diff --git a/fs/backing-file.c b/fs/backing-file.c
index 45da8600d564..1f3bbfc75882 100644
--- a/fs/backing-file.c
+++ b/fs/backing-file.c
@@ -12,6 +12,7 @@
#include <linux/backing-file.h>
#include <linux/splice.h>
#include <linux/mm.h>
+#include <linux/security.h>
#include "internal.h"
@@ -29,14 +30,15 @@
* returned file into a container structure that also stores the stacked
* file's path, which can be retrieved using backing_file_user_path().
*/
-struct file *backing_file_open(const struct path *user_path, int flags,
+struct file *backing_file_open(const struct file *user_file, int flags,
const struct path *real_path,
const struct cred *cred)
{
+ const struct path *user_path = &user_file->f_path;
struct file *f;
int error;
- f = alloc_empty_backing_file(flags, cred);
+ f = alloc_empty_backing_file(flags, cred, user_file);
if (IS_ERR(f))
return f;
@@ -52,15 +54,16 @@ struct file *backing_file_open(const struct path *user_path, int flags,
}
EXPORT_SYMBOL_GPL(backing_file_open);
-struct file *backing_tmpfile_open(const struct path *user_path, int flags,
+struct file *backing_tmpfile_open(const struct file *user_file, int flags,
const struct path *real_parentpath,
umode_t mode, const struct cred *cred)
{
struct mnt_idmap *real_idmap = mnt_idmap(real_parentpath->mnt);
+ const struct path *user_path = &user_file->f_path;
struct file *f;
int error;
- f = alloc_empty_backing_file(flags, cred);
+ f = alloc_empty_backing_file(flags, cred, user_file);
if (IS_ERR(f))
return f;
@@ -336,8 +339,13 @@ int backing_file_mmap(struct file *file, struct vm_area_struct *vma,
vma_set_file(vma, file);
- scoped_with_creds(ctx->cred)
+ scoped_with_creds(ctx->cred) {
+ ret = security_mmap_backing_file(vma, file, user_file);
+ if (ret)
+ return ret;
+
ret = vfs_mmap(vma->vm_file, vma);
+ }
if (ctx->accessed)
ctx->accessed(user_file);
diff --git a/fs/erofs/ishare.c b/fs/erofs/ishare.c
index 829d50d5c717..ec3fc5ac1a55 100644
--- a/fs/erofs/ishare.c
+++ b/fs/erofs/ishare.c
@@ -4,6 +4,7 @@
*/
#include <linux/xxhash.h>
#include <linux/mount.h>
+#include <linux/security.h>
#include "internal.h"
#include "xattr.h"
@@ -106,7 +107,8 @@ static int erofs_ishare_file_open(struct inode *inode, struct file *file)
if (file->f_flags & O_DIRECT)
return -EINVAL;
- realfile = alloc_empty_backing_file(O_RDONLY|O_NOATIME, current_cred());
+ realfile = alloc_empty_backing_file(O_RDONLY|O_NOATIME, current_cred(),
+ file);
if (IS_ERR(realfile))
return PTR_ERR(realfile);
ihold(sharedinode);
@@ -150,8 +152,14 @@ static ssize_t erofs_ishare_file_read_iter(struct kiocb *iocb,
static int erofs_ishare_mmap(struct file *file, struct vm_area_struct *vma)
{
struct file *realfile = file->private_data;
+ int err;
vma_set_file(vma, realfile);
+
+ err = security_mmap_backing_file(vma, realfile, file);
+ if (err)
+ return err;
+
return generic_file_readonly_mmap(file, vma);
}
diff --git a/fs/file_table.c b/fs/file_table.c
index aaa5faaace1e..0bdc26cae138 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -50,6 +50,7 @@ struct backing_file {
struct path user_path;
freeptr_t bf_freeptr;
};
+ void *security;
};
#define backing_file(f) container_of(f, struct backing_file, file)
@@ -66,6 +67,11 @@ void backing_file_set_user_path(struct file *f, const struct path *path)
}
EXPORT_SYMBOL_GPL(backing_file_set_user_path);
+void *backing_file_security(const struct file *f)
+{
+ return backing_file(f)->security;
+}
+
static inline void file_free(struct file *f)
{
security_file_free(f);
@@ -73,8 +79,11 @@ static inline void file_free(struct file *f)
percpu_counter_dec(&nr_files);
put_cred(f->f_cred);
if (unlikely(f->f_mode & FMODE_BACKING)) {
- path_put(backing_file_user_path(f));
- kmem_cache_free(bfilp_cachep, backing_file(f));
+ struct backing_file *ff = backing_file(f);
+
+ security_backing_file_free(&ff->security);
+ path_put(&ff->user_path);
+ kmem_cache_free(bfilp_cachep, ff);
} else {
kmem_cache_free(filp_cachep, f);
}
@@ -290,7 +299,8 @@ struct file *alloc_empty_file_noaccount(int flags, const struct cred *cred)
* This is only for kernel internal use, and the allocate file must not be
* installed into file tables or such.
*/
-struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
+struct file *alloc_empty_backing_file(int flags, const struct cred *cred,
+ const struct file *user_file)
{
struct backing_file *ff;
int error;
@@ -306,6 +316,11 @@ struct file *alloc_empty_backing_file(int flags, const struct cred *cred)
}
ff->file.f_mode |= FMODE_BACKING | FMODE_NOACCOUNT;
+ error = security_backing_file_alloc(&ff->security, user_file);
+ if (unlikely(error)) {
+ fput(&ff->file);
+ return ERR_PTR(error);
+ }
return &ff->file;
}
EXPORT_SYMBOL_GPL(alloc_empty_backing_file);
diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
index 72de97c03d0e..f2d08ac2459b 100644
--- a/fs/fuse/passthrough.c
+++ b/fs/fuse/passthrough.c
@@ -167,7 +167,7 @@ struct fuse_backing *fuse_passthrough_open(struct file *file, int backing_id)
goto out;
/* Allocate backing file per fuse file to store fuse path */
- backing_file = backing_file_open(&file->f_path, file->f_flags,
+ backing_file = backing_file_open(file, file->f_flags,
&fb->file->f_path, fb->cred);
err = PTR_ERR(backing_file);
if (IS_ERR(backing_file)) {
diff --git a/fs/internal.h b/fs/internal.h
index cbc384a1aa09..77e90e4124e0 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -106,7 +106,8 @@ extern void chroot_fs_refs(const struct path *, const struct path *);
*/
struct file *alloc_empty_file(int flags, const struct cred *cred);
struct file *alloc_empty_file_noaccount(int flags, const struct cred *cred);
-struct file *alloc_empty_backing_file(int flags, const struct cred *cred);
+struct file *alloc_empty_backing_file(int flags, const struct cred *cred,
+ const struct file *user_file);
void backing_file_set_user_path(struct file *f, const struct path *path);
static inline void file_put_write_access(struct file *file)
diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index ff3dbd1ca61f..f2f20a611af3 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -1374,7 +1374,7 @@ static int ovl_create_tmpfile(struct file *file, struct dentry *dentry,
return PTR_ERR(cred);
ovl_path_upper(dentry->d_parent, &realparentpath);
- realfile = backing_tmpfile_open(&file->f_path, flags, &realparentpath,
+ realfile = backing_tmpfile_open(file, flags, &realparentpath,
mode, current_cred());
err = PTR_ERR_OR_ZERO(realfile);
pr_debug("tmpfile/open(%pd2, 0%o) = %i\n", realparentpath.dentry, mode, err);
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 97bed2286030..27cc07738f33 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -48,7 +48,7 @@ static struct file *ovl_open_realfile(const struct file *file,
if (!inode_owner_or_capable(real_idmap, realinode))
flags &= ~O_NOATIME;
- realfile = backing_file_open(file_user_path(file),
+ realfile = backing_file_open(file,
flags, realpath, current_cred());
}
}
diff --git a/include/linux/backing-file.h b/include/linux/backing-file.h
index 1476a6ed1bfd..c939cd222730 100644
--- a/include/linux/backing-file.h
+++ b/include/linux/backing-file.h
@@ -18,10 +18,10 @@ struct backing_file_ctx {
void (*end_write)(struct kiocb *iocb, ssize_t);
};
-struct file *backing_file_open(const struct path *user_path, int flags,
+struct file *backing_file_open(const struct file *user_file, int flags,
const struct path *real_path,
const struct cred *cred);
-struct file *backing_tmpfile_open(const struct path *user_path, int flags,
+struct file *backing_tmpfile_open(const struct file *user_file, int flags,
const struct path *real_parentpath,
umode_t mode, const struct cred *cred);
ssize_t backing_file_read_iter(struct file *file, struct iov_iter *iter,
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8b3dd145b25e..8f5702cfb5e0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2474,6 +2474,7 @@ struct file *dentry_open_nonotify(const struct path *path, int flags,
struct file *dentry_create(struct path *path, int flags, umode_t mode,
const struct cred *cred);
const struct path *backing_file_user_path(const struct file *f);
+void *backing_file_security(const struct file *f);
/*
* When mmapping a file on a stackable filesystem (e.g., overlayfs), the file
diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
index 382c56a97bba..584db296e43b 100644
--- a/include/linux/lsm_audit.h
+++ b/include/linux/lsm_audit.h
@@ -94,7 +94,7 @@ struct common_audit_data {
#endif
char *kmod_name;
struct lsm_ioctlop_audit *op;
- struct file *file;
+ const struct file *file;
struct lsm_ibpkey_audit *ibpkey;
struct lsm_ibendport_audit *ibendport;
int reason;
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 8c42b4bde09c..2c4da40757ad 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -191,6 +191,9 @@ LSM_HOOK(int, 0, file_permission, struct file *file, int mask)
LSM_HOOK(int, 0, file_alloc_security, struct file *file)
LSM_HOOK(void, LSM_RET_VOID, file_release, struct file *file)
LSM_HOOK(void, LSM_RET_VOID, file_free_security, struct file *file)
+LSM_HOOK(int, 0, backing_file_alloc, void *backing_file_blobp,
+ const struct file *user_file)
+LSM_HOOK(void, LSM_RET_VOID, backing_file_free, void *backing_file_blobp)
LSM_HOOK(int, 0, file_ioctl, struct file *file, unsigned int cmd,
unsigned long arg)
LSM_HOOK(int, 0, file_ioctl_compat, struct file *file, unsigned int cmd,
@@ -198,6 +201,8 @@ LSM_HOOK(int, 0, file_ioctl_compat, struct file *file, unsigned int cmd,
LSM_HOOK(int, 0, mmap_addr, unsigned long addr)
LSM_HOOK(int, 0, mmap_file, struct file *file, unsigned long reqprot,
unsigned long prot, unsigned long flags)
+LSM_HOOK(int, 0, mmap_backing_file, struct vm_area_struct *vma,
+ struct file *backing_file, struct file *user_file)
LSM_HOOK(int, 0, file_mprotect, struct vm_area_struct *vma,
unsigned long reqprot, unsigned long prot)
LSM_HOOK(int, 0, file_lock, struct file *file, unsigned int cmd)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index d48bf0ad26f4..b4f8cad53ddb 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -104,6 +104,7 @@ struct security_hook_list {
struct lsm_blob_sizes {
unsigned int lbs_cred;
unsigned int lbs_file;
+ unsigned int lbs_backing_file;
unsigned int lbs_ib;
unsigned int lbs_inode;
unsigned int lbs_sock;
diff --git a/include/linux/security.h b/include/linux/security.h
index 83a646d72f6f..1e4c68d5877f 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -471,11 +471,17 @@ int security_file_permission(struct file *file, int mask);
int security_file_alloc(struct file *file);
void security_file_release(struct file *file);
void security_file_free(struct file *file);
+int security_backing_file_alloc(void **backing_file_blobp,
+ const struct file *user_file);
+void security_backing_file_free(void **backing_file_blobp);
int security_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
int security_file_ioctl_compat(struct file *file, unsigned int cmd,
unsigned long arg);
int security_mmap_file(struct file *file, unsigned long prot,
unsigned long flags);
+int security_mmap_backing_file(struct vm_area_struct *vma,
+ struct file *backing_file,
+ struct file *user_file);
int security_mmap_addr(unsigned long addr);
int security_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
unsigned long prot);
@@ -1140,6 +1146,15 @@ static inline void security_file_release(struct file *file)
static inline void security_file_free(struct file *file)
{ }
+int security_backing_file_alloc(void **backing_file_blobp,
+ const struct file *user_file)
+{
+ return 0;
+}
+
+void security_backing_file_free(void **backing_file_blobp)
+{ }
+
static inline int security_file_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
@@ -1159,6 +1174,13 @@ static inline int security_mmap_file(struct file *file, unsigned long prot,
return 0;
}
+static inline int security_mmap_backing_file(struct vm_area_struct *vma,
+ struct file *backing_file,
+ struct file *user_file)
+{
+ return 0;
+}
+
static inline int security_mmap_addr(unsigned long addr)
{
return cap_mmap_addr(addr);
diff --git a/security/lsm.h b/security/lsm.h
index db77cc83e158..32f808ad4335 100644
--- a/security/lsm.h
+++ b/security/lsm.h
@@ -29,6 +29,7 @@ extern struct lsm_blob_sizes blob_sizes;
/* LSM blob caches */
extern struct kmem_cache *lsm_file_cache;
+extern struct kmem_cache *lsm_backing_file_cache;
extern struct kmem_cache *lsm_inode_cache;
/* LSM blob allocators */
diff --git a/security/lsm_init.c b/security/lsm_init.c
index 573e2a7250c4..020eace65973 100644
--- a/security/lsm_init.c
+++ b/security/lsm_init.c
@@ -293,6 +293,8 @@ static void __init lsm_prepare(struct lsm_info *lsm)
blobs = lsm->blobs;
lsm_blob_size_update(&blobs->lbs_cred, &blob_sizes.lbs_cred);
lsm_blob_size_update(&blobs->lbs_file, &blob_sizes.lbs_file);
+ lsm_blob_size_update(&blobs->lbs_backing_file,
+ &blob_sizes.lbs_backing_file);
lsm_blob_size_update(&blobs->lbs_ib, &blob_sizes.lbs_ib);
/* inode blob gets an rcu_head in addition to LSM blobs. */
if (blobs->lbs_inode && blob_sizes.lbs_inode == 0)
@@ -441,6 +443,8 @@ int __init security_init(void)
if (lsm_debug) {
lsm_pr("blob(cred) size %d\n", blob_sizes.lbs_cred);
lsm_pr("blob(file) size %d\n", blob_sizes.lbs_file);
+ lsm_pr("blob(backing_file) size %d\n",
+ blob_sizes.lbs_backing_file);
lsm_pr("blob(ib) size %d\n", blob_sizes.lbs_ib);
lsm_pr("blob(inode) size %d\n", blob_sizes.lbs_inode);
lsm_pr("blob(ipc) size %d\n", blob_sizes.lbs_ipc);
@@ -462,6 +466,11 @@ int __init security_init(void)
lsm_file_cache = kmem_cache_create("lsm_file_cache",
blob_sizes.lbs_file, 0,
SLAB_PANIC, NULL);
+ if (blob_sizes.lbs_backing_file)
+ lsm_backing_file_cache = kmem_cache_create(
+ "lsm_backing_file_cache",
+ blob_sizes.lbs_file, 0,
+ SLAB_PANIC, NULL);
if (blob_sizes.lbs_inode)
lsm_inode_cache = kmem_cache_create("lsm_inode_cache",
blob_sizes.lbs_inode, 0,
diff --git a/security/security.c b/security/security.c
index 67af9228c4e9..651a0d643c9f 100644
--- a/security/security.c
+++ b/security/security.c
@@ -81,6 +81,7 @@ const struct lsm_id *lsm_idlist[MAX_LSM_COUNT];
struct lsm_blob_sizes blob_sizes;
struct kmem_cache *lsm_file_cache;
+struct kmem_cache *lsm_backing_file_cache;
struct kmem_cache *lsm_inode_cache;
#define SECURITY_HOOK_ACTIVE_KEY(HOOK, IDX) security_hook_active_##HOOK##_##IDX
@@ -172,6 +173,28 @@ static int lsm_file_alloc(struct file *file)
return 0;
}
+/**
+ * lsm_backing_file_alloc - allocate a composite backing file blob
+ * @backing_file_blobp: pointer to the backing file LSM blob pointer
+ *
+ * Allocate the backing file blob for all the modules.
+ *
+ * Returns 0, or -ENOMEM if memory can't be allocated.
+ */
+static int lsm_backing_file_alloc(void **backing_file_blobp)
+{
+ if (!lsm_backing_file_cache) {
+ *backing_file_blobp = NULL;
+ return 0;
+ }
+
+ *backing_file_blobp = kmem_cache_zalloc(lsm_backing_file_cache,
+ GFP_KERNEL);
+ if (*backing_file_blobp == NULL)
+ return -ENOMEM;
+ return 0;
+}
+
/**
* lsm_blob_alloc - allocate a composite blob
* @dest: the destination for the blob
@@ -2417,6 +2440,57 @@ void security_file_free(struct file *file)
}
}
+/**
+ * security_backing_file_alloc() - Allocate and setup a backing file blob
+ * @backing_file_blobp: pointer to the backing file LSM blob pointer
+ * @user_file: the associated user visible file
+ *
+ * Allocate a backing file LSM blob and perform any necessary initialization of
+ * the LSM blob. There will be some operations where the LSM will not have
+ * access to @user_file after this point, so any important state associated
+ * with @user_file that is important to the LSM should be captured in the
+ * backing file's LSM blob.
+ *
+ * LSM's should avoid taking a reference to @user_file in this hook as it will
+ * result in problems later when the system attempts to drop/put the file
+ * references due to a circular dependency.
+ *
+ * Return: Return 0 if the hook is successful, negative values otherwise.
+ */
+int security_backing_file_alloc(void **backing_file_blobp,
+ const struct file *user_file)
+{
+ int rc;
+
+ rc = lsm_backing_file_alloc(backing_file_blobp);
+ if (rc)
+ return rc;
+ rc = call_int_hook(backing_file_alloc, *backing_file_blobp, user_file);
+ if (unlikely(rc))
+ security_backing_file_free(backing_file_blobp);
+
+ return rc;
+}
+
+/**
+ * security_backing_file_free() - Free a backing file blob
+ * @backing_file_blobp: pointer to the backing file LSM blob pointer
+ *
+ * Free any LSM state associate with a backing file's LSM blob, including the
+ * blob itself.
+ */
+void security_backing_file_free(void **backing_file_blobp)
+{
+ void *backing_file_blob = *backing_file_blobp;
+
+ call_void_hook(backing_file_free, backing_file_blob);
+
+ if (backing_file_blob) {
+ *backing_file_blobp = NULL;
+ kmem_cache_free(lsm_backing_file_cache, backing_file_blob);
+ }
+}
+
/**
* security_file_ioctl() - Check if an ioctl is allowed
* @file: associated file
@@ -2505,6 +2579,32 @@ int security_mmap_file(struct file *file, unsigned long prot,
flags);
}
+/**
+ * security_mmap_backing_file - Check if mmap'ing a backing file is allowed
+ * @vma: the vm_area_struct for the mmap'd region
+ * @backing_file: the backing file being mmap'd
+ * @user_file: the user file being mmap'd
+ *
+ * Check permissions for a mmap operation on a stacked filesystem. This hook
+ * is called after the security_mmap_file() and is responsible for authorizing
+ * the mmap on @backing_file. It is important to note that the mmap operation
+ * on @user_file has already been authorized and the @vma->vm_file has been
+ * set to @backing_file.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_mmap_backing_file(struct vm_area_struct *vma,
+ struct file *backing_file,
+ struct file *user_file)
+{
+ /* recommended by the stackable filesystem devs */
+ if (WARN_ON_ONCE(!(backing_file->f_mode & FMODE_BACKING)))
+ return -EIO;
+
+ return call_int_hook(mmap_backing_file, vma, backing_file, user_file);
+}
+EXPORT_SYMBOL_GPL(security_mmap_backing_file);
+
/**
* security_mmap_addr() - Check if mmap'ing an address is allowed
* @addr: address
--
2.53.0
^ permalink raw reply related
* [RFC PATCH v2 0/2] Fix incorrect overlayfs mmap() and mprotect() LSM access controls
From: Paul Moore @ 2026-03-23 4:24 UTC (permalink / raw)
To: linux-security-module, selinux, linux-fsdevel, linux-unionfs,
linux-erofs
Cc: Amir Goldstein, Gao Xiang
This is a follow-up revision to the patchset[1] posted a week ago. This
second version has changed significantly in terms of approach and
implementation, as it has become clear that the overlayfs/VFS devs are
unable to make the user O_PATH file approach work. Unfortunately, this
pushes a lot of the complexity down into the LSM, as opposed to the
backing file code, and will likely result in code and state duplication
across the different LSMs, but at this point in time it doesn't appear
we have any other options.
I'm marking this patchset as a RFC since I've only done basic testing
on this patchset, and I still haven't satisfied myself that the code
covers all of the different cases. Additional inspection and testing
is required, however, please feel free to take a look and comment on
anything that looks odd. As always, additional testing is welcome and
encouraged.
[1] https://lore.kernel.org/linux-security-module/20260316213606.374109-5-paul@paul-moore.com/
--
CHANGELOG:
v2:
- remove the user O_PATH file patch from Amir
- add the backing_file LSM blob and lifecycle hooks
- update the SELinux code to reflect the other changes
v1:
- initial version
--
Paul Moore (2):
lsm: add backing_file LSM hooks
selinux: fix overlayfs mmap() and mprotect() access checks
fs/backing-file.c | 18 +-
fs/erofs/ishare.c | 10 +
fs/file_table.c | 21 ++
fs/fuse/passthrough.c | 2
fs/internal.h | 3
fs/overlayfs/dir.c | 2
fs/overlayfs/file.c | 2
include/linux/backing-file.h | 4
include/linux/fs.h | 1
include/linux/lsm_audit.h | 2
include/linux/lsm_hook_defs.h | 5
include/linux/lsm_hooks.h | 1
include/linux/security.h | 22 ++
security/lsm.h | 1
security/lsm_init.c | 9 +
security/security.c | 100 +++++++++++
security/selinux/hooks.c | 252 +++++++++++++++++++++---------
security/selinux/include/objsec.h | 17 ++
18 files changed, 387 insertions(+), 85 deletions(-)
^ permalink raw reply
* Re: [PATCH 6/7] tomoyo: Convert from sb_mount to granular mount hooks
From: Song Liu @ 2026-03-23 3:32 UTC (permalink / raw)
To: Tetsuo Handa
Cc: viro@zeniv.linux.org.uk, Song Liu, paul@paul-moore.com,
jmorris@namei.org, serge@hallyn.com, brauner@kernel.org,
jack@suse.cz, john.johansen@canonical.com,
stephen.smalley.work@gmail.com, omosnace@redhat.com,
mic@digikod.net, gnoack@google.com, takedakn@nttdata.co.jp,
herton@canonical.com, Kernel Team, selinux@vger.kernel.org,
apparmor@lists.ubuntu.com, linux-fsdevel@vger.kernel.org,
linux-security-module@vger.kernel.org
In-Reply-To: <33abcf34-13e2-4a37-83f3-78bb27ecbc11@I-love.SAKURA.ne.jp>
> On Mar 22, 2026, at 3:46 AM, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
>
> On 2026/03/22 10:06, Song Liu wrote:
>>>> @@ -70,6 +70,7 @@ static bool tomoyo_check_mount_acl(struct tomoyo_request_info *r,
>>>> * @dir: Pointer to "struct path".
>>>> * @type: Name of filesystem type.
>>>> * @flags: Mount options.
>>>> + * @dev_path: Pre-resolved device/source path. Maybe NULL.
>>>
>>> I guess that we can avoid passing maybe-NULL dev_name if Al Viro can accept
>>> resolving maybe-NULL dev_path argument before calling LSM hooks.
>>
>> If I understand the code correctly, tomoyo records requested_dev_name for
>> new mount. namespace.c, OTOH, does NOT do kern_path() for new mount. This
>> is why we keep the maybe-NULL dev_name here. I personally think this is
>> the best approach without changing tomoyo behavior.
>
> Well, namespace.c does kern_path() for new mount, but it happens a bit later after
> security_mount_new() was called.
>
> do_new_mount_fc() => fc_mount() => vfs_get_tree() => fc->ops->get_tree() == e.g. ext4_get_tree()
> => get_tree_bdev() => get_tree_bdev_flags() => lookup_bdev() => kern_path()
>
> @@ -3835,6 +3855,9 @@ static int do_new_mount(const struct path *path, const char *fstype,
> err = parse_monolithic_mount_data(fc, data);
> if (!err && !mount_capable(fc))
> err = -EPERM;
> +
> + if (!err)
> + err = security_mount_new(fc, path, mnt_flags, flags, data);
> if (!err)
> err = do_new_mount_fc(fc, path, mnt_flags);
>
>
> Since not all filesystems need to resolve dev_name argument, conversion from dev_name
> to "struct path" is up to individual filesystem. If we can use a flag like FS_REQUIRES_DEV
> that tells whether that filesystem will resolve dev_name argument, I think we can resolve
> the dev_name argument before security_mount_new() is called (and can avoid TOCTOU).
I guess we can add dev_path to fs_context?
Thanks,
Song
^ permalink raw reply
* Re: [PATCH] ima: abort file hash computation on fatal signal
From: Eric Biggers @ 2026-03-22 14:17 UTC (permalink / raw)
To: Shigeru Yoshida
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn,
syzbot+6ed94e81a1492fe1d512, linux-integrity,
linux-security-module, linux-kernel
In-Reply-To: <20260322111019.2815601-1-syoshida@redhat.com>
On Sun, Mar 22, 2026 at 08:10:19PM +0900, Shigeru Yoshida wrote:
> ima_calc_file_hash_atfm() and ima_calc_file_hash_tfm() compute a hash
> over the entire file contents without checking for pending fatal
> signals. When a very large file is being hashed during mmap (via
> ima_file_mmap), the computation can take an extended period. If a
> coredump is initiated by another thread in the same thread group during
> this time, the dumper thread waits in coredump_wait() for all other
> threads to exit. However, the hashing thread cannot exit until the hash
> loop completes, resulting in a hung task.
>
> Add fatal_signal_pending() checks to both the ahash and shash file
> hashing loops so that the computation is aborted promptly when SIGKILL
> is received.
>
> Fixes: 3323eec921ef ("integrity: IMA as an integrity service provider")
> Reported-by: syzbot+6ed94e81a1492fe1d512@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=6ed94e81a1492fe1d512
> Tested-by: syzbot+6ed94e81a1492fe1d512@syzkaller.appspotmail.com
> Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
> ---
> security/integrity/ima/ima_crypto.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index aff61643415d..7b721b9c944f 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -17,6 +17,7 @@
> #include <linux/crypto.h>
> #include <linux/scatterlist.h>
> #include <linux/err.h>
> +#include <linux/sched/signal.h>
> #include <linux/slab.h>
> #include <crypto/hash.h>
>
> @@ -416,6 +417,12 @@ static int ima_calc_file_hash_atfm(struct file *file,
>
> if (rbuf[1])
> active = !active; /* swap buffers, if we use two */
> +
> + if (fatal_signal_pending(current)) {
> + ahash_wait(ahash_rc, &wait);
> + rc = -EINTR;
> + goto out3;
> + }
I think you'll need to rebase onto
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/log/?h=next-integrity
since there is a patch queued up that removes ima_calc_file_hash_atfm().
So only ima_calc_file_hash_tfm() will need to be updated.
- Eric
^ permalink raw reply
* [PATCH] ima: abort file hash computation on fatal signal
From: Shigeru Yoshida @ 2026-03-22 11:10 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn
Cc: Shigeru Yoshida, syzbot+6ed94e81a1492fe1d512, linux-integrity,
linux-security-module, linux-kernel
ima_calc_file_hash_atfm() and ima_calc_file_hash_tfm() compute a hash
over the entire file contents without checking for pending fatal
signals. When a very large file is being hashed during mmap (via
ima_file_mmap), the computation can take an extended period. If a
coredump is initiated by another thread in the same thread group during
this time, the dumper thread waits in coredump_wait() for all other
threads to exit. However, the hashing thread cannot exit until the hash
loop completes, resulting in a hung task.
Add fatal_signal_pending() checks to both the ahash and shash file
hashing loops so that the computation is aborted promptly when SIGKILL
is received.
Fixes: 3323eec921ef ("integrity: IMA as an integrity service provider")
Reported-by: syzbot+6ed94e81a1492fe1d512@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6ed94e81a1492fe1d512
Tested-by: syzbot+6ed94e81a1492fe1d512@syzkaller.appspotmail.com
Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
---
security/integrity/ima/ima_crypto.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index aff61643415d..7b721b9c944f 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -17,6 +17,7 @@
#include <linux/crypto.h>
#include <linux/scatterlist.h>
#include <linux/err.h>
+#include <linux/sched/signal.h>
#include <linux/slab.h>
#include <crypto/hash.h>
@@ -416,6 +417,12 @@ static int ima_calc_file_hash_atfm(struct file *file,
if (rbuf[1])
active = !active; /* swap buffers, if we use two */
+
+ if (fatal_signal_pending(current)) {
+ ahash_wait(ahash_rc, &wait);
+ rc = -EINTR;
+ goto out3;
+ }
}
/* wait for the last update request to complete */
rc = ahash_wait(ahash_rc, &wait);
@@ -491,6 +498,10 @@ static int ima_calc_file_hash_tfm(struct file *file,
rc = crypto_shash_update(shash, rbuf, rbuf_len);
if (rc)
break;
+ if (fatal_signal_pending(current)) {
+ rc = -EINTR;
+ break;
+ }
}
kfree(rbuf);
out:
--
2.52.0
^ permalink raw reply related
* Re: [PATCH 6/7] tomoyo: Convert from sb_mount to granular mount hooks
From: Tetsuo Handa @ 2026-03-22 10:46 UTC (permalink / raw)
To: Song Liu, viro@zeniv.linux.org.uk
Cc: Song Liu, paul@paul-moore.com, jmorris@namei.org,
serge@hallyn.com, brauner@kernel.org, jack@suse.cz,
john.johansen@canonical.com, stephen.smalley.work@gmail.com,
omosnace@redhat.com, mic@digikod.net, gnoack@google.com,
takedakn@nttdata.co.jp, herton@canonical.com, Kernel Team,
selinux@vger.kernel.org, apparmor@lists.ubuntu.com,
linux-fsdevel@vger.kernel.org,
linux-security-module@vger.kernel.org
In-Reply-To: <4DF5C4A8-7C92-4F76-9B34-2262089E7289@meta.com>
On 2026/03/22 10:06, Song Liu wrote:
>>> @@ -70,6 +70,7 @@ static bool tomoyo_check_mount_acl(struct tomoyo_request_info *r,
>>> * @dir: Pointer to "struct path".
>>> * @type: Name of filesystem type.
>>> * @flags: Mount options.
>>> + * @dev_path: Pre-resolved device/source path. Maybe NULL.
>>
>> I guess that we can avoid passing maybe-NULL dev_name if Al Viro can accept
>> resolving maybe-NULL dev_path argument before calling LSM hooks.
>
> If I understand the code correctly, tomoyo records requested_dev_name for
> new mount. namespace.c, OTOH, does NOT do kern_path() for new mount. This
> is why we keep the maybe-NULL dev_name here. I personally think this is
> the best approach without changing tomoyo behavior.
Well, namespace.c does kern_path() for new mount, but it happens a bit later after
security_mount_new() was called.
do_new_mount_fc() => fc_mount() => vfs_get_tree() => fc->ops->get_tree() == e.g. ext4_get_tree()
=> get_tree_bdev() => get_tree_bdev_flags() => lookup_bdev() => kern_path()
@@ -3835,6 +3855,9 @@ static int do_new_mount(const struct path *path, const char *fstype,
err = parse_monolithic_mount_data(fc, data);
if (!err && !mount_capable(fc))
err = -EPERM;
+
+ if (!err)
+ err = security_mount_new(fc, path, mnt_flags, flags, data);
if (!err)
err = do_new_mount_fc(fc, path, mnt_flags);
Since not all filesystems need to resolve dev_name argument, conversion from dev_name
to "struct path" is up to individual filesystem. If we can use a flag like FS_REQUIRES_DEV
that tells whether that filesystem will resolve dev_name argument, I think we can resolve
the dev_name argument before security_mount_new() is called (and can avoid TOCTOU).
^ permalink raw reply
* Re: [PATCH 6/7] tomoyo: Convert from sb_mount to granular mount hooks
From: Song Liu @ 2026-03-22 1:06 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Song Liu, viro@zeniv.linux.org.uk, paul@paul-moore.com,
jmorris@namei.org, serge@hallyn.com, brauner@kernel.org,
jack@suse.cz, john.johansen@canonical.com,
stephen.smalley.work@gmail.com, omosnace@redhat.com,
mic@digikod.net, gnoack@google.com, takedakn@nttdata.co.jp,
herton@canonical.com, Kernel Team, selinux@vger.kernel.org,
apparmor@lists.ubuntu.com, linux-fsdevel@vger.kernel.org,
linux-security-module@vger.kernel.org
In-Reply-To: <b720f521-e930-4f35-9505-1bfdf9e2818c@I-love.SAKURA.ne.jp>
> On Mar 21, 2026, at 5:54 AM, Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> wrote:
>
> On 2026/03/19 3:43, Song Liu wrote:
>> Replace tomoyo_sb_mount() with granular mount hooks. Each hook
>> reconstructs the MS_* flags expected by tomoyo_mount_permission()
>> using the original flags parameter where available.
>>
>> Key changes:
>> - mount_bind: passes the pre-resolved source path to
>> tomoyo_mount_acl() via a new dev_path parameter, instead of
>> re-resolving dev_name via kern_path(). This eliminates a TOCTOU
>> vulnerability.
>> - mount_new, mount_remount, mount_reconfigure: use the original
>> mount(2) flags for policy matching.
>> - mount_move: passes pre-resolved paths for both source and
>> destination.
>> - mount_change_type: passes raw ms_flags directly.
>>
>> Also removes the unused data_page parameter from
>> tomoyo_mount_permission().
>>
>> Code generated with the assistance of Claude, reviewed by human.
>>
>> Signed-off-by: Song Liu <song@kernel.org>
>
> Basically OK. One question to Al Viro.
>
> Do you still refuse the idea of resolving dev_path argument before calling LSM hooks
> (the proposal you NAKed at https://lkml.kernel.org/r/20250709102410.GU1880847@ZenIV )
> despite this series removes security_sb_mount() and security_move_mount() hooks?
>
>> diff --git a/security/tomoyo/mount.c b/security/tomoyo/mount.c
>> index 322dfd188ada..82ffe7d02814 100644
>> --- a/security/tomoyo/mount.c
>> +++ b/security/tomoyo/mount.c
>> @@ -70,6 +70,7 @@ static bool tomoyo_check_mount_acl(struct tomoyo_request_info *r,
>> * @dir: Pointer to "struct path".
>> * @type: Name of filesystem type.
>> * @flags: Mount options.
>> + * @dev_path: Pre-resolved device/source path. Maybe NULL.
>
> I guess that we can avoid passing maybe-NULL dev_name if Al Viro can accept
> resolving maybe-NULL dev_path argument before calling LSM hooks.
If I understand the code correctly, tomoyo records requested_dev_name for
new mount. namespace.c, OTOH, does NOT do kern_path() for new mount. This
is why we keep the maybe-NULL dev_name here. I personally think this is
the best approach without changing tomoyo behavior.
Thanks,
Song
^ permalink raw reply
* Re: [PATCH 6/7] tomoyo: Convert from sb_mount to granular mount hooks
From: Tetsuo Handa @ 2026-03-21 12:54 UTC (permalink / raw)
To: Song Liu, viro
Cc: paul, jmorris, serge, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn, herton,
kernel-team, selinux, apparmor, linux-fsdevel,
linux-security-module
In-Reply-To: <20260318184400.3502908-7-song@kernel.org>
On 2026/03/19 3:43, Song Liu wrote:
> Replace tomoyo_sb_mount() with granular mount hooks. Each hook
> reconstructs the MS_* flags expected by tomoyo_mount_permission()
> using the original flags parameter where available.
>
> Key changes:
> - mount_bind: passes the pre-resolved source path to
> tomoyo_mount_acl() via a new dev_path parameter, instead of
> re-resolving dev_name via kern_path(). This eliminates a TOCTOU
> vulnerability.
> - mount_new, mount_remount, mount_reconfigure: use the original
> mount(2) flags for policy matching.
> - mount_move: passes pre-resolved paths for both source and
> destination.
> - mount_change_type: passes raw ms_flags directly.
>
> Also removes the unused data_page parameter from
> tomoyo_mount_permission().
>
> Code generated with the assistance of Claude, reviewed by human.
>
> Signed-off-by: Song Liu <song@kernel.org>
Basically OK. One question to Al Viro.
Do you still refuse the idea of resolving dev_path argument before calling LSM hooks
(the proposal you NAKed at https://lkml.kernel.org/r/20250709102410.GU1880847@ZenIV )
despite this series removes security_sb_mount() and security_move_mount() hooks?
> diff --git a/security/tomoyo/mount.c b/security/tomoyo/mount.c
> index 322dfd188ada..82ffe7d02814 100644
> --- a/security/tomoyo/mount.c
> +++ b/security/tomoyo/mount.c
> @@ -70,6 +70,7 @@ static bool tomoyo_check_mount_acl(struct tomoyo_request_info *r,
> * @dir: Pointer to "struct path".
> * @type: Name of filesystem type.
> * @flags: Mount options.
> + * @dev_path: Pre-resolved device/source path. Maybe NULL.
I guess that we can avoid passing maybe-NULL dev_name if Al Viro can accept
resolving maybe-NULL dev_path argument before calling LSM hooks.
^ permalink raw reply
* Re: [PATCH v6 3/9] landlock: Control pathname UNIX domain socket resolution by path
From: Mickaël Salaün @ 2026-03-21 9:09 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, Tingmao Wang, Justin Suess,
Sebastian Andrzej Siewior, Kuniyuki Iwashima, Jann Horn,
linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Tahera Fahimi, Kees Cook
In-Reply-To: <20260320.ee35c8125f6f@gnoack.org>
On Fri, Mar 20, 2026 at 11:25:04PM +0100, Günther Noack wrote:
> On Fri, Mar 20, 2026 at 06:51:13PM +0100, Mickaël Salaün wrote:
> > On Fri, Mar 20, 2026 at 05:15:40PM +0100, Günther Noack wrote:
> > > On Wed, Mar 18, 2026 at 05:52:48PM +0100, Mickaël Salaün wrote:
> > > > On Sun, Mar 15, 2026 at 11:21:44PM +0100, Günther Noack wrote:
> > > > > + * @client: Client domain
> > > > > + * @server: Server domain
> > > > > + * @masks: Layer access masks to unmask
> > > > > + * @access: Access bit that controls scoping
> > > > > + */
> > > > > +static void unmask_scoped_access(const struct landlock_ruleset *const client,
> > > > > + const struct landlock_ruleset *const server,
> > > > > + struct layer_access_masks *const masks,
> > > > > + const access_mask_t access)
> > > > > +{
> > > > > + int client_layer, server_layer;
> > > > > + const struct landlock_hierarchy *client_walker, *server_walker;
> > > > > +
> > > > > + /* This should not happen. */
> > > > > + if (WARN_ON_ONCE(!client))
> > > > > + return;
> > > > > +
> > > > > + /* Server has no Landlock domain; nothing to clear. */
> > > > > + if (!server)
> > > > > + return;
> > > > > +
> > > >
> > > > Please also copy the BUILD_BUG_ON() from domain_is_scoped().
> > >
> > > I don't understand what this check is good for. It says:
> > >
> > > /*
> > > * client_layer must be a signed integer with greater capacity
> > > * than client->num_layers to ensure the following loop stops.
> > > */
> > > BUILD_BUG_ON(sizeof(client_layer) > sizeof(client->num_layers));
> > >
> > > For the loop to terminate, in my understanding, client_layer must be
> > > able to store client->num_layers - 1 down to - 1, but that is anyway a
> > > given since num_layers can't exceed 16 and client_layer is signed. It
> > > seems that the termination of this would anyway be caught in our tests
> > > as well?
> > >
> > > Could you please clarify what this BUILD_BUG_ON() is trying to assert?
> >
> > The intent was to make sure client_layer is indeed an int and not an
> > unsigned int for instance. Hopefully tests catch that but using a
> > build-time assert catch the potential issue and document it. Also, it
> > would be weird to not have the same checks in both copies of code.
>
> It isn't clear to me why the BUILD_BUG_ON is checking based on the
> sizeof() of these variables then. The number of bytes in an integer
> type is independent of its signedness, after all. Should we rather do
> this maybe?
>
> /*
> * client_layer must be a signed integer to ensure the following
> * loop stops.
> */
> BUILD_BUG_ON(!is_signed_type(typeof(client_layer)));
I didn't know about this macro, looks good.
>
> (Although that is also a bit of a triviality given that the same
> variable is being defined as a signed integer just a few lines above,
> but at least it is very explicit about it.)
Yeah, I know, but I added this canary check because I was bitten by a
bug. Even if it might be trivial now, it might help when working on
other parts, and it's just a build-time check that also serves as doc.
If in doubt, let's keep build-time checks that were most likely added
for a good reason. I prefer to have build-time errors than run-time
ones.
>
> I'd drop the remark about the capacity, as even a signed 8-bit integer
> is large enough to hold the layer indices and the -1.
Large enough for now... All these checks are useful to easily spot
issues if/when the current invariant change (e.g. if one day we extend
the number of max layers). Integer C types can silently cast to other
integer types (with different capacity or sign/unsigned), which might be
the source of bugs. So yeah, please keep the current capacity check and
add the sign one, it won't do any harm.
>
> Does that sound reasonable? I can do it in the other function as well
> if you want to keep things consistent.
Yes, please update the other function as well.
^ permalink raw reply
* Re: [PATCH v6 3/9] landlock: Control pathname UNIX domain socket resolution by path
From: Günther Noack @ 2026-03-20 22:25 UTC (permalink / raw)
To: Mickaël Salaün
Cc: John Johansen, Tingmao Wang, Justin Suess,
Sebastian Andrzej Siewior, Kuniyuki Iwashima, Jann Horn,
linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Tahera Fahimi
In-Reply-To: <20260320.eez3sheeThul@digikod.net>
On Fri, Mar 20, 2026 at 06:51:13PM +0100, Mickaël Salaün wrote:
> On Fri, Mar 20, 2026 at 05:15:40PM +0100, Günther Noack wrote:
> > On Wed, Mar 18, 2026 at 05:52:48PM +0100, Mickaël Salaün wrote:
> > > On Sun, Mar 15, 2026 at 11:21:44PM +0100, Günther Noack wrote:
> > > > + * @client: Client domain
> > > > + * @server: Server domain
> > > > + * @masks: Layer access masks to unmask
> > > > + * @access: Access bit that controls scoping
> > > > + */
> > > > +static void unmask_scoped_access(const struct landlock_ruleset *const client,
> > > > + const struct landlock_ruleset *const server,
> > > > + struct layer_access_masks *const masks,
> > > > + const access_mask_t access)
> > > > +{
> > > > + int client_layer, server_layer;
> > > > + const struct landlock_hierarchy *client_walker, *server_walker;
> > > > +
> > > > + /* This should not happen. */
> > > > + if (WARN_ON_ONCE(!client))
> > > > + return;
> > > > +
> > > > + /* Server has no Landlock domain; nothing to clear. */
> > > > + if (!server)
> > > > + return;
> > > > +
> > >
> > > Please also copy the BUILD_BUG_ON() from domain_is_scoped().
> >
> > I don't understand what this check is good for. It says:
> >
> > /*
> > * client_layer must be a signed integer with greater capacity
> > * than client->num_layers to ensure the following loop stops.
> > */
> > BUILD_BUG_ON(sizeof(client_layer) > sizeof(client->num_layers));
> >
> > For the loop to terminate, in my understanding, client_layer must be
> > able to store client->num_layers - 1 down to - 1, but that is anyway a
> > given since num_layers can't exceed 16 and client_layer is signed. It
> > seems that the termination of this would anyway be caught in our tests
> > as well?
> >
> > Could you please clarify what this BUILD_BUG_ON() is trying to assert?
>
> The intent was to make sure client_layer is indeed an int and not an
> unsigned int for instance. Hopefully tests catch that but using a
> build-time assert catch the potential issue and document it. Also, it
> would be weird to not have the same checks in both copies of code.
It isn't clear to me why the BUILD_BUG_ON is checking based on the
sizeof() of these variables then. The number of bytes in an integer
type is independent of its signedness, after all. Should we rather do
this maybe?
/*
* client_layer must be a signed integer to ensure the following
* loop stops.
*/
BUILD_BUG_ON(!is_signed_type(typeof(client_layer)));
(Although that is also a bit of a triviality given that the same
variable is being defined as a signed integer just a few lines above,
but at least it is very explicit about it.)
I'd drop the remark about the capacity, as even a signed 8-bit integer
is large enough to hold the layer indices and the -1.
Does that sound reasonable? I can do it in the other function as well
if you want to keep things consistent.
–Günther
^ permalink raw reply
* Re: [PATCH v3 0/8] module: Move 'struct module_signature' to UAPI
From: Nicolas Schier @ 2026-03-20 20:06 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: David Howells, David Woodhouse, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn,
Nathan Chancellor, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
Hao Luo, Jiri Olsa, Shuah Khan, keyrings, linux-kernel,
linux-modules, linux-s390, linux-integrity, linux-security-module,
linux-kbuild, bpf, linux-kselftest
In-Reply-To: <20260305-module-signature-uapi-v3-0-92f45ea6028c@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 2929 bytes --]
On Thu, Mar 05, 2026 at 10:31:36AM +0100, Thomas Weißschuh wrote:
> This structure definition is used outside the kernel proper.
> For example in kmod and the kernel build environment.
>
> To allow reuse, move it to a new UAPI header.
>
> While it is not a true UAPI, it is a common practice to have
> non-UAPI interface definitions in the kernel's UAPI headers.
>
> This came up as part of my CONFIG_MODULE_HASHES series [0].
> But it is useful on its own and so we get it out of the way.
>
> [0] https://lore.kernel.org/lkml/aZ3OfJJSJgfOb0rJ@levanger/
>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
> Changes in v3:
> - Also adapt the include path for the custom sign-file rule in the bpf selftests.
> (My manual run of BPF CI still fails, due to an BUG() on s390,
> I don't see how this is due to this patch)
> - Link to v2: https://lore.kernel.org/r/20260305-module-signature-uapi-v2-0-dc4d81129dee@linutronix.de
>
> Changes in v2:
> - Drop spurious definition of MODULE_SIGNATURE_TYPE_MERKLE.
> - s/modules/module/ in two patch subjects.
> - Pick up review tags.
> - Link to v1: https://lore.kernel.org/r/20260302-module-signature-uapi-v1-0-207d955e0d69@linutronix.de
>
> ---
> Thomas Weißschuh (8):
> extract-cert: drop unused definition of PKEY_ID_PKCS7
> module: Drop unused signature types
> module: Give 'enum pkey_id_type' a more specific name
> module: Give MODULE_SIG_STRING a more descriptive name
> module: Move 'struct module_signature' to UAPI
> tools uapi headers: add linux/module_signature.h
> sign-file: use 'struct module_signature' from the UAPI headers
> selftests/bpf: verify_pkcs7_sig: Use 'struct module_signature' from the UAPI headers
>
> arch/s390/kernel/machine_kexec_file.c | 6 ++--
> certs/extract-cert.c | 2 --
> include/linux/module_signature.h | 30 +---------------
> include/uapi/linux/module_signature.h | 41 ++++++++++++++++++++++
> kernel/module/signing.c | 4 +--
> kernel/module_signature.c | 2 +-
> scripts/Makefile | 1 +
> scripts/sign-file.c | 19 +++-------
> security/integrity/ima/ima_modsig.c | 6 ++--
> tools/include/uapi/linux/module_signature.h | 41 ++++++++++++++++++++++
> tools/testing/selftests/bpf/Makefile | 1 +
> .../selftests/bpf/prog_tests/verify_pkcs7_sig.c | 28 ++-------------
> 12 files changed, 101 insertions(+), 80 deletions(-)
> ---
> base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
> change-id: 20260302-module-signature-uapi-61fa80b1e2bb
>
Thanks for these patches!
For the whole series:
Reviewed-by: Nicolas Schier <nsc@kernel.org>
--
Nicolas
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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