* [PATCH v2 3/5] samples/landlock: Add support for named UNIX domain socket restrictions
From: Günther Noack @ 2026-01-10 14:33 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Justin Suess, linux-security-module,
Tingmao Wang, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Jann Horn, Tahera Fahimi
In-Reply-To: <20260110143300.71048-2-gnoack3000@gmail.com>
The access rights for UNIX domain socket lookups are grouped with the
read-write rights in the sample tool. Rationale: In the general case,
any operations are possible through a UNIX domain socket, including
data-mutating operations.
Cc: Justin Suess <utilityemal77@gmail.com>
Cc: Mickaël Salaün <mic@digikod.net>
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
samples/landlock/sandboxer.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index e7af02f98208..f7e73ba8910c 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -295,11 +295,14 @@ static bool check_ruleset_scope(const char *const env_var,
LANDLOCK_ACCESS_FS_MAKE_SYM | \
LANDLOCK_ACCESS_FS_REFER | \
LANDLOCK_ACCESS_FS_TRUNCATE | \
- LANDLOCK_ACCESS_FS_IOCTL_DEV)
+ LANDLOCK_ACCESS_FS_IOCTL_DEV | \
+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | \
+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | \
+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET)
/* clang-format on */
-#define LANDLOCK_ABI_LAST 7
+#define LANDLOCK_ABI_LAST 8
#define XSTR(s) #s
#define STR(s) XSTR(s)
@@ -444,6 +447,17 @@ int main(const int argc, char *const argv[], char *const *const envp)
"provided by ABI version %d (instead of %d).\n",
LANDLOCK_ABI_LAST, abi);
__attribute__((fallthrough));
+ case 7:
+ /*
+ * Removes LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM,
+ * LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM and
+ * LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET for ABI < 8
+ */
+ ruleset_attr.handled_access_fs &=
+ ~(LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM |
+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM |
+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET);
+ __attribute__((fallthrough));
case LANDLOCK_ABI_LAST:
break;
default:
--
2.52.0
^ permalink raw reply related
* [PATCH v2 4/5] landlock/selftests: Test named UNIX domain socket restrictions
From: Günther Noack @ 2026-01-10 14:33 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Justin Suess, linux-security-module,
Tingmao Wang, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Jann Horn, Tahera Fahimi
In-Reply-To: <20260110143300.71048-2-gnoack3000@gmail.com>
* Exercise the access rights for connect() and sendmsg() to named UNIX
domain sockets, in various combinations.
* Extract common helpers from an existing IOCTL test that
also uses pathname unix(7) sockets.
Cc: Justin Suess <utilityemal77@gmail.com>
Cc: Mickaël Salaün <mic@digikod.net>
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
tools/testing/selftests/landlock/fs_test.c | 218 +++++++++++++++++++--
1 file changed, 202 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 0cbde65e032a..e1822fa687e8 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -4360,30 +4360,61 @@ TEST_F_FORK(layout1, named_pipe_ioctl)
ASSERT_EQ(child_pid, waitpid(child_pid, NULL, 0));
}
+/*
+ * set_up_named_unix_server - Create a pathname unix socket
+ *
+ * If the socket type is not SOCK_DGRAM, also invoke listen(2).
+ *
+ * Return: The listening FD - it is the caller responsibility to close it.
+ */
+static int set_up_named_unix_server(struct __test_metadata *const _metadata,
+ int type, const char *const path)
+{
+ int fd;
+ struct sockaddr_un addr = {
+ .sun_family = AF_UNIX,
+ };
+
+ fd = socket(AF_UNIX, type, 0);
+ ASSERT_LE(0, fd);
+
+ strncpy(addr.sun_path, path, sizeof(addr.sun_path));
+ ASSERT_EQ(0, bind(fd, (struct sockaddr *)&addr, sizeof(addr)));
+
+ if (type != SOCK_DGRAM)
+ ASSERT_EQ(0, listen(fd, 10 /* qlen */));
+ return fd;
+}
+
+/*
+ * test_connect_named_unix - connect to the given named UNIX socket
+ *
+ * Return: The errno from connect(), or 0
+ */
+static int test_connect_named_unix(int fd, const char *const path)
+{
+ struct sockaddr_un addr = {
+ .sun_family = AF_UNIX,
+ };
+ strncpy(addr.sun_path, path, sizeof(addr.sun_path));
+
+ if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) == -1)
+ return errno;
+ return 0;
+}
+
/* For named UNIX domain sockets, no IOCTL restrictions apply. */
TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
{
const char *const path = file1_s1d1;
int srv_fd, cli_fd, ruleset_fd;
- struct sockaddr_un srv_un = {
- .sun_family = AF_UNIX,
- };
- struct sockaddr_un cli_un = {
- .sun_family = AF_UNIX,
- };
const struct landlock_ruleset_attr attr = {
.handled_access_fs = LANDLOCK_ACCESS_FS_IOCTL_DEV,
};
/* Sets up a server */
ASSERT_EQ(0, unlink(path));
- srv_fd = socket(AF_UNIX, SOCK_STREAM, 0);
- ASSERT_LE(0, srv_fd);
-
- strncpy(srv_un.sun_path, path, sizeof(srv_un.sun_path));
- ASSERT_EQ(0, bind(srv_fd, (struct sockaddr *)&srv_un, sizeof(srv_un)));
-
- ASSERT_EQ(0, listen(srv_fd, 10 /* qlen */));
+ srv_fd = set_up_named_unix_server(_metadata, SOCK_STREAM, path);
/* Enables Landlock. */
ruleset_fd = landlock_create_ruleset(&attr, sizeof(attr), 0);
@@ -4395,9 +4426,7 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
cli_fd = socket(AF_UNIX, SOCK_STREAM, 0);
ASSERT_LE(0, cli_fd);
- strncpy(cli_un.sun_path, path, sizeof(cli_un.sun_path));
- ASSERT_EQ(0,
- connect(cli_fd, (struct sockaddr *)&cli_un, sizeof(cli_un)));
+ ASSERT_EQ(0, test_connect_named_unix(cli_fd, path));
/* FIONREAD and other IOCTLs should not be forbidden. */
EXPECT_EQ(0, test_fionread_ioctl(cli_fd));
@@ -4572,6 +4601,163 @@ TEST_F_FORK(ioctl, handle_file_access_file)
ASSERT_EQ(0, close(file_fd));
}
+/* clang-format off */
+FIXTURE(unix_socket) {};
+
+FIXTURE_SETUP(unix_socket) {};
+
+FIXTURE_TEARDOWN(unix_socket) {};
+/* clang-format on */
+
+FIXTURE_VARIANT(unix_socket)
+{
+ const __u64 handled;
+ const __u64 allowed;
+ const int sock_type;
+ const int expected;
+ const bool use_sendto;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, stream_handled_not_allowed)
+{
+ /* clang-format on */
+ .handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM,
+ .allowed = 0,
+ .sock_type = SOCK_STREAM,
+ .expected = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, stream_handled_and_allowed)
+{
+ /* clang-format on */
+ .handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM,
+ .allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM,
+ .sock_type = SOCK_STREAM,
+ .expected = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, dgram_handled_not_allowed)
+{
+ /* clang-format on */
+ .handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM,
+ .allowed = 0,
+ .sock_type = SOCK_DGRAM,
+ .expected = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, dgram_handled_and_allowed)
+{
+ /* clang-format on */
+ .handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM,
+ .allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM,
+ .sock_type = SOCK_DGRAM,
+ .expected = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, dgram_sendto_handled_not_allowed)
+{
+ /* clang-format on */
+ .handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM,
+ .allowed = 0,
+ .sock_type = SOCK_DGRAM,
+ .use_sendto = true,
+ .expected = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, dgram_sendto_handled_and_allowed)
+{
+ /* clang-format on */
+ .handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM,
+ .allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM,
+ .sock_type = SOCK_DGRAM,
+ .use_sendto = true,
+ .expected = 0,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, seqpacket_handled_not_allowed)
+{
+ /* clang-format on */
+ .handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET,
+ .allowed = 0,
+ .sock_type = SOCK_SEQPACKET,
+ .expected = EACCES,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(unix_socket, seqpacket_handled_and_allowed)
+{
+ /* clang-format on */
+ .handled = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET,
+ .allowed = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET,
+ .sock_type = SOCK_SEQPACKET,
+ .expected = 0,
+};
+
+/*
+ * test_sendto_named_unix - sendto to the given named UNIX socket
+ *
+ * sendto() is equivalent to sendmsg() in this respect.
+ *
+ * Return: The errno from sendto(), or 0
+ */
+static int test_sendto_named_unix(int fd, const char *const path)
+{
+ static const char buf[] = "dummy";
+ struct sockaddr_un addr = {
+ .sun_family = AF_UNIX,
+ };
+ strncpy(addr.sun_path, path, sizeof(addr.sun_path));
+
+ if (sendto(fd, buf, sizeof(buf), 0, (struct sockaddr *)&addr,
+ sizeof(addr)) == -1)
+ return errno;
+ return 0;
+}
+
+TEST_F_FORK(unix_socket, test)
+{
+ const char *const path = "sock";
+ int cli_fd, srv_fd, ruleset_fd, res;
+ const struct rule rules[] = {
+ {
+ .path = path,
+ .access = variant->allowed,
+ },
+ {},
+ };
+
+ /* Sets up a server */
+ srv_fd = set_up_named_unix_server(_metadata, variant->sock_type, path);
+
+ /* Enables Landlock. */
+ ruleset_fd = create_ruleset(_metadata, variant->handled, rules);
+ ASSERT_LE(0, ruleset_fd);
+ enforce_ruleset(_metadata, ruleset_fd);
+ ASSERT_EQ(0, close(ruleset_fd));
+
+ /* Sets up a client connection to it */
+ cli_fd = socket(AF_UNIX, variant->sock_type, 0);
+ ASSERT_LE(0, cli_fd);
+
+ /* Connecting or sendto to the Unix socket is denied. */
+ if (variant->use_sendto)
+ res = test_sendto_named_unix(cli_fd, path);
+ else
+ res = test_connect_named_unix(cli_fd, path);
+ EXPECT_EQ(variant->expected, res);
+
+ ASSERT_EQ(0, close(cli_fd));
+ ASSERT_EQ(0, close(srv_fd));
+ ASSERT_EQ(0, unlink(path));
+}
+
/* clang-format off */
FIXTURE(layout1_bind) {};
/* clang-format on */
--
2.52.0
^ permalink raw reply related
* [PATCH v2 5/5] landlock: Document FS access rights for pathname UNIX sockets
From: Günther Noack @ 2026-01-10 14:33 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Justin Suess, linux-security-module,
Tingmao Wang, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Jann Horn, Tahera Fahimi
In-Reply-To: <20260110143300.71048-2-gnoack3000@gmail.com>
Cc: Justin Suess <utilityemal77@gmail.com>
Cc: Mickaël Salaün <mic@digikod.net>
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
Documentation/userspace-api/landlock.rst | 25 +++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 1d0c2c15c22e..29afde4f7e75 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -77,7 +77,10 @@ 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_STREAM |
+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM |
+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET,
.handled_access_net =
LANDLOCK_ACCESS_NET_BIND_TCP |
LANDLOCK_ACCESS_NET_CONNECT_TCP,
@@ -127,6 +130,17 @@ 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:
+ /*
+ * Removes LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM,
+ * LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM and
+ * LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET for ABI < 8
+ */
+ ruleset_attr.handled_access_fs &=
+ ~(LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM |
+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM |
+ LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET);
}
This enables the creation of an inclusive ruleset that will contain our rules.
@@ -604,6 +618,15 @@ Landlock audit events with the ``LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF``,
sys_landlock_restrict_self(). See Documentation/admin-guide/LSM/landlock.rst
for more details on audit.
+Pathname UNIX sockets (ABI < 8)
+-------------------------------
+
+Starting with the Landlock ABI version 8, it is possible to restrict
+connections to pathname :manpage:`unix(7)` sockets using the new
+``LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM``,
+``LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM`` and
+``LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET`` rights.
+
.. _kernel_support:
Kernel support
--
2.52.0
^ permalink raw reply related
* Re: [RFC PATCH 3/5] samples/landlock: Add support for LANDLOCK_ACCESS_FS_CONNECT_UNIX
From: Günther Noack @ 2026-01-10 15:05 UTC (permalink / raw)
To: Justin Suess
Cc: demiobenour, fahimitahera, hi, ivanov.mikhail1, jannh,
konstantin.meskhidze, linux-security-module, m, matthieu, mic,
paul, samasth.norway.ananda
In-Reply-To: <20260101193009.4005972-1-utilityemal77@gmail.com>
Hello!
On Thu, Jan 01, 2026 at 02:30:06PM -0500, Justin Suess wrote:
> Allow users to separately specify unix socket rights,
> document the variable, and make the right optional.
>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> Cc: Günther Noack <gnoack3000@gmail.com>
FYI, I ended up not applying this on V2.
I am unconvinced whether further separating the groups of access
rights is a good idea for the sandboxer. This is just sample code to
be used as reference, so it is good to keep it simple. I feel that
giving it more granular control over access rights does not help
readers to understand it much further?
It is true though that it would make sense to have this feature in
more production-grade tools. 👍
–Günther
^ permalink raw reply
* Re: [PATCH v2 1/5] lsm: Add hook unix_path_connect
From: Justin Suess @ 2026-01-10 16:45 UTC (permalink / raw)
To: Günther Noack, Mickaël Salaün, Paul Moore,
James Morris, Serge E . Hallyn
Cc: linux-security-module, Tingmao Wang, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
Demi Marie Obenour, Alyssa Ross, Jann Horn, Tahera Fahimi,
Simon Horman, netdev, Alexander Viro, Christian Brauner
In-Reply-To: <20260110143300.71048-4-gnoack3000@gmail.com>
On 1/10/26 09:32, Günther Noack wrote:
> From: Justin Suess <utilityemal77@gmail.com>
>
> Adds an LSM hook unix_path_connect.
>
> This hook is called to check the path of a named unix socket before a
> connection is initiated.
>
> Cc: Günther Noack <gnoack3000@gmail.com>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
> include/linux/lsm_hook_defs.h | 4 ++++
> include/linux/security.h | 11 +++++++++++
> net/unix/af_unix.c | 9 +++++++++
> security/security.c | 20 ++++++++++++++++++++
> 4 files changed, 44 insertions(+)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 8c42b4bde09c..1dee5d8d52d2 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -317,6 +317,10 @@ LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
> LSM_HOOK(int, 0, watch_key, struct key *key)
> #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +LSM_HOOK(int, 0, unix_path_connect, const struct path *path, int type, int flags)
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_SECURITY_NETWORK
> LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
> struct sock *newsk)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f..382612af27a6 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1931,6 +1931,17 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
> }
> #endif /* CONFIG_SECURITY_NETWORK */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +
> +int security_unix_path_connect(const struct path *path, int type, int flags);
> +
> +#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +static inline int security_unix_path_connect(const struct path *path, int type, int flags)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_SECURITY_INFINIBAND
> int security_ib_pkey_access(void *sec, u64 subnet_prefix, u16 pkey);
> int security_ib_endport_manage_subnet(void *sec, const char *name, u8 port_num);
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 55cdebfa0da0..3aabe2d489ae 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1226,6 +1226,15 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
> if (!S_ISSOCK(inode->i_mode))
> goto path_put;
>
> + /*
> + * We call the hook because we know that the inode is a socket
> + * and we hold a valid reference to it via the path.
> + */
> + err = security_unix_path_connect(&path, type, flags);
> + if (err)
> + goto path_put;
> +
> + err = -ECONNREFUSED;
> sk = unix_find_socket_byinode(inode);
> if (!sk)
> goto path_put;
> diff --git a/security/security.c b/security/security.c
> index 31a688650601..0cee3502db83 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4731,6 +4731,26 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
>
> #endif /* CONFIG_SECURITY_NETWORK */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +/*
> + * security_unix_path_connect() - Check if a named AF_UNIX socket can connect
> + * @path: path of the socket being connected to
> + * @type: type of the socket
> + * @flags: flags associated with the socket
> + *
> + * This hook is called to check permissions before connecting to a named
> + * AF_UNIX socket.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_unix_path_connect(const struct path *path, int type, int flags)
> +{
> + return call_int_hook(unix_path_connect, path, type, flags);
> +}
> +EXPORT_SYMBOL(security_unix_path_connect);
> +
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_SECURITY_INFINIBAND
> /**
> * security_ib_pkey_access() - Check if access to an IB pkey is allowed
Just for awareness,
I'm considering renaming this hook to unix_socket_path_lookup, since as Günther
pointed out this hook is not just hit on connect, but also on sendmsg.
Justin
^ permalink raw reply
* Re: [PATCH v3] selftests: complete kselftest include centralization
From: Bala-Vignesh-Reddy @ 2026-01-10 16:00 UTC (permalink / raw)
To: jackmanb
Cc: Liam.Howlett, akpm, davem, david.shane.hunter, david, edumazet,
gnoack, horms, khalid, kuba, linux-kernel-mentees, linux-kernel,
linux-kselftest, linux-mm, linux-security-module, lorenzo.stoakes,
mhocko, mic, ming.lei, pabeni, reddybalavignesh9979,
richard.weiyang, shuah, surenb, vbabka
In-Reply-To: <DFHI984SEFV3.2JL88CLHNT2SO@google.com>
Hey Brendan,
Thanks for the report.
This issue is caused by my change that centralized the kselftest.h
include path in lib.mk, while the x86 selftests Makefile overwrites CFLAGS
with :=, so shared include path unable to find kselftest.h. The fix is to
explicity add the selftests include directory to CFLAGS in
tools/testing/selftests/x86/Makefile.
I have already submitted this:
[PATCH] selftests/x86: Add selftests include path for kselftest.h after centralization
Link: https://lore.kernel.org/lkml/20251022062948.162852-1-reddybalavignesh9979@gmail.com/
it has been tested and confirmed working.
Tested-by: Anders Roxell <anders.roxell@linaro.org>
Once that patch is merged, the x86 selftests build issue should be
resolved.
Thanks
Bala Vignesh
^ permalink raw reply
* Re: [PATCH v2 3/5] samples/landlock: Add support for named UNIX domain socket restrictions
From: Günther Noack @ 2026-01-11 9:50 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Justin Suess, linux-security-module, Tingmao Wang,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi
In-Reply-To: <20260110143300.71048-7-gnoack3000@gmail.com>
On Sat, Jan 10, 2026 at 03:33:00PM +0100, Günther Noack wrote:
> The access rights for UNIX domain socket lookups are grouped with the
> read-write rights in the sample tool. Rationale: In the general case,
> any operations are possible through a UNIX domain socket, including
> data-mutating operations.
Sorry, I missed a part of the discussion in V1, which was suggested by
Tingmao Wang in [1]:
You are right, the new access rights should indeed become part of
ACCESS_FILE in the sample tool. (When the sample tool is adding a
rule for a non-directory, it only applies access rights that are also
in ACCESS_FILE.)
Will add it in V3.
–Günther
[1] https://lore.kernel.org/all/423dd2ca-ecba-47cf-98a7-4d99a48939da@maowtm.org/
^ permalink raw reply
* Re: [PATCH v2 1/5] lsm: Add hook unix_path_connect
From: Günther Noack @ 2026-01-11 9:55 UTC (permalink / raw)
To: Justin Suess
Cc: Mickaël Salaün, Paul Moore, James Morris,
Serge E . Hallyn, linux-security-module, Tingmao Wang,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Simon Horman, netdev, Alexander Viro,
Christian Brauner
In-Reply-To: <4bc22faa-2927-4ef9-b5dc-67a7575177e9@gmail.com>
On Sat, Jan 10, 2026 at 11:45:03AM -0500, Justin Suess wrote:
> Just for awareness,
>
> I'm considering renaming this hook to unix_socket_path_lookup, since as Günther
> pointed out this hook is not just hit on connect, but also on sendmsg.
+1 I would be in favor of this.
(In doubt, Paul Moore has the last word on LSM hook naming. I believe
that both "lookup" and "resolve" are being used exchangeably to refer
to path lookups. (e.g. see the path_resolution(7) man page [1]))
–Günther
[1] https://man7.org/linux/man-pages/man7/path_resolution.7.html
^ permalink raw reply
* Re: [RFC PATCH 0/5] landlock: Pathname-based UNIX connect() control
From: Günther Noack @ 2026-01-11 10:15 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Paul Moore, 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, Kuniyuki Iwashima
In-Reply-To: <20260109.aiXa3Iesh5wa@digikod.net>
On Fri, Jan 09, 2026 at 04:20:49PM +0100, Mickaël Salaün wrote:
> On Fri, Jan 09, 2026 at 03:41:33PM +0100, Günther Noack wrote:
> > On Fri, Jan 09, 2026 at 11:37:12AM +0100, Mickaël Salaün wrote:
> > > As Kuniyuki pointed out [1], we should handle both connect and send.
> > > This would be similar to the scoped restriction from Tingmao. I guess
> > > we'll need a similar hook for the send operation. Because there is no
> > > need to differenciate between connected and disconnected unix socket in
> > > a security policy, we should have one access right for both. Any
> > > proposal for its name? Something like TRANSMIT_UNIX or EMIT_UNIX?
> > >
> > > [1] https://lore.kernel.org/all/CAAVpQUAd==+Pw02+E6UC-qwaDNm7aFg+Q9YDbWzyniShAkAhFQ@mail.gmail.com/
> >
> > Ah, thanks for pointing it out.
> >
> > The restriction as implemented in this patch set already solves this
> > for all the three cases where a Unix socket file is looked up. I
> > believe that it is happening in all the right times (everytime when
> > the lookup has to happen).
> >
> > The cases where the restriction applies are the following:
> >
> > * unix_stream_connect - when calling connect() on a stream socket
> > * unix_dgram_connect - when calling connect() on a dgram socket
> > * unix_dgram_sendmsg - when calling sendmsg() on a dgram socket
> > (per-message lookup only)
> >
> > You can find the code locations by looking for the call to
> > unix_find_other() in af_unix.c. (That function invokes either
> > unix_find_bsd() or the lookup for abstract Unix sockets.)
> >
> > In the unix_dgram_sendmsg() case, the lookup is only performed if an
> > explicit sockaddr_un was provided together with the arguments to the
> > sendmsg(). (And sendto(2) also uses the same code path as
> > sendmsg(2).)
>
> Great
FYI, V2 splits the access rights for the three UNIX access types
SOCK_STREAM, SOCK_DGRAM and SOCK_SEQPACKET.
FYI, I double checked that the logic extends cleanly to the
SOCK_SEQPACKET case as well: SOCK_SEQPACKET is implemented as a thin
layer above the existing hooks for SOCK_STREAM and SOCK_DGRAM - it
does connect(2) with the SOCK_STREAM implementation, and it does
sendmsg(2) with the SOCK_DGRAM implementation, but in the sendmsg(2)
case it removes any explicitly passed recipient addresses beforehand.
(See "unix_seqpacket_ops" in net/unix/af_unix.c) So as far as the UNIX
path lookup hook is concerned, SOCK_SEQPACKET behaves like
SOCK_STREAM, and only invokes the hook during connect(2).
> > It is true that the current name for the access right is slightly
> > misleading. How about LANDLOCK_ACCESS_FS_UNIX_SEND? (Like
> > "transmit", but a bit closer to the naming of the sendmsg(2)
> > networking API?)
>
> We should try to keep the access right naming consistent:
> LANDLOCK_ACCESS_FS_<VERB>[_NOUN]
>
> What about USE_UNIX, or FIND_UNIX (closer to the kernel function), or
> RESOLVE_UNIX? It should be clear with the name that it is not about
> listening nor receiving from a process outside of the sandbox (which
> should have its own access right BTW).
Thanks, changed in V2 to say RESOLVE_UNIX, and also split up the
access rights for the three UNIX socket types.
(For reference, the path_resolution(7) man page [1] also uses the verb
"resolve". I think both "resolve" and "lookup" are used exchangeably
for paths.)
[1] https://man7.org/linux/man-pages/man7/path_resolution.7.html
> > (I guess the other alternative would be to wire the socket type
> > information through to the unix_find_bsd() function and pass it
> > through. Would require a small change to the af_unix.c implementation,
> > but then we could tell apart LANDLOCK_ACCESS_FS_UNIX_STREAM_CONNECT
> > and LANDLOCK_ACCESS_FS_UNIX_DGRAM_SEND). WDYT?
>
> I think the hook should have the same arguments as unix_find_bsd()'s
> ones. This gives the full context of the call.
You are right, I had not recalled that the socket type information
already gets passed to unix_find_bsd() - it required not much
additional wiring at all, in the end. 👍
–Günther
^ permalink raw reply
* [PATCH] landlock: Clarify documentation for the IOCTL access right
From: Günther Noack @ 2026-01-11 17:52 UTC (permalink / raw)
To: Mickaël Salaüne
Cc: linux-security-module, Tingmao Wang, Samasth Norway Ananda,
Günther Noack
Move the description of the LANDLOCK_ACCESS_FS_IOCTL_DEV access right
together with the file access rights.
This group of access rights applies to files (in this case device
files), and they can be added to file or directory inodes using
landlock_add_rule(2). The check for that works the same for all file
access rights, including LANDLOCK_ACCESS_FS_IOCTL_DEV.
Invoking ioctl(2) on directory FDs can not currently be restricted
with Landlock. Having it grouped separately in the documentation is a
remnant from earlier revisions of the LANDLOCK_ACCESS_FS_IOCTL_DEV
patch set.
Link: https://lore.kernel.org/all/20260108.Thaex5ruach2@digikod.net/
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
include/uapi/linux/landlock.h | 37 ++++++++++++++++-------------------
1 file changed, 17 insertions(+), 20 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index eac65da687c1..fbd18cf60a88 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -216,6 +216,23 @@ struct landlock_net_port_attr {
* :manpage:`ftruncate(2)`, :manpage:`creat(2)`, or :manpage:`open(2)` with
* ``O_TRUNC``. This access right is available since the third version of the
* Landlock ABI.
+ * - %LANDLOCK_ACCESS_FS_IOCTL_DEV: Invoke :manpage:`ioctl(2)` commands on an opened
+ * character or block device.
+ *
+ * This access right applies to all `ioctl(2)` commands implemented by device
+ * drivers. However, the following common IOCTL commands continue to be
+ * invokable independent of the %LANDLOCK_ACCESS_FS_IOCTL_DEV right:
+ *
+ * * IOCTL commands targeting file descriptors (``FIOCLEX``, ``FIONCLEX``),
+ * * IOCTL commands targeting file descriptions (``FIONBIO``, ``FIOASYNC``),
+ * * IOCTL commands targeting file systems (``FIFREEZE``, ``FITHAW``,
+ * ``FIGETBSZ``, ``FS_IOC_GETFSUUID``, ``FS_IOC_GETFSSYSFSPATH``)
+ * * Some IOCTL commands which do not make sense when used with devices, but
+ * whose implementations are safe and return the right error codes
+ * (``FS_IOC_FIEMAP``, ``FICLONE``, ``FICLONERANGE``, ``FIDEDUPERANGE``)
+ *
+ * This access right is available since the fifth version of the Landlock
+ * ABI.
*
* Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used
* with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as
@@ -275,26 +292,6 @@ struct landlock_net_port_attr {
* If multiple requirements are not met, the ``EACCES`` error code takes
* precedence over ``EXDEV``.
*
- * The following access right applies both to files and directories:
- *
- * - %LANDLOCK_ACCESS_FS_IOCTL_DEV: Invoke :manpage:`ioctl(2)` commands on an opened
- * character or block device.
- *
- * This access right applies to all `ioctl(2)` commands implemented by device
- * drivers. However, the following common IOCTL commands continue to be
- * invokable independent of the %LANDLOCK_ACCESS_FS_IOCTL_DEV right:
- *
- * * IOCTL commands targeting file descriptors (``FIOCLEX``, ``FIONCLEX``),
- * * IOCTL commands targeting file descriptions (``FIONBIO``, ``FIOASYNC``),
- * * IOCTL commands targeting file systems (``FIFREEZE``, ``FITHAW``,
- * ``FIGETBSZ``, ``FS_IOC_GETFSUUID``, ``FS_IOC_GETFSSYSFSPATH``)
- * * Some IOCTL commands which do not make sense when used with devices, but
- * whose implementations are safe and return the right error codes
- * (``FS_IOC_FIEMAP``, ``FICLONE``, ``FICLONERANGE``, ``FIDEDUPERANGE``)
- *
- * This access right is available since the fifth version of the Landlock
- * ABI.
- *
* .. warning::
*
* It is currently not possible to restrict some file-related actions
--
2.52.0
^ permalink raw reply related
* Re: [RFC PATCH 1/2] landlock: access_mask_subset() helper
From: Günther Noack @ 2026-01-11 20:01 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze
In-Reply-To: <20260109.Te8xoaceiqu3@digikod.net>
On Fri, Jan 09, 2026 at 05:06:10PM +0100, Mickaël Salaün wrote:
> On Tue, Dec 30, 2025 at 11:39:19AM +0100, Günther Noack wrote:
> > This helper function checks whether an access_mask_t has a subset of the
> > bits enabled than another one. This expresses the intent a bit smoother
> > in the code and does not cost us anything when it gets inlined.
> >
> > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > ---
> > security/landlock/fs.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > index fe794875ad461..b4ce03bef4b8e 100644
> > --- a/security/landlock/fs.c
> > +++ b/security/landlock/fs.c
> > @@ -398,6 +398,15 @@ static const struct access_masks any_fs = {
> > .fs = ~0,
> > };
> >
> > +/*
> > + * Returns true iff a has a subset of the bits of b.
> > + * It helps readability and gets inlined.
> > + */
> > +static bool access_mask_subset(access_mask_t a, access_mask_t b)
> > +{
> > + return (a | b) == b;
>
> I'm curious about why this switches to a binary OR instead of the
> original AND.
It is slightly more intuitive to me, but other than that, no specific reason.
(a | b) == b has the same results as (b & a) == a.
I'm not feeling strongly about it.
We can also do it the other way around if you prefer.
–Günther
^ permalink raw reply
* Re: [RFC PATCH 2/2] landlock: transpose the layer masks data structure
From: Günther Noack @ 2026-01-11 20:51 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze
In-Reply-To: <20260109.hie6Teis2ha9@digikod.net>
On Fri, Jan 09, 2026 at 05:18:43PM +0100, Mickaël Salaün wrote:
> This looks good overall but I need to spend more time reviewing it.
>
> Because this changes may impact other ongoing patch series, I think I'll
> take this patch first to ease potential future fix backports.
>
> On Tue, Dec 30, 2025 at 11:39:21AM +0100, Günther Noack wrote:
> > The layer masks data structure tracks the requested but unfulfilled
> > access rights during an operations security check. It stores one bit
> > for each combination of access right and layer index. If the bit is
> > set, that access right is not granted (yet) in the given layer and we
> > have to traverse the path further upwards to grant it.
> >
> > Previously, the layer masks were stored as arrays mapping from access
> > right indices to layer_mask_t. The layer_mask_t value then indicates
> > all layers in which the given access right is still (tentatively)
> > denied.
> >
> > This patch introduces struct layer_access_masks instead: This struct
> > contains an array with the access_mask_t of each (tentatively) denied
> > access right in that layer.
> >
> > The hypothesis of this patch is that this simplifies the code enough
> > so that the resulting code will run faster:
> >
> > * We can use bitwise operations in multiple places where we previously
> > looped over bits individually with macros. (Should require less
> > branch speculation)
> >
> > * Code is ~160 lines smaller.
>
> What about the KUnit test lines?
Those are counted as well. The 160 lines statistic is directly from
the diffstat in the cover letter.
(I removed the test_get_layer_deny_mask KUnit test, because the
function under test was also not needed any more. Other than that, the
KUnit tests are just adapted to test the equivalent logic with the new
data structure.)
> > Other noteworthy changes:
> >
> > * Clarify deny_mask_t and the code assembling it.
> > * Document what that value looks like
> > * Make writing and reading functions specific to file system rules.
> > (It only worked for FS rules before as well, but going all the way
> > simplifies the code logic more.)
> > * In no_more_access(), call a new helper function may_refer(), which
> > only solves the asymmetric case. Previously, the code interleaved
> > the checks for the two symmetric cases in RENAME_EXCHANGE. It feels
> > that the code is clearer when renames without RENAME_EXCHANGE are
> > more obviously the normal case.
>
> It would be interesting to check the stackframe diff. You can use
> scripts/stackdelta for that, see
> https://git.kernel.org/mic/c/602acfb541195eb35584d7a3fc7d1db676f059bd
Acknowledged. I did not get around to it yet, but put it on my todo
list.
> > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > ---
> > security/landlock/access.h | 10 +-
> > security/landlock/audit.c | 155 ++++++----------
> > security/landlock/audit.h | 3 +-
> > security/landlock/domain.c | 120 +++----------
> > security/landlock/domain.h | 6 +-
> > security/landlock/fs.c | 350 ++++++++++++++++--------------------
> > security/landlock/net.c | 10 +-
> > security/landlock/ruleset.c | 78 +++-----
> > security/landlock/ruleset.h | 18 +-
> > 9 files changed, 290 insertions(+), 460 deletions(-)
460 - 290 is 170. Well, almost 160. :)
> > diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> > index dfcdc19ea2683..d20e28d38e9c9 100644
> > --- a/security/landlock/ruleset.c
> > +++ b/security/landlock/ruleset.c
> > @@ -622,49 +622,24 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
> > * request are empty).
> > */
> > bool landlock_unmask_layers(const struct landlock_rule *const rule,
> > - const access_mask_t access_request,
> > - layer_mask_t (*const layer_masks)[],
> > - const size_t masks_array_size)
> > + struct layer_access_masks *masks)
> > {
> > - size_t layer_level;
> > -
> > - if (!access_request || !layer_masks)
> > + if (!masks)
> > return true;
> > if (!rule)
> > return false;
> >
> > - /*
> > - * An access is granted if, for each policy layer, at least one rule
> > - * encountered on the pathwalk grants the requested access,
> > - * regardless of its position in the layer stack. We must then check
> > - * the remaining layers for each inode, from the first added layer to
> > - * the last one. When there is multiple requested accesses, for each
> > - * policy layer, the full set of requested accesses may not be granted
> > - * by only one rule, but by the union (binary OR) of multiple rules.
> > - * E.g. /a/b <execute> + /a <read> => /a/b <execute + read>
> > - */
>
> Why removing this comment?
I did not understand why the comment was discussing this higher level
picture when the surrounding function landlock_unmask_layers() is only
concerned with a single rule. Should this comment be better moved
elsewhere?
I don't feel strongly about it and re-reading it, the comment is still
true. In doubt, I can also just put it back into the same function
again. Let me know what you prefer.
> > - for (layer_level = 0; layer_level < rule->num_layers; layer_level++) {
> > - const struct landlock_layer *const layer =
> > - &rule->layers[layer_level];
> > - const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
> > - const unsigned long access_req = access_request;
> > - unsigned long access_bit;
> > - bool is_empty;
> > + for (int i = 0; i < rule->num_layers; i++) {
> > + const struct landlock_layer *l = &rule->layers[i];
> >
> > - /*
> > - * Records in @layer_masks which layer grants access to each requested
> > - * access: bit cleared if the related layer grants access.
> > - */
> > - is_empty = true;
> > - for_each_set_bit(access_bit, &access_req, masks_array_size) {
> > - if (layer->access & BIT_ULL(access_bit))
> > - (*layer_masks)[access_bit] &= ~layer_bit;
> > - is_empty = is_empty && !(*layer_masks)[access_bit];
> > - }
> > - if (is_empty)
> > - return true;
> > + masks->access[l->level - 1] &= ~l->access;
> > }
> > - return false;
> > +
> > + for (int i = 0; i < LANDLOCK_MAX_NUM_LAYERS; i++) {
> > + if (masks->access[i])
> > + return false;
> > + }
> > + return true;
> > }
> >
> > typedef access_mask_t
–Günther
^ permalink raw reply
* Re: [RFC PATCH v3 0/8] landlock: Add UDP access control support
From: Günther Noack @ 2026-01-11 21:23 UTC (permalink / raw)
To: Matthieu Buffet
Cc: Mickaël Salaün, Günther Noack,
linux-security-module, Mikhail Ivanov, konstantin.meskhidze,
netdev
In-Reply-To: <20251212163704.142301-1-matthieu@buffet.re>
Hello Matthieu!
On Fri, Dec 12, 2025 at 05:36:56PM +0100, Matthieu Buffet wrote:
> Here is v3 of UDP support for Landlock. My apologies for the delay, I've
> had to deal with unrelated problems. All feedback from v1/v2 should be
> merged, thanks again for taking the time to review them.
Good to see the patch again. :)
Apologies for review delay as well. There are many Landlock reviews
in flight at the moment, it might take some time to catch up with all
of them.
FYI: In [1], I have been sending a patch for controlling UNIX socket
lookup, which is restricting connect() and sendmsg() operations for
UNIX domain sockets of types SOCK_STREAM, SOCK_DGRAM and
SOCK_SEQPACKET. I am bringing it up because it feels that the
semantics for the UDP and UNIX datagram access rights hook in similar
places and therefore should work similarly?
In the current UNIX socket patch set (v2), there is only one Landlock
access right which controls both connect() and sendmsg() when they are
done on a UNIX datagram socket. This feels natural to be, because you
can reach the same recipient address whether that is done with
connect() or with sendmsg()...?
(Was there a previous discussion where it was decided that these
should be two different access rights for UDP sockets and UNIX dgram
sockets?)
[1] https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
Thanks,
–Günther
^ permalink raw reply
* Re: [RFC PATCH 0/2] landlock: Refactor layer masks
From: Günther Noack @ 2026-01-11 21:40 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze
In-Reply-To: <20260109.au3vee9Eisei@digikod.net>
On Fri, Jan 09, 2026 at 04:59:19PM +0100, Mickaël Salaün wrote:
> On Tue, Dec 30, 2025 at 11:48:21AM +0100, Günther Noack wrote:
> > To compile it, use:
> >
> > cc -o benchmark_worsecase benchmark_worsecase.c
>
> It would be useful to clean up a bit this benchmark and add it to the
> selftests' Landlock directory (see seccomp_benchmark.c).
Thanks for the pointer, I did not realize this existed.
I'll have a look for V2.
–Günther
^ permalink raw reply
* Re: [RFC PATCH 2/2] landlock: transpose the layer masks data structure
From: Günther Noack @ 2026-01-11 21:52 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze
In-Reply-To: <20251230103917.10549-7-gnoack3000@gmail.com>
On Tue, Dec 30, 2025 at 11:39:21AM +0100, Günther Noack wrote:
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 7961c6630a2d7..aa0efa36a37db 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> [...]
> /*
> * Tracks domains responsible of a denied access. This is required to avoid
> * storing in each object the full layer_masks[] required by update_request().
> + *
> + * Each nibble represents the layer index of the newest layer which denied a
> + * certain access right. For file system access rights, the upper four bits are
> + * the index of the layer which denies LANDLOCK_ACCESS_FS_IOCTL_DEV and the
> + * lower nibble represents LANDLOCK_ACCESS_FS_TRUNCATE.
> */
> typedef u8 deny_masks_t;
FYI: I left this out for now because it felt a bit out of scope (and
transposing the layer masks was adventurous enough), but I was tempted
to go one step further here and turn this into a struct with
bitfields:
/* A collection of layer indices denying specific access rights. */
struct layers_denying_fs_access {
unsigned int truncate : 4;
unsigned int ioctl_dev : 4;
}
(Type name TBD, I am open for suggestions.)
I think if we accept that this data structure is specific to FS access
rights, we win clarity in the code. When I came across the code that
put this together dynamically and in a more generic way, it took me a
while to figure out what it did.
–Günther
^ permalink raw reply
* Re: [PATCH] fuse: fix conversion of fuse_reverse_inval_entry() to start_removing()
From: Christian Brauner @ 2026-01-12 9:45 UTC (permalink / raw)
To: NeilBrown
Cc: Christian Brauner, Amir Goldstein, Jan Kara, linux-fsdevel,
Jeff Layton, Chris Mason, David Sterba, David Howells,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich,
Tyler Hicks, Miklos Szeredi, Chuck Lever, Olga Kornievskaia,
Dai Ngo, Namjae Jeon, Steve French, Sergey Senozhatsky,
Carlos Maiolino, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Stephen Smalley, Ondrej Mosnacek, Mateusz Guzik,
Lorenzo Stoakes, Stefan Berger, Darrick J. Wong, linux-kernel,
netfs, ecryptfs, linux-nfs, linux-unionfs, linux-cifs, linux-xfs,
linux-security-module, selinux, Alexander Viro, Val Packett
In-Reply-To: <176454037897.634289.3566631742434963788@noble.neil.brown.name>
On Mon, 01 Dec 2025 09:06:18 +1100, NeilBrown wrote:
> The recent conversion of fuse_reverse_inval_entry() to use
> start_removing() was wrong.
> As Val Packett points out the original code did not call ->lookup
> while the new code does. This can lead to a deadlock.
>
> Rather than using full_name_hash() and d_lookup() as the old code
> did, we can use try_lookup_noperm() which combines these. Then
> the result can be given to start_removing_dentry() to get the required
> locks for removal. We then double check that the name hasn't
> changed.
>
> [...]
Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes
[1/1] fuse: fix conversion of fuse_reverse_inval_entry() to start_removing()
https://git.kernel.org/vfs/vfs/c/cab012375122
^ permalink raw reply
* Re: [PATCH v5 20/36] locking/ww_mutex: Support Clang's context analysis
From: Maarten Lankhorst @ 2026-01-12 10:32 UTC (permalink / raw)
To: Bart Van Assche
Cc: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon,
David S. Miller, Luc Van Oostenryck, Chris Li, Paul E. McKenney,
Alexander Potapenko, Arnd Bergmann, Christoph Hellwig,
Dmitry Vyukov, Eric Dumazet, Frederic Weisbecker,
Greg Kroah-Hartman, Herbert Xu, Ian Rogers, Jann Horn,
Joel Fernandes, Johannes Berg, Jonathan Corbet, Josh Triplett,
Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Nathan Chancellor,
Neeraj Upadhyay, Nick Desaulniers, Steven Rostedt, Tetsuo Handa,
Thomas Gleixner, Thomas Graf, Uladzislau Rezki, Waiman Long,
kasan-dev, linux-crypto, linux-doc, linux-kbuild, linux-kernel,
linux-mm, linux-security-module, linux-sparse, linux-wireless,
llvm, rcu
In-Reply-To: <8143ab09-fd9b-4615-8afb-7ee10e073c51@acm.org>
Hey,
The acquire_done() call was always optional. It's meant to indicate that after this point,
ww_acquire_lock may no longer be called and backoff can no longer occur.
It's allowed to call ww_acquire_fini() without ww_acquire_done()
Think of this case:
ww_acquire_init()
ww_acquire_lock_interruptible() -> -ERESTARTSYS
ww_acquire_fini()
Here it wouldn't make sense to call ww_acquire_done().
It's mostly to facilitate this case:
ww_acquire_init()
ww_acquire_lock() a bunch.
/* Got all locks, do the work as no more backoff occurs */
ww_acquire_done()
...
unlock_all()
ww_acquire_fini()
If you call ww_acquire_lock after done, a warning should occur as this should no longer happen.
Kind regards,
~Maarten Lankhorst
Den 2026-01-09 kl. 22:26, skrev Bart Van Assche:
> (+Maarten)
>
> On 1/9/26 2:06 PM, Marco Elver wrote:
>> If there's 1 out of N ww_mutex users that missed ww_acquire_done()
>> there's a good chance that 1 case is wrong.
>
> $ git grep -w ww_acquire_done '**c'|wc -l
> 11
> $ git grep -w ww_acquire_fini '**c'|wc -l
> 33
>
> The above statistics show that there are more cases where
> ww_acquire_done() is not called rather than cases where
> ww_acquire_done() is called.
>
> Maarten, since you introduced the ww_mutex code, do you perhaps prefer
> that calling ww_acquire_done() is optional or rather that all users that
> do not call ww_acquire_done() are modified such that they call
> ww_acquire_done()? The full email conversation is available here:
> https://lore.kernel.org/all/20251219154418.3592607-1-elver@google.com/
>
> Thanks,
>
> Bart.
^ permalink raw reply
* Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Mimi Zohar @ 2026-01-12 14:02 UTC (permalink / raw)
To: Jeff Layton, Frederick Lawler
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, Darrick J. Wong, Christian Brauner,
Josef Bacik, linux-kernel, linux-integrity, linux-security-module,
kernel-team
In-Reply-To: <25b6d1b42ea07b058be4e4f48bb5a7c6b879b3ed.camel@kernel.org>
On Tue, 2026-01-06 at 14:50 -0500, Jeff Layton wrote:
> > > > > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > > */
> > > > > static inline bool
> > > > > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > > > > - const struct inode *inode)
> > > > > + struct file *file, struct inode *inode)
> > > > > {
> > > > > - return (inode->i_sb->s_dev != attrs->dev ||
> > > > > - inode->i_ino != attrs->ino ||
> > > > > - !inode_eq_iversion(inode, attrs->version));
> > > > > + struct kstat stat;
> > > > > +
> > > > > + if (inode->i_sb->s_dev != attrs->dev ||
> > > > > + inode->i_ino != attrs->ino)
> > > > > + return true;
> > > > > +
> > > > > + if (inode_eq_iversion(inode, attrs->version))
> > > > > + return false;
> > > > > +
> > > > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> > > > > + AT_STATX_SYNC_AS_STAT))
> > > > > + return true;
> > > > > +
> > > >
> > > > This is rather odd. You're sampling the i_version field directly, but
> > > > if it's not equal then you go through ->getattr() to get the ctime.
> > > >
> > > > It's particularly odd since you don't know whether the i_version field
> > > > is even implemented on the fs. On filesystems where it isn't, the
> > > > i_version field generally stays at 0, so won't this never fall through
> > > > to do the vfs_getattr_nosec() call on those filesystems?
> > > >
> > >
> > > You're totally right. I didn't consider FS's caching the value at zero.
> >
> > Actually, I'm going to amend this. I think I did consider FSs without an
> > implementation. Where this is called at, it is often guarded by a
> > !IS_I_VERSION() || integrity_inode_attrs_change(). If I'm
> > understanding this correctly, the check call doesn't occur unless the inode
> > has i_version support.
> >
>
>
> It depends on what you mean by i_version support:
>
> That flag just tells the VFS that it needs to bump the i_version field
> when updating timestamps. It's not a reliable indicator of whether the
> i_version field is suitable for the purpose you want here.
>
> The problem here and the one that we ultimately fixed with multigrain
> timestamps is that XFS in particular will bump i_version on any change
> to the log. That includes atime updates due to reads.
>
> XFS still tracks the i_version the way it always has, but we've stopped
> getattr() from reporting it because it's not suitable for the purpose
> that nfsd (and IMA) need it for.
>
> > It seems to me the suggestion then is to remove the IS_I_VERSION()
> > checks guarding the call sites, grab both ctime and cookie from stat,
> > and if IS_I_VERSION() use that, otherwise cookie, and compare
> > against the cached i_version with one of those values, and then fall
> > back to ctime?
> >
>
> Not exactly.
>
> You want to call getattr() for STATX_CHANGE_COOKIE|STATX_CTIME, and
> then check the kstat->result_mask. If STATX_CHANGE_COOKIE is set, then
> use that. If it's not then use the ctime.
>
> The part I'm not sure about is whether it's actually safe to do this.
> vfs_getattr_nosec() can block in some situations. Is it ok to do this
> in any context where integrity_inode_attrs_changed() may be called?
Frederick, before making any changes, please describe the problem you're
actually seeing. From my limited testing, file change IS being detected. A major
change like Jeff is suggesting is not something that would or should be back
ported. Remember, Jeff's interest is remote filesystems, not necessarily with
your particular XFS concern.
So again, what is the problem you're trying to address?
Mimi
>
> ISTR that this was an issue at one point, but maybe isn't now that IMA
> is an LSM?
^ permalink raw reply
* Re: [PATCH v2 2/5] landlock: Control pathname UNIX domain socket resolution by path
From: Justin Suess @ 2026-01-12 15:38 UTC (permalink / raw)
To: Günther Noack, Mickaël Salaün
Cc: Jann Horn, linux-security-module, Tingmao Wang,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross,
Tahera Fahimi
In-Reply-To: <20260110143300.71048-6-gnoack3000@gmail.com>
On 1/10/26 09:32, Günther Noack wrote:
> * Add new access rights which control the look up operations for named
> UNIX domain sockets. The resolution happens during connect() and
> sendmsg() (depending on socket type).
> * LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM
> * LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM
> * LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET
Might be a crazy thought but would it be better to implement the
STREAM/DGRAM/SEQPACKET as an add_rule flag rather than as a separate
access right? There are other types of address families like AF_CAN,
AF_BLUETOOTH, AF_VSOCK that support multiple socket types.
This saves us on access right numbers if they get added in the future to
landlock.
So we could have:
LANDLOCK_ADD_RULE_SOCK_STREAM
LANDLOCK_ADD_RULE_SOCK_DGRAM
LANDLOCK_ADD_RULE_SOCK_SEQPACKET
and use it as such:
landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
&path_beneath_for_unix_socket,
LANDLOCK_ADD_RULE_SOCK_STREAM |
LANDLOCK_ADD_RULE_SOCK_DGRAM);
For address families with only one socket type (ie tcp and udp), the
socket family be implied, which keeps backward compatibility w/ the
existing tcp access right.
This way, we don't have to make completely separate access rights for
future socket families. So we could add a single access right for bluetooth,
for instance, and distinguish which socket families we give it with the
LANDLOCK_ADD_RULE_SOCK_* flags.
We'd have to track the SOCK_(socket_type) for unix sockets as we gather
access rights. But afaik unix sockets should be the only socket type that
has to deal with tree traversal.
> * 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.
>
> Cc: Justin Suess <utilityemal77@gmail.com>
> Cc: Mickaël Salaün <mic@digikod.net>
> Suggested-by: Jann Horn <jannh@google.com>
> Link: https://github.com/landlock-lsm/linux/issues/36
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> include/uapi/linux/landlock.h | 10 ++++++
> security/landlock/access.h | 2 +-
> security/landlock/audit.c | 6 ++++
> security/landlock/fs.c | 34 +++++++++++++++++++-
> security/landlock/limits.h | 2 +-
> security/landlock/syscalls.c | 2 +-
> tools/testing/selftests/landlock/base_test.c | 2 +-
> tools/testing/selftests/landlock/fs_test.c | 7 ++--
> 8 files changed, 58 insertions(+), 7 deletions(-)
>
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index f030adc462ee..455edc241c12 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -216,6 +216,13 @@ struct landlock_net_port_attr {
> * :manpage:`ftruncate(2)`, :manpage:`creat(2)`, or :manpage:`open(2)` with
> * ``O_TRUNC``. This access right is available since the third version of the
> * Landlock ABI.
> + * - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM: Connect to named
> + * :manpage:`unix(7)` ``SOCK_STREAM`` sockets.
> + * - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM: Send messages to named
> + * :manpage:`unix(7)` ``SOCK_DGRAM`` sockets or connect to them using
> + * :manpage:`connect(2)`.
> + * - %LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET: Connect to named
> + * :manpage:`unix(7)` ``SOCK_SEQPACKET`` sockets.
> *
> * Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used
> * with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as
> @@ -321,6 +328,9 @@ struct landlock_net_port_attr {
> #define LANDLOCK_ACCESS_FS_REFER (1ULL << 13)
> #define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14)
> #define LANDLOCK_ACCESS_FS_IOCTL_DEV (1ULL << 15)
> +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM (1ULL << 16)
> +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM (1ULL << 17)
> +#define LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET (1ULL << 18)
> /* clang-format on */
>
> /**
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 7961c6630a2d..c7784922be3c 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -34,7 +34,7 @@
> LANDLOCK_ACCESS_FS_IOCTL_DEV)
> /* clang-format on */
>
> -typedef u16 access_mask_t;
> +typedef u32 access_mask_t;
>
> /* Makes sure all filesystem access rights can be stored. */
> static_assert(BITS_PER_TYPE(access_mask_t) >= LANDLOCK_NUM_ACCESS_FS);
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index e899995f1fd5..0645304e0375 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -37,6 +37,12 @@ static const char *const fs_access_strings[] = {
> [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer",
> [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate",
> [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev",
> + [BIT_INDEX(LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM)] =
> + "fs.resolve_unix_stream",
> + [BIT_INDEX(LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM)] =
> + "fs.resolve_unix_dgram",
> + [BIT_INDEX(LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET)] =
> + "fs.resolve_unix_seqpacket",
> };
>
> static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 8205673c8b1c..94f5fc7ee9fd 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -9,6 +9,7 @@
> * Copyright © 2023-2024 Google LLC
> */
>
> +#include "linux/net.h"
> #include <asm/ioctls.h>
> #include <kunit/test.h>
> #include <linux/atomic.h>
> @@ -314,7 +315,10 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> LANDLOCK_ACCESS_FS_WRITE_FILE | \
> LANDLOCK_ACCESS_FS_READ_FILE | \
> LANDLOCK_ACCESS_FS_TRUNCATE | \
> - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | \
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | \
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET)
> /* clang-format on */
>
> /*
> @@ -1588,6 +1592,33 @@ static int hook_path_truncate(const struct path *const path)
> return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
> }
>
> +static int hook_unix_path_connect(const struct path *const path, int type,
> + int flags)
> +{
> + access_mask_t access_request = 0;
> +
> + /* Lookup for the purpose of saving coredumps is OK. */
> + if (flags & SOCK_COREDUMP)
> + return 0;
> +
> + switch (type) {
> + case SOCK_STREAM:
> + access_request = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM;
> + break;
> + case SOCK_DGRAM:
> + access_request = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM;
> + break;
> + case SOCK_SEQPACKET:
> + access_request = LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET;
> + break;
> + }
> +
> + if (!access_request)
> + return 0;
> +
> + return current_check_access_path(path, access_request);
> +}
> +
> /* File hooks */
>
> /**
> @@ -1872,6 +1903,7 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
> LSM_HOOK_INIT(path_unlink, hook_path_unlink),
> LSM_HOOK_INIT(path_rmdir, hook_path_rmdir),
> LSM_HOOK_INIT(path_truncate, hook_path_truncate),
> + LSM_HOOK_INIT(unix_path_connect, hook_unix_path_connect),
>
> LSM_HOOK_INIT(file_alloc_security, hook_file_alloc_security),
> LSM_HOOK_INIT(file_open, hook_file_open),
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index 65b5ff051674..1f6f864afec2 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -19,7 +19,7 @@
> #define LANDLOCK_MAX_NUM_LAYERS 16
> #define LANDLOCK_MAX_NUM_RULES U32_MAX
>
> -#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_IOCTL_DEV
> +#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET
> #define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
> #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS)
>
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 0116e9f93ffe..66fd196be85a 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -161,7 +161,7 @@ static const struct file_operations ruleset_fops = {
> * Documentation/userspace-api/landlock.rst should be updated to reflect the
> * UAPI change.
> */
> -const int landlock_abi_version = 7;
> +const int landlock_abi_version = 8;
>
> /**
> * sys_landlock_create_ruleset - Create a new ruleset
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 7b69002239d7..f4b1a275d8d9 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -76,7 +76,7 @@ TEST(abi_version)
> const struct landlock_ruleset_attr ruleset_attr = {
> .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
> };
> - ASSERT_EQ(7, landlock_create_ruleset(NULL, 0,
> + ASSERT_EQ(8, landlock_create_ruleset(NULL, 0,
> LANDLOCK_CREATE_RULESET_VERSION));
>
> ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 968a91c927a4..0cbde65e032a 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -575,9 +575,12 @@ TEST_F_FORK(layout1, inval)
> LANDLOCK_ACCESS_FS_WRITE_FILE | \
> LANDLOCK_ACCESS_FS_READ_FILE | \
> LANDLOCK_ACCESS_FS_TRUNCATE | \
> - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM | \
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM | \
> + LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET)
>
> -#define ACCESS_LAST LANDLOCK_ACCESS_FS_IOCTL_DEV
> +#define ACCESS_LAST LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET
>
> #define ACCESS_ALL ( \
> ACCESS_FILE | \
^ permalink raw reply
* Re: [RFC PATCH v3 0/8] landlock: Add UDP access control support
From: Mickaël Salaün @ 2026-01-12 16:03 UTC (permalink / raw)
To: Günther Noack
Cc: Matthieu Buffet, Günther Noack, linux-security-module,
Mikhail Ivanov, konstantin.meskhidze, netdev
In-Reply-To: <20260111.f025d6aefcf4@gnoack.org>
On Sun, Jan 11, 2026 at 10:23:16PM +0100, Günther Noack wrote:
> Hello Matthieu!
>
> On Fri, Dec 12, 2025 at 05:36:56PM +0100, Matthieu Buffet wrote:
> > Here is v3 of UDP support for Landlock. My apologies for the delay, I've
> > had to deal with unrelated problems. All feedback from v1/v2 should be
> > merged, thanks again for taking the time to review them.
>
> Good to see the patch again. :)
>
> Apologies for review delay as well. There are many Landlock reviews
> in flight at the moment, it might take some time to catch up with all
> of them.
>
> FYI: In [1], I have been sending a patch for controlling UNIX socket
> lookup, which is restricting connect() and sendmsg() operations for
> UNIX domain sockets of types SOCK_STREAM, SOCK_DGRAM and
> SOCK_SEQPACKET. I am bringing it up because it feels that the
> semantics for the UDP and UNIX datagram access rights hook in similar
> places and therefore should work similarly?
Thanks for bringing this up.
>
> In the current UNIX socket patch set (v2), there is only one Landlock
> access right which controls both connect() and sendmsg() when they are
> done on a UNIX datagram socket. This feels natural to be, because you
> can reach the same recipient address whether that is done with
> connect() or with sendmsg()...?
>
> (Was there a previous discussion where it was decided that these
> should be two different access rights for UDP sockets and UNIX dgram
> sockets?)
The rationale for these three access rights (connect, bind, and sendto)
is in the related commit message and it was discussed here:
https://lore.kernel.org/all/3631edfd-7f41-4ff1-9f30-20dcaa17b726@buffet.re/
Access rights for UNIX sockets can be simpler because we always know the
peer process, which is not the case for IP requests. For the later,
being able to filter on the socket type can help.
>
> [1] https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
>
> Thanks,
> –Günther
>
^ permalink raw reply
* Re: [RFC PATCH 1/5] landlock/selftests: add a missing close(srv_fd) call
From: Mickaël Salaün @ 2026-01-12 16:04 UTC (permalink / raw)
To: Günther Noack
Cc: Paul Moore, 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
In-Reply-To: <20260110.789827dda36e@gnoack.org>
On Sat, Jan 10, 2026 at 11:37:20AM +0100, Günther Noack wrote:
> On Fri, Jan 09, 2026 at 11:49:48AM +0100, Mickaël Salaün wrote:
> > On Fri, Jan 09, 2026 at 11:41:30AM +0100, Mickaël Salaün wrote:
> > > Good, I'll pick that in my -next branch.
> > >
> > > Nit: The prefix should be "selftests/landlock"
> > >
> > > On Thu, Jan 01, 2026 at 02:40:58PM +0100, Günther Noack wrote:
> > > > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > > > ---
> > > > tools/testing/selftests/landlock/fs_test.c | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> > > > index 37a5a3df712ec..16503f2e6a481 100644
> > > > --- a/tools/testing/selftests/landlock/fs_test.c
> > > > +++ b/tools/testing/selftests/landlock/fs_test.c
> > > > @@ -4400,6 +4400,7 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
> > > > EXPECT_EQ(0, test_fionread_ioctl(cli_fd));
> > > >
> > > > ASSERT_EQ(0, close(cli_fd));
> > > > + ASSERT_EQ(0, close(srv_fd));
> >
> > I'll also replace these ASSERT_EQ() with EXPECT_EQ().
>
> Fair enough. I would normally prefer ASSERT here, because that would
> be more symmetric with the corresponding setup steps, but it feels not
> worth bikeshedding over this.
My thinking is that a close() call will not have any impact on the
tests, and it's worth cleaning things as much as possible, but an error
should not happen anyway.
As you said a few years ago (or as I remember it), we should use EXPECT
as much as possible, especially when checks don't impact following
checks. At least, that's how I see things now. ;)
>
> The selftests, both Landlock and others, are inconsistent in how they
> use ASSERT and EXPECT, especially for close().
Indeed. I try to make sure the new Landlock tests use EXPECT for
close() though. It's difficult to explain when to use ASSERT or EXPECT,
especially because they are used everywhere, and we may not even
agree...
> I wish we had an
> easier way to do state teardown in the selftests without having to tie
> it to a FIXTURE()...
>
> –Günther
>
^ permalink raw reply
* Re: [RFC PATCH 0/5] landlock: Pathname-based UNIX connect() control
From: Mickaël Salaün @ 2026-01-12 16:05 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Günther Noack, Tingmao Wang, Paul Moore,
linux-security-module, Justin Suess, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
Alyssa Ross, Jann Horn, Tahera Fahimi, Christian Brauner
In-Reply-To: <96bac7b4-9c43-4232-9899-5c1cfac409ef@gmail.com>
On Fri, Jan 09, 2026 at 04:02:25PM -0500, Demi Marie Obenour wrote:
> On 1/9/26 10:25, Mickaël Salaün wrote:
> > On Fri, Jan 09, 2026 at 06:33:10AM -0500, Demi Marie Obenour wrote:
> >> On 1/8/26 06:14, Mickaël Salaün wrote:
> >>> On Fri, Jan 02, 2026 at 01:37:14PM -0500, Demi Marie Obenour wrote:
> >>>> On 1/2/26 05:50, Günther Noack wrote:
> >>>>> On Fri, Jan 02, 2026 at 05:27:40AM -0500, Demi Marie Obenour wrote:
> >>>>>> On 1/2/26 05:16, Günther Noack wrote:
> >>>>>>> On Thu, Jan 01, 2026 at 05:44:51PM -0500, Demi Marie Obenour wrote:
> >>>>>>>> On 1/1/26 17:34, Tingmao Wang wrote:
> >>>>>>>>> On 1/1/26 22:14, Demi Marie Obenour wrote:
> >>>>>>>>>> [...]
> >>>>>>>>>> Does this leave directory traversal as the only missing Landlock
> >>>>>>>>>> filesystem access control? Ideally Landlock could provide the same
> >>>>>>>>>> isolation from the filesystem that mount namespaces do.
> >>>>>>>>>
> >>>>>>>>> I think that level of isolation would require path walk control - see:
> >>>>>>>>> https://github.com/landlock-lsm/linux/issues/9
> >>>>>>>>>
> >>>>>>>>> (Landlock also doesn't currently control some metadata operations - see
> >>>>>>>>> the warning at the end of the "Filesystem flags" section in [1])
> >>>>>>>>>
> >>>>>>>>> [1]: https://docs.kernel.org/6.18/userspace-api/landlock.html#filesystem-flags
> >>>>>>>>
> >>>>>>>> Could this replace all of the existing hooks?
> >>>>>>>
> >>>>>>> If you do not need to distinguish between the different operations
> >>>>>>> which Landlock offers access rights for, but you only want to limit
> >>>>>>> the visibility of directory hierarchies in the file system, then yes,
> >>>>>>> the path walk control described in issue 9 would be sufficient and a
> >>>>>>> more complete control.
> >>>>>>>
> >>>>>>> The path walk control is probably among the more difficult Landlock
> >>>>>>> feature requests. A simple implementation would be easy to implement
> >>>>>>> technically, but it also requires a new LSM hook which will have to
> >>>>>>> get called *during* path lookup, and we'd have to make sure that the
> >>>>>>> performance impact stays in check. Path lookup is after all a very
> >>>>>>> central facility in a OS kernel.
> >>>>>>
> >>>>>> What about instead using the inode-based hooks for directory searching?
> >>>>>> SELinux can already restrict that.
> >>>>>
> >>>>> Oh, thanks, good pointer! I was under the impression that this didn't
> >>>>> exist yet -- I assume you are referring to the
> >>>>> security_inode_follow_link() hook, which is already happening during
> >>>>> path resolution?
> >>>>
> >>>> I'm not familiar with existing LSM hooks, but I do know that SELinux
> >>>> enforces checks on searching and reading directories and symlinks.
> >>>
> >>> SELinux uses inode-based hooks, which is not (directly) possible for
> >>> Landlock because it is an unprivileged access control, which means it
> >>> cannot rely on extended file attributes to define a security policy.
> >>>
> >>> See https://github.com/landlock-lsm/linux/issues/9
> >>
> >> Could Landlock use a side table, with the inode's address in memory
> >> as the key?
> >
> > A struct inode is not enough because we need to resolve a file
> > hierarchy, which is only possible with a struct path.
>
> Could Landlock "piggyback" on the core kernel's own path resolution,
> updating its state when each hook gets called?
Most of the inode-based hooks are called in places where there is no
path data (in the current context). We should be very careful with
changes to the VFS wrt performance and complexity. If you have an idea
of how to do it, I encourage you to test it and send an RFC.
^ permalink raw reply
* Re: [PATCH] landlock: Clarify documentation for the IOCTL access right
From: Mickaël Salaün @ 2026-01-12 16:07 UTC (permalink / raw)
To: Günther Noack
Cc: linux-security-module, Tingmao Wang, Samasth Norway Ananda
In-Reply-To: <20260111175203.6545-2-gnoack3000@gmail.com>
On Sun, Jan 11, 2026 at 06:52:04PM +0100, Günther Noack wrote:
> Move the description of the LANDLOCK_ACCESS_FS_IOCTL_DEV access right
> together with the file access rights.
>
> This group of access rights applies to files (in this case device
> files), and they can be added to file or directory inodes using
> landlock_add_rule(2). The check for that works the same for all file
> access rights, including LANDLOCK_ACCESS_FS_IOCTL_DEV.
>
> Invoking ioctl(2) on directory FDs can not currently be restricted
> with Landlock. Having it grouped separately in the documentation is a
> remnant from earlier revisions of the LANDLOCK_ACCESS_FS_IOCTL_DEV
> patch set.
>
> Link: https://lore.kernel.org/all/20260108.Thaex5ruach2@digikod.net/
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
Thanks, applied.
> ---
> include/uapi/linux/landlock.h | 37 ++++++++++++++++-------------------
> 1 file changed, 17 insertions(+), 20 deletions(-)
>
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index eac65da687c1..fbd18cf60a88 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -216,6 +216,23 @@ struct landlock_net_port_attr {
> * :manpage:`ftruncate(2)`, :manpage:`creat(2)`, or :manpage:`open(2)` with
> * ``O_TRUNC``. This access right is available since the third version of the
> * Landlock ABI.
> + * - %LANDLOCK_ACCESS_FS_IOCTL_DEV: Invoke :manpage:`ioctl(2)` commands on an opened
> + * character or block device.
> + *
> + * This access right applies to all `ioctl(2)` commands implemented by device
> + * drivers. However, the following common IOCTL commands continue to be
> + * invokable independent of the %LANDLOCK_ACCESS_FS_IOCTL_DEV right:
> + *
> + * * IOCTL commands targeting file descriptors (``FIOCLEX``, ``FIONCLEX``),
> + * * IOCTL commands targeting file descriptions (``FIONBIO``, ``FIOASYNC``),
> + * * IOCTL commands targeting file systems (``FIFREEZE``, ``FITHAW``,
> + * ``FIGETBSZ``, ``FS_IOC_GETFSUUID``, ``FS_IOC_GETFSSYSFSPATH``)
> + * * Some IOCTL commands which do not make sense when used with devices, but
> + * whose implementations are safe and return the right error codes
> + * (``FS_IOC_FIEMAP``, ``FICLONE``, ``FICLONERANGE``, ``FIDEDUPERANGE``)
> + *
> + * This access right is available since the fifth version of the Landlock
> + * ABI.
> *
> * Whether an opened file can be truncated with :manpage:`ftruncate(2)` or used
> * with `ioctl(2)` is determined during :manpage:`open(2)`, in the same way as
> @@ -275,26 +292,6 @@ struct landlock_net_port_attr {
> * If multiple requirements are not met, the ``EACCES`` error code takes
> * precedence over ``EXDEV``.
> *
> - * The following access right applies both to files and directories:
> - *
> - * - %LANDLOCK_ACCESS_FS_IOCTL_DEV: Invoke :manpage:`ioctl(2)` commands on an opened
> - * character or block device.
> - *
> - * This access right applies to all `ioctl(2)` commands implemented by device
> - * drivers. However, the following common IOCTL commands continue to be
> - * invokable independent of the %LANDLOCK_ACCESS_FS_IOCTL_DEV right:
> - *
> - * * IOCTL commands targeting file descriptors (``FIOCLEX``, ``FIONCLEX``),
> - * * IOCTL commands targeting file descriptions (``FIONBIO``, ``FIOASYNC``),
> - * * IOCTL commands targeting file systems (``FIFREEZE``, ``FITHAW``,
> - * ``FIGETBSZ``, ``FS_IOC_GETFSUUID``, ``FS_IOC_GETFSSYSFSPATH``)
> - * * Some IOCTL commands which do not make sense when used with devices, but
> - * whose implementations are safe and return the right error codes
> - * (``FS_IOC_FIEMAP``, ``FICLONE``, ``FICLONERANGE``, ``FIDEDUPERANGE``)
> - *
> - * This access right is available since the fifth version of the Landlock
> - * ABI.
> - *
> * .. warning::
> *
> * It is currently not possible to restrict some file-related actions
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH v2 0/5] landlock: Pathname-based UNIX connect() control
From: Mickaël Salaün @ 2026-01-12 16:08 UTC (permalink / raw)
To: Günther Noack
Cc: Paul Moore, James Morris, Serge E . Hallyn, 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,
Simon Horman, netdev, Alexander Viro, Christian Brauner
In-Reply-To: <20260110143300.71048-2-gnoack3000@gmail.com>
On Sat, Jan 10, 2026 at 03:32:55PM +0100, Günther Noack wrote:
> 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 a file system access right for each
> type of UNIX domain socket:
>
> * LANDLOCK_ACCESS_FS_RESOLVE_UNIX_STREAM
> * LANDLOCK_ACCESS_FS_RESOLVE_UNIX_DGRAM
> * LANDLOCK_ACCESS_FS_RESOLVE_UNIX_SEQPACKET
>
> 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.
>
> ## 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 can come as a surprise for users (e.g. in
> [1]) and it can be used to escape a sandbox when a Unix service offers
> command execution (some scenarios were listed by Tingmao Wang in [2]).
>
> The original feature request is at [4].
>
> ## Alternatives and Related Work
>
> ### Alternative: Use existing LSM hooks
>
> The existing hooks security_unix_stream_connect(),
> security_unix_may_send() and security_socket_connect() do not give
> access to the resolved file system path.
>
> Resolving the file system path again within Landlock would in my
> understanding produce a TOCTOU race, so making the decision based on
> the struct sockaddr_un contents is not an option.
>
> It is tempting to use the struct path that the listening socket is
> bound to, which can be acquired through the existing hooks.
> Unfortunately, the listening socket may have been bound from within a
> different namespace, and it is therefore a path that can not actually
> be referenced by the sandboxed program at the time of constructing the
> Landlock policy. (More details are on the Github issue at [6] and on
> the LKML at [9]).
Please move (or duplicate) this rationale in the patch dedicated to the
new hook. It helps patch review (and to understand commits when already
merged).
>
> ### 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 my reply to this patch set [3], I have discussed the differences
> between these two approaches. On the related discussions on Github
> [4] and [5], there was consensus that the scope-based control is
> complimentary to the file system based control, but does not replace
> it. Mickael's opening remark on [5] says:
>
> > This scoping would be complementary to #36 which would mainly be
> > about allowing a sandboxed process to connect to a more privileged
> > service (identified with a path).
>
> ## Open questions in V2
>
> Seeking feedback on:
>
> - Feedback on the LSM hook name would be appreciated. We realize that
> not all invocations of the LSM hook are related to connect(2) as the
> name suggests, but some also happen during sendmsg(2).
Renaming security_unix_path_connect() to security_unix_find() would look
appropriate to me wrt the caller.
> - Feedback on the structuring of the Landlock access rights, splitting
> them up by socket type. (Also naming; they are now consistently
> called "RESOLVE", but could be named "CONNECT" in the stream and
> seqpacket cases?)
I don't see use cases where differenciating the type of unix socket
would be useful. LANDLOCK_ACCESS_FS_RESOLVE_UNIX would look good to me.
Tests should still cover all these types though.
What would be the inverse of "resolve" (i.e. to restrict the server
side)? Would LANDLOCK_ACCESS_FS_MAKE_SOCK be enough?
>
> ## 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] and Tingmao Wang has
> sent the patch set for the scoped access control for pathname Unix
> sockets [2].
>
> Justin Suess has sent the patch for the LSM hook in [8].
>
> Ryan Sullivan has started on an initial implementation and has brought
> up relevant discussion points on the Github issue at [4] that lead to
> the current approach.
>
> [1] https://lore.kernel.org/landlock/515ff0f4-2ab3-46de-8d1e-5c66a93c6ede@gmail.com/
> [2] Tingmao Wang's "Implemnet 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/
>
> ---
>
> ## Older versions of this patch set
>
> V1: https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
>
> 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 (4):
> landlock: Control pathname UNIX domain socket resolution by path
> samples/landlock: Add support for named UNIX domain socket
> restrictions
> landlock/selftests: Test named UNIX domain socket restrictions
> landlock: Document FS access rights for pathname UNIX sockets
>
> Justin Suess (1):
> lsm: Add hook unix_path_connect
>
> Documentation/userspace-api/landlock.rst | 25 ++-
> include/linux/lsm_hook_defs.h | 4 +
> include/linux/security.h | 11 +
> include/uapi/linux/landlock.h | 10 +
> net/unix/af_unix.c | 9 +
> samples/landlock/sandboxer.c | 18 +-
> security/landlock/access.h | 2 +-
> security/landlock/audit.c | 6 +
> security/landlock/fs.c | 34 ++-
> security/landlock/limits.h | 2 +-
> security/landlock/syscalls.c | 2 +-
> security/security.c | 20 ++
> tools/testing/selftests/landlock/base_test.c | 2 +-
> tools/testing/selftests/landlock/fs_test.c | 225 +++++++++++++++++--
> 14 files changed, 344 insertions(+), 26 deletions(-)
>
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates
From: Frederick Lawler @ 2026-01-12 17:14 UTC (permalink / raw)
To: Mimi Zohar
Cc: Jeff Layton, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, linux-kernel, linux-integrity,
linux-security-module, kernel-team
In-Reply-To: <15eaa3613b0552cc48b55972b81882ac1e1d1150.camel@linux.ibm.com>
On Mon, Jan 12, 2026 at 09:02:02AM -0500, Mimi Zohar wrote:
> On Tue, 2026-01-06 at 14:50 -0500, Jeff Layton wrote:
> > > > > > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
> > > > > > */
> > > > > > static inline bool
> > > > > > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > > > > > - const struct inode *inode)
> > > > > > + struct file *file, struct inode *inode)
> > > > > > {
> > > > > > - return (inode->i_sb->s_dev != attrs->dev ||
> > > > > > - inode->i_ino != attrs->ino ||
> > > > > > - !inode_eq_iversion(inode, attrs->version));
> > > > > > + struct kstat stat;
> > > > > > +
> > > > > > + if (inode->i_sb->s_dev != attrs->dev ||
> > > > > > + inode->i_ino != attrs->ino)
> > > > > > + return true;
> > > > > > +
> > > > > > + if (inode_eq_iversion(inode, attrs->version))
> > > > > > + return false;
> > > > > > +
> > > > > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME,
> > > > > > + AT_STATX_SYNC_AS_STAT))
> > > > > > + return true;
> > > > > > +
> > > > >
> > > > > This is rather odd. You're sampling the i_version field directly, but
> > > > > if it's not equal then you go through ->getattr() to get the ctime.
> > > > >
> > > > > It's particularly odd since you don't know whether the i_version field
> > > > > is even implemented on the fs. On filesystems where it isn't, the
> > > > > i_version field generally stays at 0, so won't this never fall through
> > > > > to do the vfs_getattr_nosec() call on those filesystems?
> > > > >
> > > >
> > > > You're totally right. I didn't consider FS's caching the value at zero.
> > >
> > > Actually, I'm going to amend this. I think I did consider FSs without an
> > > implementation. Where this is called at, it is often guarded by a
> > > !IS_I_VERSION() || integrity_inode_attrs_change(). If I'm
> > > understanding this correctly, the check call doesn't occur unless the inode
> > > has i_version support.
> > >
> >
> >
> > It depends on what you mean by i_version support:
> >
> > That flag just tells the VFS that it needs to bump the i_version field
> > when updating timestamps. It's not a reliable indicator of whether the
> > i_version field is suitable for the purpose you want here.
> >
> > The problem here and the one that we ultimately fixed with multigrain
> > timestamps is that XFS in particular will bump i_version on any change
> > to the log. That includes atime updates due to reads.
> >
> > XFS still tracks the i_version the way it always has, but we've stopped
> > getattr() from reporting it because it's not suitable for the purpose
> > that nfsd (and IMA) need it for.
> >
> > > It seems to me the suggestion then is to remove the IS_I_VERSION()
> > > checks guarding the call sites, grab both ctime and cookie from stat,
> > > and if IS_I_VERSION() use that, otherwise cookie, and compare
> > > against the cached i_version with one of those values, and then fall
> > > back to ctime?
> > >
> >
> > Not exactly.
> >
> > You want to call getattr() for STATX_CHANGE_COOKIE|STATX_CTIME, and
> > then check the kstat->result_mask. If STATX_CHANGE_COOKIE is set, then
> > use that. If it's not then use the ctime.
> >
> > The part I'm not sure about is whether it's actually safe to do this.
> > vfs_getattr_nosec() can block in some situations. Is it ok to do this
> > in any context where integrity_inode_attrs_changed() may be called?
>
> Frederick, before making any changes, please describe the problem you're
> actually seeing. From my limited testing, file change IS being detected. A major
> change like Jeff is suggesting is not something that would or should be back
> ported. Remember, Jeff's interest is remote filesystems, not necessarily with
> your particular XFS concern.
>
> So again, what is the problem you're trying to address?
It's easier if I paste a simpler version of test I've been promising
for v1 to help show this (below).
In 6.12 the test snippet passes, for >= 6.13 we get an audit
evaluation on the each execution when there should only be 1.
The struct integrity_inode_attributes.version stays at zero for XFS
in the below test, as well as file systems that calls into
generic_fillattr() or otherwise that doesn't set the change cookie
request mask.
When file systems have a mutated file, the cookie is then updated,
but the compare against inode.i_version could be out of date depending
on file system implementation. Thus we see since 6.13, XFS an atime change
will cause another evaluation due to stale cache.
I'm not expecting a backport to 6.13 as there has been a lot of changes
in IMA/EVM, but I think to the 6.18 LTS is reasonable. With leaving
EVM alone, it's a small diff.
I have a updated patch that hopefully addresses all your concerns
from other responses in this thread. I want to point out that the updated
code is more EVM/IMA invariant mindful than this RFC. I'd
like to submit that, and then move discussion over there if possible.
Hopefully this helps,
Fred
_fragment.config_
CONFIG_XFS_FS=y
CONFIG_OVERLAY_FS=y
CONFIG_IMA=y
CONFIG_IMA_WRITE_POLICY=y
CONFIG_IMA_READ_POLICY=y
_./test.sh_
#!/bin/bash -e
IMA_POLICY="/sys/kernel/security/ima/policy"
TEST_BIN="/bin/date"
MNT_BASE="/tmp/ima_test_root"
mkdir -p "$MNT_BASE"
mount -t tmpfs tmpfs "$MNT_BASE"
mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl}
dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300
mkfs.xfs -q "$MNT_BASE/xfs.img"
mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk"
cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog"
mount -t overlay overlay -o \
"lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \
"$MNT_BASE/ovl"
echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
target_prog="$MNT_BASE/ovl/test_prog"
setpriv --reuid nobody "$target_prog"
setpriv --reuid nobody "$target_prog"
setpriv --reuid nobody "$target_prog"
audit_count=$(dmesg | grep -c "file=\"$target_prog\"")
if [[ "$audit_count" -eq 1 ]]; then
echo "PASS: Found exactly 1 audit event."
else
echo "FAIL: Expected 1 audit event, but found $audit_count."
exit 1
fi
>
> Mimi
>
> >
> > ISTR that this was an issue at one point, but maybe isn't now that IMA
> > is an LSM?
>
^ 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