* [PATCH v4 05/17] module: Switch load_info::len to size_t
From: Thomas Weißschuh @ 2026-01-13 12:28 UTC (permalink / raw)
To: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng, Nicolas Schier,
Christophe Leroy
Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity, Thomas Weißschuh
In-Reply-To: <20260113-module-hashes-v4-0-0b932db9b56b@weissschuh.net>
Switching the types will make some later changes cleaner.
size_t is also the semantically correct type for this field.
As both 'size_t' and 'unsigned int' are always the same size, this
should be risk-free.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
kernel/module/internal.h | 2 +-
kernel/module/main.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index e68fbcd60c35..037fbb3b7168 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -66,7 +66,7 @@ struct load_info {
/* pointer to module in temporary copy, freed at end of load_module() */
struct module *mod;
Elf_Ehdr *hdr;
- unsigned long len;
+ size_t len;
Elf_Shdr *sechdrs;
char *secstrings, *strtab;
unsigned long symoffs, stroffs, init_typeoffs, core_typeoffs;
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 710ee30b3bea..a88f95a13e06 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1838,7 +1838,7 @@ static int validate_section_offset(const struct load_info *info, Elf_Shdr *shdr)
static int elf_validity_ehdr(const struct load_info *info)
{
if (info->len < sizeof(*(info->hdr))) {
- pr_err("Invalid ELF header len %lu\n", info->len);
+ pr_err("Invalid ELF header len %zu\n", info->len);
return -ENOEXEC;
}
if (memcmp(info->hdr->e_ident, ELFMAG, SELFMAG) != 0) {
--
2.52.0
^ permalink raw reply related
* [PATCH v4 01/17] module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y
From: Thomas Weißschuh @ 2026-01-13 12:28 UTC (permalink / raw)
To: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng, Nicolas Schier,
Christophe Leroy
Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity, Thomas Weißschuh, Coiby Xu,
kernel test robot
In-Reply-To: <20260113-module-hashes-v4-0-0b932db9b56b@weissschuh.net>
From: Coiby Xu <coxu@redhat.com>
Currently if set_module_sig_enforced is called with CONFIG_MODULE_SIG=n
e.g. [1], it can lead to a linking error,
ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'
This happens because the actual implementation of
set_module_sig_enforced comes from CONFIG_MODULE_SIG but both the
function declaration and the empty stub definition are tied to
CONFIG_MODULES.
So bind set_module_sig_enforced to CONFIG_MODULE_SIG instead. This
allows (future) users to call set_module_sig_enforced directly without
the "if IS_ENABLED(CONFIG_MODULE_SIG)" safeguard.
Note this issue hasn't caused a real problem because all current callers
of set_module_sig_enforced e.g. security/integrity/ima/ima_efi.c
use "if IS_ENABLED(CONFIG_MODULE_SIG)" safeguard.
[1] https://lore.kernel.org/lkml/20250928030358.3873311-1-coxu@redhat.com/
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
From modules/modules-next
---
include/linux/module.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index d80c3ea57472..f288ca5cd95b 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -770,8 +770,6 @@ static inline bool is_livepatch_module(struct module *mod)
#endif
}
-void set_module_sig_enforced(void);
-
void module_for_each_mod(int(*func)(struct module *mod, void *data), void *data);
#else /* !CONFIG_MODULES... */
@@ -866,10 +864,6 @@ static inline bool module_requested_async_probing(struct module *module)
}
-static inline void set_module_sig_enforced(void)
-{
-}
-
/* Dereference module function descriptor */
static inline
void *dereference_module_function_descriptor(struct module *mod, void *ptr)
@@ -925,6 +919,8 @@ static inline bool retpoline_module_ok(bool has_retpoline)
#ifdef CONFIG_MODULE_SIG
bool is_module_sig_enforced(void);
+void set_module_sig_enforced(void);
+
static inline bool module_sig_ok(struct module *module)
{
return module->sig_ok;
@@ -935,6 +931,10 @@ static inline bool is_module_sig_enforced(void)
return false;
}
+static inline void set_module_sig_enforced(void)
+{
+}
+
static inline bool module_sig_ok(struct module *module)
{
return true;
--
2.52.0
^ permalink raw reply related
* [PATCH v4 00/17] module: Introduce hash-based integrity checking
From: Thomas Weißschuh @ 2026-01-13 12:28 UTC (permalink / raw)
To: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng, Nicolas Schier,
Christophe Leroy
Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity, Thomas Weißschuh, Coiby Xu,
kernel test robot
The current signature-based module integrity checking has some drawbacks
in combination with reproducible builds. Either the module signing key
is generated at build time, which makes the build unreproducible, or a
static signing key is used, which precludes rebuilds by third parties
and makes the whole build and packaging process much more complicated.
The goal is to reach bit-for-bit reproducibility. Excluding certain
parts of the build output from the reproducibility analysis would be
error-prone and force each downstream consumer to introduce new tooling.
Introduce a new mechanism to ensure only well-known modules are loaded
by embedding a merkle tree root of all modules built as part of the full
kernel build into vmlinux.
Interest has been proclaimed by NixOS, Arch Linux, Proxmox, SUSE and the
general reproducible builds community.
Compatibility with IMA modsig is not provided yet. It is still unclear
to me if it should be hooked up transparently without any changes to the
policy or it should require new policy options.
Further improvements:
* Use MODULE_SIG_HASH for configuration
* UAPI for discovery?
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Changes in v4:
- Use as Merkle tree over a linera list of hashes.
- Provide compatibilith with INSTALL_MOD_STRIP
- Rework commit messages.
- Use vmlinux.unstripped over plain "vmlinux".
- Link to v3: https://lore.kernel.org/r/20250429-module-hashes-v3-0-00e9258def9e@weissschuh.net
Changes in v3:
- Rebase on v6.15-rc1
- Use openssl to calculate hash
- Avoid warning if no modules are built
- Simplify module_integrity_check() a bit
- Make incompatibility with INSTALL_MOD_STRIP explicit
- Update docs
- Add IMA cleanups
- Link to v2: https://lore.kernel.org/r/20250120-module-hashes-v2-0-ba1184e27b7f@weissschuh.net
Changes in v2:
- Drop RFC state
- Mention interested parties in cover letter
- Expand Kconfig description
- Add compatibility with CONFIG_MODULE_SIG
- Parallelize module-hashes.sh
- Update Documentation/kbuild/reproducible-builds.rst
- Link to v1: https://lore.kernel.org/r/20241225-module-hashes-v1-0-d710ce7a3fd1@weissschuh.net
---
Coiby Xu (1):
module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y
Thomas Weißschuh (16):
powerpc/ima: Drop unnecessary check for CONFIG_MODULE_SIG
ima: efi: Drop unnecessary check for CONFIG_MODULE_SIG/CONFIG_KEXEC_SIG
module: Make mod_verify_sig() static
module: Switch load_info::len to size_t
kbuild: add stamp file for vmlinux BTF data
kbuild: generate module BTF based on vmlinux.unstripped
module: Deduplicate signature extraction
module: Make module loading policy usable without MODULE_SIG
module: Move integrity checks into dedicated function
module: Move lockdown check into generic module loader
module: Move signature splitting up
module: Report signature type to users
lockdown: Make the relationship to MODULE_SIG a dependency
module: Introduce hash-based integrity checking
kbuild: move handling of module stripping to Makefile.lib
kbuild: make CONFIG_MODULE_HASHES compatible with module stripping
.gitignore | 2 +
Documentation/kbuild/reproducible-builds.rst | 5 +-
Makefile | 8 +-
arch/powerpc/kernel/ima_arch.c | 3 +-
include/asm-generic/vmlinux.lds.h | 11 +
include/linux/module.h | 20 +-
include/linux/module_hashes.h | 25 ++
include/linux/module_signature.h | 5 +-
kernel/module/Kconfig | 29 +-
kernel/module/Makefile | 1 +
kernel/module/hashes.c | 92 ++++++
kernel/module/hashes_root.c | 6 +
kernel/module/internal.h | 13 +-
kernel/module/main.c | 68 +++-
kernel/module/signing.c | 83 +----
kernel/module_signature.c | 49 ++-
scripts/.gitignore | 1 +
scripts/Makefile | 3 +
scripts/Makefile.lib | 32 ++
scripts/Makefile.modfinal | 28 +-
scripts/Makefile.modinst | 46 +--
scripts/Makefile.vmlinux | 6 +
scripts/link-vmlinux.sh | 20 +-
scripts/modules-merkle-tree.c | 467 +++++++++++++++++++++++++++
security/integrity/ima/ima_efi.c | 6 +-
security/integrity/ima/ima_modsig.c | 28 +-
security/lockdown/Kconfig | 2 +-
27 files changed, 884 insertions(+), 175 deletions(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20241225-module-hashes-7a50a7cc2a30
Best regards,
--
Thomas Weißschuh <linux@weissschuh.net>
^ permalink raw reply
* [PATCH v4 03/17] ima: efi: Drop unnecessary check for CONFIG_MODULE_SIG/CONFIG_KEXEC_SIG
From: Thomas Weißschuh @ 2026-01-13 12:28 UTC (permalink / raw)
To: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng, Nicolas Schier,
Christophe Leroy
Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity, Thomas Weißschuh
In-Reply-To: <20260113-module-hashes-v4-0-0b932db9b56b@weissschuh.net>
When configuration settings are disabled the guarded functions are
defined as empty stubs, so the check is unnecessary.
The specific configuration option for set_module_sig_enforced() is
about to change and removing the checks avoids some later churn.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
security/integrity/ima/ima_efi.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/security/integrity/ima/ima_efi.c b/security/integrity/ima/ima_efi.c
index 138029bfcce1..a35dd166ad47 100644
--- a/security/integrity/ima/ima_efi.c
+++ b/security/integrity/ima/ima_efi.c
@@ -68,10 +68,8 @@ static const char * const sb_arch_rules[] = {
const char * const *arch_get_ima_policy(void)
{
if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) {
- if (IS_ENABLED(CONFIG_MODULE_SIG))
- set_module_sig_enforced();
- if (IS_ENABLED(CONFIG_KEXEC_SIG))
- set_kexec_sig_enforced();
+ set_module_sig_enforced();
+ set_kexec_sig_enforced();
return sb_arch_rules;
}
return NULL;
--
2.52.0
^ permalink raw reply related
* [PATCH v4 02/17] powerpc/ima: Drop unnecessary check for CONFIG_MODULE_SIG
From: Thomas Weißschuh @ 2026-01-13 12:28 UTC (permalink / raw)
To: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng, Nicolas Schier,
Christophe Leroy
Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity, Thomas Weißschuh
In-Reply-To: <20260113-module-hashes-v4-0-0b932db9b56b@weissschuh.net>
When CONFIG_MODULE_SIG is disabled set_module_sig_enforced() is defined
as an empty stub, so the check is unnecessary.
The specific configuration option for set_module_sig_enforced() is
about to change and removing the check avoids some later churn.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
arch/powerpc/kernel/ima_arch.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c
index b7029beed847..690263bf4265 100644
--- a/arch/powerpc/kernel/ima_arch.c
+++ b/arch/powerpc/kernel/ima_arch.c
@@ -63,8 +63,7 @@ static const char *const secure_and_trusted_rules[] = {
const char *const *arch_get_ima_policy(void)
{
if (is_ppc_secureboot_enabled()) {
- if (IS_ENABLED(CONFIG_MODULE_SIG))
- set_module_sig_enforced();
+ set_module_sig_enforced();
if (is_ppc_trustedboot_enabled())
return secure_and_trusted_rules;
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v2 1/5] lsm: Add hook unix_path_connect
From: Christian Brauner @ 2026-01-13 9:34 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, Paul Moore, James Morris,
Serge E . Hallyn, 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, Simon Horman, netdev,
Alexander Viro
In-Reply-To: <20260110143300.71048-4-gnoack3000@gmail.com>
On Sat, Jan 10, 2026 at 03:32:57PM +0100, 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;
Couldn't we try reflowing the code here so the path is passed to
security_unix_stream_connect() and security_unix_may_send() so that all
LSMs get the same data and we don't have to have different LSMs hooks
into different callpaths that effectively do the same thing.
I mean the objects are even in two completely different states between
those hooks. Even what type of sockets get a call to the LSM is
different between those two hooks.
^ permalink raw reply
* [PATCH] landlock: Fix unsigned comparison in get_id_range()
From: Samasth Norway Ananda @ 2026-01-13 6:36 UTC (permalink / raw)
To: mic, gnoack; +Cc: linux-security-module, linux-kernel
Fix unsigned comparison where size_t variable is compared with '<= 0'.
Change to '== 0' to match the actual intent.
Signed-off-by: Samasth Norway Ananda <samasth.norway.ananda@oracle.com>
---
security/landlock/id.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/landlock/id.c b/security/landlock/id.c
index 838c3ed7bb82..5424cb3d7ee3 100644
--- a/security/landlock/id.c
+++ b/security/landlock/id.c
@@ -96,7 +96,7 @@ static u64 get_id_range(size_t number_of_ids, atomic64_t *const counter,
* We should return at least 1 ID, and we may need a set of consecutive
* ones (e.g. to generate a set of inodes).
*/
- if (WARN_ON_ONCE(number_of_ids <= 0))
+ if (WARN_ON_ONCE(number_of_ids == 0))
number_of_ids = 1;
/*
--
2.50.1
^ permalink raw reply related
* Re: [PATCH v6 05/11] tpm2-sessions: Remove AUTH_MAX_NAMES
From: ross.philipson @ 2026-01-13 0:22 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-integrity
Cc: Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20251214153808.73831-6-jarkko@kernel.org>
On 12/14/25 7:38 AM, Jarkko Sakkinen wrote:
> In all of the call sites only one session is ever append. Thus, reduce
> AUTH_MAX_NAMES, which leads into removing constant completely.
>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> drivers/char/tpm/tpm2-sessions.c | 31 +++++++++++--------------------
> 1 file changed, 11 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> index 3bc3c31cf512..37570dc088cf 100644
> --- a/drivers/char/tpm/tpm2-sessions.c
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -72,9 +72,6 @@
> #include <crypto/sha2.h>
> #include <crypto/utils.h>
>
> -/* maximum number of names the TPM must remember for authorization */
> -#define AUTH_MAX_NAMES 3
> -
> #define AES_KEY_BYTES AES_KEYSIZE_128
> #define AES_KEY_BITS (AES_KEY_BYTES*8)
>
> @@ -136,8 +133,8 @@ struct tpm2_auth {
> * handle, but they are part of the session by name, which
> * we must compute and remember
> */
> - u8 name[AUTH_MAX_NAMES][TPM2_MAX_NAME_SIZE];
> - u16 name_size_tbl[AUTH_MAX_NAMES];
> + u8 name[TPM2_MAX_NAME_SIZE];
> + u16 name_size;
> };
>
> #ifdef CONFIG_TCG_TPM2_HMAC
> @@ -261,11 +258,14 @@ EXPORT_SYMBOL_GPL(tpm2_read_public);
> int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> u32 handle, u8 *name, u16 name_size)
> {
> -#ifdef CONFIG_TCG_TPM2_HMAC
Removing CONFIG_TCG_TPM2_HMAC here causes a warning during compile since
the auth variable is only used in the CONFIG_TCG_TPM2_HMAC block below.
Ross
> struct tpm2_auth *auth;
> - int slot;
> int ret;
> -#endif
> +
> + if (tpm_buf_length(buf) != TPM_HEADER_SIZE) {
> + dev_err(&chip->dev, "too many handles\n");
> + ret = -EIO;
> + goto err;
> + }
>
> if (!tpm2_chip_auth(chip)) {
> tpm_buf_append_handle(chip, buf, handle);
> @@ -273,12 +273,6 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> }
>
> #ifdef CONFIG_TCG_TPM2_HMAC
> - slot = (tpm_buf_length(buf) - TPM_HEADER_SIZE) / 4;
> - if (slot >= AUTH_MAX_NAMES) {
> - dev_err(&chip->dev, "too many handles\n");
> - ret = -EIO;
> - goto err;
> - }
> auth = chip->auth;
> if (auth->session != tpm_buf_length(buf)) {
> dev_err(&chip->dev, "session state malformed");
> @@ -287,16 +281,14 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
> }
> tpm_buf_append_u32(buf, handle);
> auth->session += 4;
> - memcpy(auth->name[slot], name, name_size);
> - auth->name_size_tbl[slot] = name_size;
> + memcpy(auth->name, name, name_size);
> + auth->name_size = name_size;
> #endif
> return 0;
>
> -#ifdef CONFIG_TCG_TPM2_HMAC
> err:
> tpm2_end_auth_session(chip);
> return ret;
> -#endif
> }
> EXPORT_SYMBOL_GPL(tpm_buf_append_name);
>
> @@ -665,8 +657,7 @@ int tpm_buf_fill_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf)
> /* ordinal is already BE */
> sha256_update(&sctx, (u8 *)&head->ordinal, sizeof(head->ordinal));
> /* add the handle names */
> - for (i = 0; i < handles; i++)
> - sha256_update(&sctx, auth->name[i], auth->name_size_tbl[i]);
> + sha256_update(&sctx, auth->name, auth->name_size);
> if (offset_s != tpm_buf_length(buf))
> sha256_update(&sctx, &buf->data[offset_s],
> tpm_buf_length(buf) - offset_s);
^ permalink raw reply
* [PATCH] ima: Detect changes to files via kstat changes rather than i_version
From: Frederick Lawler @ 2026-01-12 22:32 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team,
Frederick Lawler
Commit 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
introduced a means to track change detection for an inode
via ctime updates, opposed to setting kstat.change_cookie when
calling into xfs_vn_getattr().
This introduced a regression because IMA caches kstat.change_cookie
to compare against an inode's i_version directly in
integrity_inode_attrs_changed(), and thus could be out of date
depending on how file systems increment i_version.
To address this, require integrity_inode_attrs_changed() to query
vfs_getattr_nosec() to compare the cached version against
kstat.change_cookie directly. This ensures that when updates occur,
we're accessing the same changed inode version on changes, and fallback
to compare against an artificial version generated from kstat.ctime
via integrity_ctime_guard() when there's no detected change
to the kstat.change_cookie.
This ensures that in the absence of i_version support for file systems,
and in the absence of a kstat.change_cookie update, we ultimately have a
unique-enough version to compare against.
The exact implementation for integrity_ctime_guard() is to ensure that
if tv_sec or tv_nsec are zero, there's some value to store back into
struct integrity_inode_attributes.version. This also avoids the need to
add additional storage and comparisons.
Lastly, because EVM still relies on querying and caching a backing inode's
i_version, the integrity_inode_attrs_changed() falls back to the
original inode.i_version != cached comparison. This maintains the
invariant that a re-evaluation in unknown change detection circumstances
is required.
Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
Suggested-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Frederick Lawler <fred@cloudflare.com>
---
We uncovered a case in kernels >= 6.13 where XFS is no longer updating
struct kstat.change_cookie on i_op getattr() access calls. Instead, XFS is
using multigrain ctime (as well as other file systems) for
change detection in commit 1cf7e834a6fb ("xfs: switch to
multigrain timestamps").
Because file systems may implement i_version as they see fit, IMA
caching may be behind as well as file systems that don't support/export
i_version. Thus we're proposing to compare against the kstat.change_cookie
directly to the cached version, and fall back to a ctime guard when
that's not updated.
EVM is largely left alone since there's no trivial way to query a file
directly in the LSM call paths to obtain kstat.change_cookie &
kstat.ctime to cache. Thus retains accessing i_version directly.
Regression tests will be added to the Linux Test Project instead of
selftest to help catch future file system changes that may impact
future evaluation of IMA.
I'd like this to be backported to at least 6.18 if possible.
Below is a simplified test that demonstrates the issue:
_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_
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
---
Changes since RFC:
- Remove calls to I_IS_VERSION()
- Function documentation/comments
- Abide IMA/EVM change detection fallback invariants
- Combined ctime guard into version for attributes struct
- Link to RFC: https://lore.kernel.org/r/20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com
---
include/linux/integrity.h | 42 +++++++++++++++++++++++++++++++++++----
security/integrity/evm/evm_main.c | 5 ++---
security/integrity/ima/ima_api.c | 11 +++++++---
security/integrity/ima/ima_main.c | 15 +++++---------
4 files changed, 53 insertions(+), 20 deletions(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index f5842372359be5341b6870a43b92e695e8fc78af..5eca8aa2769f9238c68bb40885ecc46910524f11 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -9,6 +9,7 @@
#include <linux/fs.h>
#include <linux/iversion.h>
+#include <linux/kernel.h>
enum integrity_status {
INTEGRITY_PASS = 0,
@@ -36,6 +37,14 @@ struct integrity_inode_attributes {
dev_t dev;
};
+/*
+ * Wrapper to generate an artificial version for a file.
+ */
+static inline u64 integrity_ctime_guard(struct kstat stat)
+{
+ return stat.ctime.tv_sec ^ stat.ctime.tv_nsec;
+}
+
/*
* On stacked filesystems the i_version alone is not enough to detect file data
* or metadata change. Additional metadata is required.
@@ -51,14 +60,39 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs,
/*
* On stacked filesystems detect whether the inode or its content has changed.
+ *
+ * Must be called in process context.
*/
static inline bool
integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
- const struct inode *inode)
+ struct file *file, struct inode *inode)
{
- return (inode->i_sb->s_dev != attrs->dev ||
- inode->i_ino != attrs->ino ||
- !inode_eq_iversion(inode, attrs->version));
+ struct kstat stat;
+
+ might_sleep();
+
+ if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
+ return true;
+
+ /*
+ * EVM currently relies on backing inode i_version. While IS_I_VERSION
+ * is not a good indicator of i_version support, this still retains
+ * the logic such that a re-evaluation should still occur for EVM, and
+ * only for IMA if vfs_getattr_nosec() fails.
+ */
+ if (!file || vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
+ AT_STATX_SYNC_AS_STAT))
+ return !IS_I_VERSION(inode) ||
+ !inode_eq_iversion(inode, attrs->version);
+
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ return stat.change_cookie != attrs->version;
+
+ if (stat.result_mask & STATX_CTIME)
+ return integrity_ctime_guard(stat) != attrs->version;
+
+ return true;
}
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
bool ret = false;
if (iint) {
- ret = (!IS_I_VERSION(metadata_inode) ||
- integrity_inode_attrs_changed(&iint->metadata_inode,
- metadata_inode));
+ ret = integrity_inode_attrs_changed(&iint->metadata_inode,
+ NULL, metadata_inode);
if (ret)
iint->evm_status = INTEGRITY_UNKNOWN;
}
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c35ea613c9f8d404ba4886e3b736c3bab29d1668..8096986f3689781d3cdf6595f330033782f9cc45 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -272,10 +272,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
* to an initial measurement/appraisal/audit, but was modified to
* assume the file changed.
*/
- result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
+ result = vfs_getattr_nosec(&file->f_path, &stat,
+ STATX_CHANGE_COOKIE | STATX_CTIME,
AT_STATX_SYNC_AS_STAT);
- if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
- i_version = stat.change_cookie;
+ if (!result) {
+ if (stat.result_mask & STATX_CHANGE_COOKIE)
+ i_version = stat.change_cookie;
+ else if (stat.result_mask & STATX_CTIME)
+ i_version = integrity_ctime_guard(stat);
+ }
hash.hdr.algo = algo;
hash.hdr.length = hash_digest_size[algo];
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 5770cf691912aa912fc65280c59f5baac35dd725..3a4c32e254f925bba85cb91b63744ac142b3b049 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -22,6 +22,7 @@
#include <linux/mount.h>
#include <linux/mman.h>
#include <linux/slab.h>
+#include <linux/stat.h>
#include <linux/xattr.h>
#include <linux/ima.h>
#include <linux/fs.h>
@@ -191,18 +192,13 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
mutex_lock(&iint->mutex);
if (atomic_read(&inode->i_writecount) == 1) {
- struct kstat stat;
-
clear_bit(IMA_EMITTED_OPENWRITERS, &iint->atomic_flags);
update = test_and_clear_bit(IMA_UPDATE_XATTR,
&iint->atomic_flags);
if ((iint->flags & IMA_NEW_FILE) ||
- vfs_getattr_nosec(&file->f_path, &stat,
- STATX_CHANGE_COOKIE,
- AT_STATX_SYNC_AS_STAT) ||
- !(stat.result_mask & STATX_CHANGE_COOKIE) ||
- stat.change_cookie != iint->real_inode.version) {
+ integrity_inode_attrs_changed(&iint->real_inode, file,
+ inode)) {
iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
iint->measured_pcrs = 0;
if (update)
@@ -328,9 +324,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
real_inode = d_real_inode(file_dentry(file));
if (real_inode != inode &&
(action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
- if (!IS_I_VERSION(real_inode) ||
- integrity_inode_attrs_changed(&iint->real_inode,
- real_inode)) {
+ if (integrity_inode_attrs_changed(&iint->real_inode,
+ file, real_inode)) {
iint->flags &= ~IMA_DONE_MASK;
iint->measured_pcrs = 0;
}
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251212-xfs-ima-fixup-931780a62c2c
Best regards,
--
Frederick Lawler <fred@cloudflare.com>
^ permalink raw reply related
* Re: [PATCH v5 36/36] sched: Enable context analysis for core.c and fair.c
From: Bart Van Assche @ 2026-01-12 22:04 UTC (permalink / raw)
To: Marco Elver, Peter Zijlstra, Boqun Feng, Ingo Molnar, Will Deacon
Cc: 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, Ingo Molnar
In-Reply-To: <20251219154418.3592607-37-elver@google.com>
On 12/19/25 8:40 AM, Marco Elver wrote:
> diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
> index a63f65aa5bdd..a22248aebcf9 100644
> --- a/include/linux/sched/signal.h
> +++ b/include/linux/sched/signal.h
> @@ -738,10 +738,12 @@ static inline int thread_group_empty(struct task_struct *p)
> (thread_group_leader(p) && !thread_group_empty(p))
>
> extern struct sighand_struct *lock_task_sighand(struct task_struct *task,
> - unsigned long *flags);
> + unsigned long *flags)
> + __acquires(&task->sighand->siglock);
I think the above annotation is wrong and should be changed into
__cond_acquires(nonnull, &task->sighand->siglock). My understanding of
the code in kernel/signal.c is that lock_task_sighand() only returns
with sighand->siglock acquired if it returns a non-NULL pointer.
Bart.
^ permalink raw reply
* Re: [PATCH v2 0/5] landlock: Pathname-based UNIX connect() control
From: Günther Noack @ 2026-01-12 20:53 UTC (permalink / raw)
To: Mickaël Salaün, Justin Suess
Cc: 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: <20260112.Wufar9coosoo@digikod.net>
Thanks for the review!
On Mon, Jan 12, 2026 at 05:08:02PM +0100, Mickaël Salaün wrote:
> On Sat, Jan 10, 2026 at 03:32:55PM +0100, Günther Noack wrote:
> > ## 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).
Justin, would you like to look into this?
Please feel free to copy the wording.
> > ### 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.
Justin, this is also on your commit. (I find security_unix_find() and
security_unix_resolve() equally acceptable options.)
> > - 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.
I did it mostly because it seemed consistent with the TCP and (soon)
UDP controls, which are also controls specific to the socket type and
not just the address family. But I agree that the granularity is
likely not needed here. I can change it back for v3 and rename it to
LANDLOCK_ACCESS_FS_RESOLVE_UNIX.
> What would be the inverse of "resolve" (i.e. to restrict the server
> side)? Would LANDLOCK_ACCESS_FS_MAKE_SOCK be enough?
Yes, that would be enough. My reasoning is as follows:
The server-side operation that is related to associating the service
with a given file system name is bind(2), and that is restrictable in
that case using LANDLOCK_ACCESS_FS_MAKE_SOCK.
Also, to my delight (and other than in TCP), listening on an unbound
socket does not work (see unix_listen() in af_unix.c):
if (!READ_ONCE(u->addr))
goto out; /* No listens on an unbound socket */
(You can get it to "autobind" during an explicit bind() or a connect()
call, but that creates an abstract UNIX address. (Documented in
socket(7) and implemented in unix_autobind() in af_unix.c))
–Günther
^ 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
* 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] 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: [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: [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 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: [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: [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 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] 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: [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: [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 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 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
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