Linux Security Modules development
 help / color / mirror / Atom feed
* Re: [PATCH v5 06/10] security: Hornet LSM
From: Fan Wu @ 2026-04-21  0:08 UTC (permalink / raw)
  To: Blaise Boscaccy
  Cc: Jonathan Corbet, Paul Moore, James Morris, Serge E. Hallyn,
	Mickaël Salaün, Günther Noack,
	Dr. David Alan Gilbert, Andrew Morton, James.Bottomley, dhowells,
	Fan Wu, Ryan Foster, Randy Dunlap, linux-security-module,
	linux-doc, linux-kernel, bpf, Song Liu
In-Reply-To: <20260420212653.438685-7-bboscaccy@linux.microsoft.com>

On Mon, Apr 20, 2026 at 2:27 PM Blaise Boscaccy
<bboscaccy@linux.microsoft.com> wrote:
>
> This adds the Hornet Linux Security Module which provides enhanced
> signature verification and data validation for eBPF programs. This
> allows users to continue to maintain an invariant that all code
> running inside of the kernel has actually been signed and verified, by
> the kernel.
>
> This effort builds upon the currently excepted upstream solution. It
> further hardens it by providing deterministic, in-kernel checking of
> map hashes to solidify auditing along with preventing TOCTOU attacks
> against lskel map hashes.
>
> Target map hashes are passed in via PKCS#7 signed attributes. Hornet
> determines the extent which the eBFP program is signed and defers to
> other LSMs for policy decisions.
>
> Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
> Nacked-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>

...

> diff --git a/security/hornet/Kconfig b/security/hornet/Kconfig
> new file mode 100644
> index 0000000000000..19406aa237ac6
> --- /dev/null
> +++ b/security/hornet/Kconfig
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config SECURITY_HORNET
> +       bool "Hornet support"
> +       depends on SECURITY

The dependency doesn't seem to be complete, for example,
PKCS7_MESSAGE_PARSER is not selected.

> +       default n
> +       help
> +         This selects Hornet.
> +         Further information can be found in
> +         Documentation/admin-guide/LSM/Hornet.rst.
> +
> +         If you are unsure how to answer this question, answer N.
> diff --git a/security/hornet/Makefile b/security/hornet/Makefile
> new file mode 100644
> index 0000000000000..26b6f954f762e
> --- /dev/null
> +++ b/security/hornet/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_SECURITY_HORNET) := hornet.o
> +
> +hornet-y := hornet.asn1.o \
> +       hornet_lsm.o \
> +
> +$(obj)/hornet.asn1.o: $(obj)/hornet.asn1.c $(obj)/hornet.asn1.h
> diff --git a/security/hornet/hornet.asn1 b/security/hornet/hornet.asn1
> new file mode 100644
> index 0000000000000..c8d47b16b65d7
> --- /dev/null
> +++ b/security/hornet/hornet.asn1
> @@ -0,0 +1,13 @@
> +-- SPDX-License-Identifier: BSD-3-Clause
> +--
> +-- Copyright (C) 2009 IETF Trust and the persons identified as authors
> +-- of the code

I'm not a lawyer, but since this is a new AA which is not in the RFC,
should the copyright belong to IETF?

> +--
> +-- https://www.rfc-editor.org/rfc/rfc5652#section-3
> +
> +HornetData ::= SET OF Map
> +
> +Map ::= SEQUENCE {
> +       index                   INTEGER ({ hornet_map_index }),
> +       sha                     OCTET STRING ({ hornet_map_hash })
> +} ({ hornet_next_map })
> diff --git a/security/hornet/hornet_lsm.c b/security/hornet/hornet_lsm.c
> new file mode 100644
> index 0000000000000..f7d62fe6229c9
> --- /dev/null
> +++ b/security/hornet/hornet_lsm.c
> @@ -0,0 +1,346 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Hornet Linux Security Module
> + *
> + * Author: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
> + *
> + * Copyright (C) 2026 Microsoft Corporation
> + */
> +
> +#include <linux/lsm_hooks.h>
> +#include <uapi/linux/lsm.h>
> +#include <linux/bpf.h>
> +#include <linux/verification.h>
> +#include <crypto/public_key.h>
> +#include <linux/module_signature.h>
> +#include <crypto/pkcs7.h>
> +#include <linux/sort.h>
> +#include <linux/asn1_decoder.h>
> +#include <linux/oid_registry.h>
> +#include "hornet.asn1.h"
> +
> +#define MAX_USED_MAPS 64
> +
> +struct hornet_maps {
> +       bpfptr_t fd_array;
> +};
> +
> +/* The only hashing algorithm available is SHA256 due to it be hardcoded
> +   in the bpf subsystem. */
> +
> +struct hornet_parse_context {
> +       int indexes[MAX_USED_MAPS];
> +       bool skips[MAX_USED_MAPS];
> +       unsigned char hashes[SHA256_DIGEST_SIZE * MAX_USED_MAPS];
> +       int hash_count;
> +};
> +
> +struct hornet_prog_security_struct {
> +       bool checked[MAX_USED_MAPS];
> +       unsigned char hashes[SHA256_DIGEST_SIZE * MAX_USED_MAPS];
> +};
> +
> +struct hornet_map_security_struct {
> +       bool checked;
> +       int index;
> +};
> +

Can maps be shared between programs? If so, since the LSM blob is
per-map, a second program's load will overwrite map_security->index
set by the first. A later run of the first program would then fail to
find its own map.

> +struct lsm_blob_sizes hornet_blob_sizes __ro_after_init = {
> +       .lbs_bpf_map = sizeof(struct hornet_map_security_struct),
> +       .lbs_bpf_prog = sizeof(struct hornet_prog_security_struct),
> +};
> +
> +static inline struct hornet_prog_security_struct *
> +hornet_bpf_prog_security(struct bpf_prog *prog)
> +{
> +       return prog->aux->security + hornet_blob_sizes.lbs_bpf_prog;
> +}
> +
> +static inline struct hornet_map_security_struct *
> +hornet_bpf_map_security(struct bpf_map *map)
> +{
> +       return map->security + hornet_blob_sizes.lbs_bpf_map;
> +}
> +
> +static int hornet_verify_hashes(struct hornet_maps *maps,
> +                               struct hornet_parse_context *ctx,
> +                               struct bpf_prog *prog)
> +{
> +       int map_fd;
> +       u32 i;
> +       struct bpf_map *map;
> +       int err = 0;
> +       unsigned char hash[SHA256_DIGEST_SIZE];
> +       struct hornet_prog_security_struct *security = hornet_bpf_prog_security(prog);
> +       struct hornet_map_security_struct *map_security;
> +
> +       for (i = 0; i < ctx->hash_count; i++) {
> +               if (ctx->skips[i])
> +                       continue;
> +
> +               err = copy_from_bpfptr_offset(&map_fd, maps->fd_array,
> +                                             ctx->indexes[i] * sizeof(map_fd),
> +                                             sizeof(map_fd));
> +               if (err < 0)
> +                       return LSM_INT_VERDICT_FAULT;
> +
> +               CLASS(fd, f)(map_fd);
> +               if (fd_empty(f))
> +                       return LSM_INT_VERDICT_FAULT;
> +               if (unlikely(fd_file(f)->f_op != &bpf_map_fops))
> +                       return LSM_INT_VERDICT_FAULT;
> +
> +               map = fd_file(f)->private_data;
> +               if (!map->frozen)
> +                       return LSM_INT_VERDICT_FAULT;
> +
> +               map->ops->map_get_hash(map, SHA256_DIGEST_SIZE, hash);
> +
> +               err = memcmp(hash, &ctx->hashes[i * SHA256_DIGEST_SIZE],
> +                             SHA256_DIGEST_SIZE);
> +               if (err)
> +                       return LSM_INT_VERDICT_UNEXPECTED;
> +
> +               security->checked[i] = true;
> +               memcpy(&security->hashes[i * SHA256_DIGEST_SIZE], hash, SHA256_DIGEST_SIZE);
> +               map_security = hornet_bpf_map_security(map);
> +               map_security->checked = true;
> +               map_security->index = i;
> +       }
> +       return LSM_INT_VERDICT_OK;
> +}
> +
> +int hornet_next_map(void *context, size_t hdrlen,
> +                    unsigned char tag,
> +                    const void *value, size_t vlen)
> +{
> +       struct hornet_parse_context *ctx = (struct hornet_parse_context *)context;
> +
> +       if (++ctx->hash_count >= MAX_USED_MAPS)
> +               return -EINVAL;
> +       return 0;
> +}
> +
> +int hornet_map_index(void *context, size_t hdrlen,
> +                    unsigned char tag,
> +                    const void *value, size_t vlen)
> +{
> +       struct hornet_parse_context *ctx = (struct hornet_parse_context *)context;
> +
> +       if (vlen > 1)
> +               return -EINVAL;
> +
> +       ctx->indexes[ctx->hash_count] = *(u8 *)value;
> +       return 0;
> +}
> +
> +int hornet_map_hash(void *context, size_t hdrlen,
> +                   unsigned char tag,
> +                   const void *value, size_t vlen)
> +
> +{
> +       struct hornet_parse_context *ctx = (struct hornet_parse_context *)context;
> +
> +       if (vlen != SHA256_DIGEST_SIZE && vlen != 0)
> +               return -EINVAL;
> +
> +       if (vlen) {
> +               ctx->skips[ctx->hash_count] = false;
> +               memcpy(&ctx->hashes[ctx->hash_count * SHA256_DIGEST_SIZE], value, vlen);
> +       } else
> +               ctx->skips[ctx->hash_count] = true;
> +
> +       return 0;
> +}
> +
> +static int hornet_check_program(struct bpf_prog *prog, union bpf_attr *attr,
> +                               struct bpf_token *token, bool is_kernel,
> +                               enum lsm_integrity_verdict *verdict)
> +{
> +       struct hornet_maps maps = {0};
> +       bpfptr_t usig = make_bpfptr(attr->signature, is_kernel);
> +       struct pkcs7_message *msg;
> +       struct hornet_parse_context *ctx;
> +       void *sig;
> +       int err;
> +       const void *authattrs;
> +       size_t authattrs_len;
> +       struct key *key;
> +
> +       if (!attr->signature) {
> +               *verdict = LSM_INT_VERDICT_UNSIGNED;
> +               return 0;
> +       }
> +
> +       ctx = kzalloc(sizeof(struct hornet_parse_context), GFP_KERNEL);
> +       if (!ctx)
> +               return -ENOMEM;
> +
> +       maps.fd_array = make_bpfptr(attr->fd_array, is_kernel);
> +       sig = kzalloc(attr->signature_size, GFP_KERNEL);
> +       if (!sig) {
> +               err = -ENOMEM;
> +               goto out;
> +       }
> +       err = copy_from_bpfptr(sig, usig, attr->signature_size);
> +       if (err != 0)
> +               goto cleanup_sig;
> +
> +       msg = pkcs7_parse_message(sig, attr->signature_size);
> +       if (IS_ERR(msg)) {
> +               *verdict = LSM_INT_VERDICT_BADSIG;
> +               err = 0;
> +               goto cleanup_sig;
> +       }
> +
> +       if (system_keyring_id_check(attr->keyring_id) == 0)
> +               key = (struct key*)(unsigned long)attr->keyring_id;
> +       else
> +               key = key_ref_to_ptr(lookup_user_key(attr->keyring_id, 0, KEY_DEFER_PERM_CHECK));

You might need to key_put the user key.

> +
> +       if (verify_pkcs7_message_sig(prog->insnsi, prog->len * sizeof(struct bpf_insn), msg,
> +                                    key,
> +                                    VERIFYING_BPF_SIGNATURE,
> +                                    NULL, NULL)) {
> +               *verdict = LSM_INT_VERDICT_UNKNOWNKEY;
> +               err = 0;
> +               goto cleanup_msg;
> +       }
> +
> +       if (pkcs7_get_authattr(msg, OID_hornet_data,
> +                              &authattrs, &authattrs_len) == -ENODATA) {
> +               *verdict = LSM_INT_VERDICT_PARTIALSIG;
> +               err = 0;
> +               goto cleanup_msg;
> +       }
> +
> +       err = asn1_ber_decoder(&hornet_decoder, ctx, authattrs, authattrs_len);
> +       if (err < 0 || authattrs == NULL) {
> +               *verdict = LSM_INT_VERDICT_BADSIG;
> +               err = 0;
> +               goto cleanup_msg;
> +       }
> +
> +       err = hornet_verify_hashes(&maps, ctx, prog);
> +       if (err == 0)
> +               *verdict = LSM_INT_VERDICT_OK;
> +       else
> +               *verdict = err;
> +
> +cleanup_msg:
> +       pkcs7_free_message(msg);
> +cleanup_sig:
> +       kfree(sig);
> +out:
> +       kfree(ctx);
> +       return err;
> +}
> +
> +static const struct lsm_id hornet_lsmid = {
> +       .name = "hornet",
> +       .id = LSM_ID_HORNET,
> +};
> +
> +static int hornet_bpf_prog_load_integrity(struct bpf_prog *prog, union bpf_attr *attr,
> +                                         struct bpf_token *token, bool is_kernel)
> +{
> +       enum lsm_integrity_verdict verdict;
> +       int result = hornet_check_program(prog, attr, token, is_kernel, &verdict);
> +
> +       if (result < 0)
> +               return result;
> +
> +       return security_bpf_prog_load_post_integrity(prog, attr, token, is_kernel,
> +                                                    &hornet_lsmid, verdict);
> +}
> +
> +static int hornet_verify_map(struct bpf_prog *prog, int index)
> +{
> +       unsigned char hash[SHA256_DIGEST_SIZE];
> +       int i;
> +       struct bpf_map *map;
> +       struct hornet_prog_security_struct *security = hornet_bpf_prog_security(prog);
> +       struct hornet_map_security_struct *map_security;
> +
> +       if (!security->checked[index])
> +               return 0;
> +
> +       for (i = 0; i < prog->aux->used_map_cnt; i++) {
> +               map = prog->aux->used_maps[i];
> +               map_security = hornet_bpf_map_security(map);
> +               if (map_security->index != index)
> +                       continue;
> +
> +               if (!map->frozen)
> +                       return -EPERM;
> +
> +               map->ops->map_get_hash(map, SHA256_DIGEST_SIZE, hash);
> +               if (memcmp(hash, &security->hashes[index * SHA256_DIGEST_SIZE],
> +                          SHA256_DIGEST_SIZE) != 0)
> +                       return -EPERM;
> +               else
> +                       return 0;
> +       }
> +       return -EINVAL;
> +}
> +
> +static int hornet_check_prog_maps(u32 ufd)
> +{
> +       CLASS(fd, f)(ufd);
> +       struct bpf_prog *prog;
> +       int i, result = 0;
> +
> +       if (fd_empty(f))
> +               return -EBADF;
> +       if (fd_file(f)->f_op != &bpf_prog_fops)
> +               return -EINVAL;
> +
> +       prog = fd_file(f)->private_data;
> +
> +       mutex_lock(&prog->aux->used_maps_mutex);
> +       if (!prog->aux->used_map_cnt)
> +               goto out;
> +
> +       for (i = 0; i < prog->aux->used_map_cnt; i++) {
> +               result = hornet_verify_map(prog, i);
> +               if (result)
> +                       goto out;
> +       }

This loop is kind of confusing for me, I guess it's just iterating
through the maps the program currently in use. I feel the nested loop
is unnecessary and the hash could be saved into the map's blob
instead. Please correct me if I'm wrong here.

-Fan

> +out:
> +       mutex_unlock(&prog->aux->used_maps_mutex);
> +       return result;
> +}
> +
> +static int hornet_bpf(int cmd, union bpf_attr *attr, unsigned int size, bool kernel)
> +{
> +       /* in horent_bpf(), anything that had originated from kernel space we assume
> +          has already been checked, in some form or another, so we don't bother
> +          checking the intergity of any maps. In hornet_bpf_prog_load_integrity(),
> +          hornet doesn't make any opinion on that and delegates that to the downstream
> +          policy enforcement. */
> +
> +       if (cmd != BPF_PROG_RUN)
> +               return 0;
> +       if (kernel)
> +               return 0;
> +
> +       return hornet_check_prog_maps(attr->test.prog_fd);
> +}
> +
> +static struct security_hook_list hornet_hooks[] __ro_after_init = {
> +       LSM_HOOK_INIT(bpf_prog_load_integrity, hornet_bpf_prog_load_integrity),
> +       LSM_HOOK_INIT(bpf, hornet_bpf),
> +};
> +
> +static int __init hornet_init(void)
> +{
> +       pr_info("Hornet: eBPF signature verification enabled\n");
> +       security_add_hooks(hornet_hooks, ARRAY_SIZE(hornet_hooks), &hornet_lsmid);
> +       return 0;
> +}
> +
> +DEFINE_LSM(hornet) = {
> +       .id = &hornet_lsmid,
> +       .blobs = &hornet_blob_sizes,
> +       .init = hornet_init,
> +};
> --
> 2.53.0
>

^ permalink raw reply

* Re: [PATCH v5 07/10] hornet: Introduce gen_sig
From: Fan Wu @ 2026-04-21  0:18 UTC (permalink / raw)
  To: Blaise Boscaccy
  Cc: Jonathan Corbet, Paul Moore, James Morris, Serge E. Hallyn,
	Mickaël Salaün, Günther Noack,
	Dr. David Alan Gilbert, Andrew Morton, James.Bottomley, dhowells,
	Fan Wu, Ryan Foster, Randy Dunlap, linux-security-module,
	linux-doc, linux-kernel, bpf, Song Liu
In-Reply-To: <20260420212653.438685-8-bboscaccy@linux.microsoft.com>

On Mon, Apr 20, 2026 at 2:27 PM Blaise Boscaccy
<bboscaccy@linux.microsoft.com> wrote:
>
> This introduces the gen_sig tool. It creates a pkcs#7 signature of a
> data payload. Additionally it appends a signed attribute containing a
> set of hashes.
>
> Typical usage is to provide a payload containing the light skeleton
> ebpf syscall program binary and it's associated maps, which can be
> extracted from the auto-generated skeleton header.
>
> Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
> ---
>  scripts/Makefile            |   1 +
>  scripts/hornet/Makefile     |   5 +
>  scripts/hornet/gen_sig.c    | 392 ++++++++++++++++++++++++++++++++++++
>  scripts/hornet/write-sig.sh |  27 +++
>  4 files changed, 425 insertions(+)
>  create mode 100644 scripts/hornet/Makefile
>  create mode 100644 scripts/hornet/gen_sig.c
>  create mode 100755 scripts/hornet/write-sig.sh
>
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 0941e5ce7b575..dea8ab91bbe4e 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -63,6 +63,7 @@ subdir-$(CONFIG_GENKSYMS) += genksyms
>  subdir-$(CONFIG_GENDWARFKSYMS) += gendwarfksyms
>  subdir-$(CONFIG_SECURITY_SELINUX) += selinux
>  subdir-$(CONFIG_SECURITY_IPE) += ipe
> +subdir-$(CONFIG_SECURITY_HORNET) += hornet
>
>  # Let clean descend into subdirs
>  subdir-        += basic dtc gdb kconfig mod
> diff --git a/scripts/hornet/Makefile b/scripts/hornet/Makefile
> new file mode 100644
> index 0000000000000..3ee41e5e9a9ff
> --- /dev/null
> +++ b/scripts/hornet/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0
> +hostprogs-always-y     := gen_sig
> +
> +HOSTCFLAGS_gen_sig.o = $(shell $(HOSTPKG_CONFIG) --cflags libcrypto 2> /dev/null)
> +HOSTLDLIBS_gen_sig = $(shell $(HOSTPKG_CONFIG) --libs libcrypto 2> /dev/null || echo -lcrypto)
> diff --git a/scripts/hornet/gen_sig.c b/scripts/hornet/gen_sig.c
> new file mode 100644
> index 0000000000000..f966516ebc99b
> --- /dev/null
> +++ b/scripts/hornet/gen_sig.c
> @@ -0,0 +1,392 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> + *
> + * Generate a signature for an eBPF program along with appending
> + * map hashes as signed attributes
> + *
> + * Copyright © 2025      Microsoft Corporation.
> + *
> + * Authors: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public License
> + * as published by the Free Software Foundation; either version 2.1
> + * of the licence, or (at your option) any later version.
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <err.h>
> +#include <getopt.h>
> +
> +#include <openssl/cms.h>
> +#include <openssl/err.h>
> +#include <openssl/evp.h>
> +#include <openssl/pkcs7.h>
> +#include <openssl/x509.h>
> +#include <openssl/pem.h>
> +#include <openssl/objects.h>
> +#include <openssl/asn1.h>
> +#include <openssl/asn1t.h>
> +#include <openssl/opensslv.h>
> +#include <openssl/bio.h>
> +#include <openssl/stack.h>
> +
> +#if OPENSSL_VERSION_MAJOR >= 3
> +# define USE_PKCS11_PROVIDER
> +# include <openssl/provider.h>
> +# include <openssl/store.h>
> +#else
> +# if !defined(OPENSSL_NO_ENGINE) && !defined(OPENSSL_NO_DEPRECATED_3_0)
> +#  define USE_PKCS11_ENGINE
> +#  include <openssl/engine.h>
> +# endif
> +#endif
> +#include "../ssl-common.h"
> +
> +#define SHA256_LEN 32
> +#define BUF_SIZE   (1 << 15) // 32 KiB
> +#define MAX_HASHES 64
> +
> +struct hash_spec {
> +       char *file;
> +       int index;
> +};
> +
> +typedef struct {
> +       ASN1_INTEGER *index;
> +       ASN1_OCTET_STRING *hash;
> +
> +} HORNET_MAP;
> +
> +DECLARE_ASN1_FUNCTIONS(HORNET_MAP)
> +ASN1_SEQUENCE(HORNET_MAP) = {
> +       ASN1_SIMPLE(HORNET_MAP, index, ASN1_INTEGER),
> +       ASN1_SIMPLE(HORNET_MAP, hash, ASN1_OCTET_STRING)
> +} ASN1_SEQUENCE_END(HORNET_MAP);
> +
> +IMPLEMENT_ASN1_FUNCTIONS(HORNET_MAP)
> +
> +DEFINE_STACK_OF(HORNET_MAP)
> +
> +typedef struct {
> +       STACK_OF(HORNET_MAP) * maps;
> +} MAP_SET;
> +
> +DECLARE_ASN1_FUNCTIONS(MAP_SET)
> +ASN1_SEQUENCE(MAP_SET) = {
> +       ASN1_SET_OF(MAP_SET, maps, HORNET_MAP)
> +} ASN1_SEQUENCE_END(MAP_SET);
> +
> +IMPLEMENT_ASN1_FUNCTIONS(MAP_SET)
> +
> +#define DIE(...) do { fprintf(stderr, __VA_ARGS__); fputc('\n', stderr); \
> +               exit(EXIT_FAILURE); } while (0)
> +
> +static BIO *bio_open_wr(const char *path)
> +{
> +       BIO *b = BIO_new_file(path, "wb");
> +
> +       if (!b) {
> +               perror(path);
> +               ERR_print_errors_fp(stderr);
> +               exit(EXIT_FAILURE);
> +       }
> +       return b;
> +}
> +
> +static void usage(const char *prog)
> +{
> +       fprintf(stderr,
> +               "Usage:\n"
> +               "  %s --data content.bin --cert signer.crt --key signer.key [-pass pass]\n"
> +               "     --out newsig.p7b \n"
> +               "     --add FILE:index [--add FILE:index ...]\n",
> +               prog);
> +}
> +
> +static const char *key_pass;
> +
> +static int pem_pw_cb(char *buf, int len, int w, void *v)
> +{
> +       int pwlen;
> +
> +       if (!key_pass)
> +               return -1;
> +
> +       pwlen = strlen(key_pass);
> +       if (pwlen >= len)
> +               return -1;
> +
> +       strcpy(buf, key_pass);
> +
> +       key_pass = NULL;
> +
> +       return pwlen;
> +}
> +
> +static EVP_PKEY *read_private_key(const char *private_key_name)
> +{
> +       EVP_PKEY *private_key;
> +       BIO *b;
> +
> +       b = BIO_new_file(private_key_name, "rb");
> +       ERR(!b, "%s", private_key_name);
> +       private_key = PEM_read_bio_PrivateKey(b, NULL, pem_pw_cb,
> +                                             NULL);
> +       ERR(!private_key, "%s", private_key_name);
> +       BIO_free(b);
> +
> +       return private_key;
> +}
> +
> +static X509 *read_x509(const char *x509_name)
> +{
> +       unsigned char buf[2];
> +       X509 *x509;
> +       BIO *b;
> +       int n;
> +
> +       b = BIO_new_file(x509_name, "rb");
> +       ERR(!b, "%s", x509_name);
> +
> +       /* Look at the first two bytes of the file to determine the encoding */
> +       n = BIO_read(b, buf, 2);
> +       if (n != 2) {
> +               if (BIO_should_retry(b)) {
> +                       fprintf(stderr, "%s: Read wanted retry\n", x509_name);
> +                       exit(1);
> +               }
> +               if (n >= 0) {
> +                       fprintf(stderr, "%s: Short read\n", x509_name);
> +                       exit(1);
> +               }
> +               ERR(1, "%s", x509_name);
> +       }
> +
> +       ERR(BIO_reset(b) != 0, "%s", x509_name);
> +
> +       if (buf[0] == 0x30 && buf[1] >= 0x81 && buf[1] <= 0x84)
> +               /* Assume raw DER encoded X.509 */
> +               x509 = d2i_X509_bio(b, NULL);
> +       else
> +               /* Assume PEM encoded X.509 */
> +               x509 = PEM_read_bio_X509(b, NULL, NULL, NULL);
> +
> +       BIO_free(b);
> +       ERR(!x509, "%s", x509_name);
> +
> +       return x509;
> +}
> +
> +static int sha256(const char *path, unsigned char out[SHA256_LEN], unsigned int *out_len)
> +{
> +       FILE *f;
> +       int rc;
> +       EVP_MD_CTX *ctx;
> +       unsigned char buf[BUF_SIZE];
> +       size_t n;
> +       unsigned int mdlen = 0;
> +
> +       if (!path || !out)
> +               return -1;
> +
> +       f = fopen(path, "rb");
> +       if (!f) {
> +               perror("fopen");
> +               return -2;
> +       }
> +
> +       ERR_load_crypto_strings();
> +
> +       rc = -3;
> +       ctx = EVP_MD_CTX_new();
> +       if (!ctx) {
> +               rc = -4;
> +               goto done;
> +       }
> +
> +#if OPENSSL_VERSION_NUMBER >= 0x30000000L
> +       if (EVP_DigestInit_ex2(ctx, EVP_sha256(), NULL) != 1) {
> +               rc = -5;
> +               goto done;
> +       }
> +#else
> +       if (EVP_DigestInit_ex(ctx, EVP_sha256(), NULL) != 1) {
> +               rc = -5;
> +               goto done;
> +       }
> +#endif
> +       while ((n = fread(buf, 1, sizeof(buf), f)) > 0) {
> +               if (EVP_DigestUpdate(ctx, buf, n) != 1) {
> +                       rc = -6;
> +                       goto done;
> +               }
> +       }
> +       if (ferror(f)) {
> +               rc = -7;
> +               goto done;
> +       }
> +
> +       if (EVP_DigestFinal_ex(ctx, out, &mdlen) != 1) {
> +               rc = -8;
> +               goto done;
> +       }
> +       if (mdlen != SHA256_LEN) {
> +               rc = -9;
> +               goto done;
> +       }
> +
> +       if (out_len)
> +               *out_len = mdlen;
> +       rc = 0;
> +
> +done:
> +       EVP_MD_CTX_free(ctx);
> +       fclose(f);
> +       ERR_free_strings();
> +       return rc;
> +}
> +
> +static void add_hash(MAP_SET *set, unsigned char *buffer, int buffer_len, int index)
> +{
> +       HORNET_MAP *map = NULL;
> +
> +       map = HORNET_MAP_new();
> +       ASN1_INTEGER_set(map->index, index);
> +       ASN1_OCTET_STRING_set(map->hash, buffer, buffer_len);
> +       sk_HORNET_MAP_push(set->maps, map);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +       const char *cert_path = NULL;
> +       const char *key_path = NULL;
> +       const char *data_path = NULL;
> +       const char *out_path = NULL;
> +
> +       X509 *signer;
> +       EVP_PKEY *pkey;
> +       BIO *data_in;
> +       CMS_ContentInfo *cms_out;
> +       struct hash_spec hashes[MAX_HASHES];
> +       int hash_count = 0;
> +       int flags;
> +       CMS_SignerInfo *si;
> +       MAP_SET *set;
> +       unsigned char hash_buffer[SHA256_LEN];
> +       unsigned int hash_len;
> +       ASN1_OBJECT *oid;
> +       unsigned char *der = NULL;
> +       int der_len;
> +       int err;
> +       BIO *b_out;
> +       int i;
> +       char opt;
> +
> +       const char *short_opts = "C:K:P:O:A:Sh";
> +
> +       static const struct option long_opts[] = {
> +               {"cert", required_argument, 0, 'C'},
> +               {"key",  required_argument, 0, 'K'},
> +               {"pass",  required_argument, 0, 'P'},
> +               {"out",  required_argument, 0, 'O'},
> +               {"data",  required_argument, 0, 'D'},
> +               {"add",  required_argument, 0, 'A'},
> +               {"help",    no_argument,       0, 'h'},
> +               {0, 0, 0, 0}
> +       };
> +
> +       while ((opt = getopt_long_only(argc, argv, short_opts, long_opts, NULL)) != -1) {
> +               switch (opt) {
> +               case 'C':
> +                       cert_path = optarg;
> +                       break;
> +               case 'K':
> +                       key_path = optarg;
> +                       break;
> +               case 'P':
> +                       key_pass = optarg;
> +                       break;
> +               case 'O':
> +                       out_path = optarg;
> +                       break;
> +               case 'D':
> +                       data_path = optarg;
> +                       break;
> +               case 'A':
> +                       if (strchr(optarg, ':')) {
> +                               hashes[hash_count].file = strsep(&optarg, ":");
> +                               hashes[hash_count].index = atoi(optarg);
> +                               hash_count++;

Should the hash_count be compared to MAX_HASHES here?

> +                       } else {
> +                               usage(argv[0]);
> +                               return EXIT_FAILURE;
> +                       }
> +               }
> +       }
> +
> +       if (!cert_path || !key_path || !out_path || !data_path) {
> +               usage(argv[0]);
> +               return EXIT_FAILURE;
> +       }
> +
> +       OpenSSL_add_all_algorithms();
> +       ERR_load_crypto_strings();
> +
> +       signer = read_x509(cert_path);
> +       ERR(!signer, "Load cert failed");
> +
> +       pkey = read_private_key(key_path);
> +       ERR(!pkey, "Load key failed");
> +
> +       data_in = BIO_new_file(data_path, "rb");
> +       ERR(!data_in, "Load data failed");
> +
> +       cms_out = CMS_sign(NULL, NULL, NULL, NULL,
> +                          CMS_NOCERTS | CMS_PARTIAL | CMS_BINARY | CMS_DETACHED);
> +       ERR(!cms_out, "create cms failed");
> +
> +       flags = CMS_NOCERTS | CMS_PARTIAL | CMS_BINARY | CMS_NOSMIMECAP | CMS_DETACHED;
> +
> +       si = CMS_add1_signer(cms_out, signer, pkey, EVP_sha256(), flags);
> +       ERR(!si, "add signer failed");
> +
> +       set = MAP_SET_new();
> +       set->maps = sk_HORNET_MAP_new_null();
> +
> +       for (i = 0; i < hash_count; i++) {
> +               sha256(hashes[i].file, hash_buffer, &hash_len);

Can sha256 fail here? In that case the program should stop here and
report errors.

-Fan

> +               add_hash(set, hash_buffer, hash_len, hashes[i].index);
> +       }
> +
> +       oid = OBJ_txt2obj("2.25.316487325684022475439036912669789383960", 1);
> +       if (!oid) {
> +               ERR_print_errors_fp(stderr);
> +               DIE("create oid failed");
> +       }
> +
> +       der_len = ASN1_item_i2d((ASN1_VALUE *)set, &der, ASN1_ITEM_rptr(MAP_SET));
> +       CMS_signed_add1_attr_by_OBJ(si, oid, V_ASN1_SEQUENCE, der, der_len);
> +
> +       err = CMS_final(cms_out, data_in, NULL, CMS_NOCERTS | CMS_BINARY);
> +       ERR(!err, "cms final failed");
> +
> +       OPENSSL_free(der);
> +       MAP_SET_free(set);
> +
> +       b_out = bio_open_wr(out_path);
> +       ERR(!b_out, "opening output path failed");
> +
> +       i2d_CMS_bio_stream(b_out, cms_out, NULL, 0);
> +
> +       BIO_free(data_in);
> +       BIO_free(b_out);
> +       EVP_cleanup();
> +       ERR_free_strings();
> +       return 0;
> +}
> diff --git a/scripts/hornet/write-sig.sh b/scripts/hornet/write-sig.sh
> new file mode 100755
> index 0000000000000..7eaabe3bab9aa
> --- /dev/null
> +++ b/scripts/hornet/write-sig.sh
> @@ -0,0 +1,27 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (c) 2025 Microsoft Corporation
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of version 2 of the GNU General Public
> +# License as published by the Free Software Foundation.
> +
> +function usage() {
> +    echo "Sample for rewriting an autogenerated eBPF lskel headers"
> +    echo "with a new signature"
> +    echo ""
> +    echo "USAGE: header_file sig"
> +    exit
> +}
> +
> +ARGC=$#
> +
> +EXPECTED_ARGS=2
> +
> +if [ $ARGC -ne $EXPECTED_ARGS ] ; then
> +    usage
> +else
> +    SIG=$(xxd -p $2 | tr -d '\n' | sed 's/\(..\)/\\\\x\1/g')
> +    sed '/const char opts_sig/,/;/c\\tstatic const char opts_sig[] __attribute__((__aligned__(8))) = "\\\n'"$(printf '%s\n' "$SIG")"'\";' $1
> +fi
> --
> 2.53.0
>

^ permalink raw reply

* Re: [PATCH v5 10/10] ipe: Add BPF program load policy enforcement via Hornet integration
From: Fan Wu @ 2026-04-21  0:27 UTC (permalink / raw)
  To: Blaise Boscaccy
  Cc: Jonathan Corbet, Paul Moore, James Morris, Serge E. Hallyn,
	Mickaël Salaün, Günther Noack,
	Dr. David Alan Gilbert, Andrew Morton, James.Bottomley, dhowells,
	Fan Wu, Ryan Foster, Randy Dunlap, linux-security-module,
	linux-doc, linux-kernel, bpf, Song Liu
In-Reply-To: <20260420212653.438685-11-bboscaccy@linux.microsoft.com>

On Mon, Apr 20, 2026 at 2:27 PM Blaise Boscaccy
<bboscaccy@linux.microsoft.com> wrote:
>
> Add support for the bpf_prog_load_post_integrity LSM hook, enabling IPE
> to make policy decisions about BPF program loading based on integrity
> verdicts provided by the Hornet LSM.
>
> New policy operation:
>   op=BPF_PROG_LOAD - Matches BPF program load events
>
> New policy properties:
>   bpf_signature=NONE      - No Verdict
>   bpf_signature=OK        - Program signature and map hashes verified
>   bpf_signature=UNSIGNED  - No signature provided
>   bpf_signature=PARTIALSIG - Signature OK but no map hash data
>   bpf_signature=UNKNOWNKEY - Cert not trusted
>   bpf_signature=UNEXPECTED - An unexpected hash value was encountered
>   bpf_signature=FAULT      - System error during verification
>   bpf_signature=BADSIG    - Signature or map hash verification failed
>   bpf_keyring=BUILTIN     - Program was signed using a builtin keyring
>   bpf_keyring=SECONDARY   - Program was signed using the secondary keyring
>   bpf_keyring=PLATFORM    - Program was signed using the platform keyring
>   bpf_kernel=TRUE         - Program originated from kernelspace
>   bpf_kernel=FALSE        - Program originated from userspace
>
> These properties map directly to the lsm_integrity_verdict enum values
> provided by the Hornet LSM through security_bpf_prog_load_post_integrity.
>
> The feature is gated on CONFIG_IPE_PROP_BPF_SIGNATURE which depends on
> CONFIG_SECURITY_HORNET.
>
> Signed-off-by: Blaise Boscaccy <bboscaccy@linux.microsoft.com>
> ---

...

> diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
> index 0ae54a880405a..03541e5bb7f60 100644
> --- a/security/ipe/hooks.c
> +++ b/security/ipe/hooks.c
> @@ -340,3 +340,40 @@ int ipe_inode_setintegrity(const struct inode *inode,
>         return -EINVAL;
>  }
>  #endif /* CONFIG_IPE_PROP_FS_VERITY_BUILTIN_SIG */
> +
> +#ifdef CONFIG_IPE_PROP_BPF_SIGNATURE
> +/**
> + * ipe_bpf_prog_load_post_integrity() - ipe security hook for BPF program load.
> + * @prog: Supplies the BPF program being loaded.
> + * @attr: Supplies the bpf syscall attributes.
> + * @token: Supplies the BPF token, if any.
> + * @kernel: Whether the call originated from the kernel.
> + * @lsmid: Supplies the LSM ID of the integrity provider.
> + * @verdict: Supplies the integrity verdict from the provider (e.g. Hornet).
> + *
> + * This LSM hook is called after an integrity verification LSM (such as Hornet)
> + * has evaluated a BPF program's cryptographic signature. IPE uses the verdict
> + * to make a policy-based allow/deny decision.
> + *
> + * Return:
> + * * %0                - Success
> + * * %-EACCES  - Did not pass IPE policy
> + */
> +int ipe_bpf_prog_load_post_integrity(struct bpf_prog *prog,
> +                                    union bpf_attr *attr,
> +                                    struct bpf_token *token,
> +                                    bool kernel,
> +                                    const struct lsm_id *lsmid,
> +                                    enum lsm_integrity_verdict verdict)
> +{
> +       struct ipe_eval_ctx ctx = IPE_EVAL_CTX_INIT;
> +
> +       ctx.op = IPE_OP_BPF_PROG_LOAD;
> +       ctx.hook = IPE_HOOK_BPF_PROG_LOAD;
> +       ctx.bpf_verdict = verdict;
> +       ctx.bpf_keyring_id = attr->keyring_id;
> +       ctx.bpf_kernel = kernel;
> +
> +       return ipe_evaluate_event(&ctx);
> +}
> +#endif /* CONFIG_IPE_PROP_BPF_SIGNATURE */
> diff --git a/security/ipe/hooks.h b/security/ipe/hooks.h
> index 07db373327402..95b74f7899750 100644
> --- a/security/ipe/hooks.h
> +++ b/security/ipe/hooks.h
> @@ -10,6 +10,7 @@
>  #include <linux/security.h>
>  #include <linux/blk_types.h>
>  #include <linux/fsverity.h>
> +#include <linux/bpf.h>
>
>  enum ipe_hook_type {
>         IPE_HOOK_BPRM_CHECK = 0,
> @@ -18,6 +19,7 @@ enum ipe_hook_type {
>         IPE_HOOK_MPROTECT,
>         IPE_HOOK_KERNEL_READ,
>         IPE_HOOK_KERNEL_LOAD,
> +       IPE_HOOK_BPF_PROG_LOAD,
>         __IPE_HOOK_MAX
>  };
>
> @@ -52,4 +54,13 @@ int ipe_inode_setintegrity(const struct inode *inode, enum lsm_integrity_type ty
>                            const void *value, size_t size);
>  #endif /* CONFIG_IPE_PROP_FS_VERITY_BUILTIN_SIG */
>
> +#ifdef CONFIG_IPE_PROP_BPF_SIGNATURE
> +int ipe_bpf_prog_load_post_integrity(struct bpf_prog *prog,
> +                                    union bpf_attr *attr,
> +                                    struct bpf_token *token,
> +                                    bool kernel,
> +                                    const struct lsm_id *lsmid,
> +                                    enum lsm_integrity_verdict verdict);
> +#endif /* CONFIG_IPE_PROP_BPF_SIGNATURE */
> +
>  #endif /* _IPE_HOOKS_H */
> diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
> index 495bb765de1b8..6502d4ddc641c 100644
> --- a/security/ipe/ipe.c
> +++ b/security/ipe/ipe.c
> @@ -60,6 +60,9 @@ static struct security_hook_list ipe_hooks[] __ro_after_init = {
>  #ifdef CONFIG_IPE_PROP_FS_VERITY_BUILTIN_SIG
>         LSM_HOOK_INIT(inode_setintegrity, ipe_inode_setintegrity),
>  #endif /* CONFIG_IPE_PROP_FS_VERITY_BUILTIN_SIG */
> +#ifdef CONFIG_IPE_PROP_BPF_SIGNATURE
> +       LSM_HOOK_INIT(bpf_prog_load_post_integrity, ipe_bpf_prog_load_post_integrity),
> +#endif /* CONFIG_IPE_PROP_BPF_SIGNATURE */
>  };
>

IPE only registers bpf_prog_load_post_integrity(), which fires only
when Hornet calls security_bpf_prog_load_post_integrity. If Hornet
isn't enabled via lsm=, this hook never triggers, and an IPE policy
like "DEFAULT op=BPF_PROG_LOAD action=DENY" silently allows every
load.

I think IPE should register bpf_prog_load() as well, post_integrity()
should only store the verdict to a per-prog blob, and bpf_prog_load
reads it and evaluates policy.

-Fan

^ permalink raw reply

* Re: [PATCH v2 0/4] Firmware LSM hook
From: Paul Moore @ 2026-04-21  0:58 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, Roberto Sassu, KP Singh, Matt Bobrowski,
	Alexei Starovoitov, Daniel Borkmann, John Fastabend,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
	Yonghong Song, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
	Saeed Mahameed, Itay Avraham, Dave Jiang, Jonathan Cameron, bpf,
	linux-kernel, linux-kselftest, linux-rdma, Chiara Meiohas,
	Maher Sanalla, linux-security-module
In-Reply-To: <20260417191749.GK2577880@ziepe.ca>

On Fri, Apr 17, 2026 at 3:17 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Wed, Apr 15, 2026 at 05:40:04PM -0400, Paul Moore wrote:
> > > The NIC doesn't know anything more than the kernel to call the LSM
> > > hook. It can't magically generate the label the admin wants to use any
> > > better than the kernel can.
> >
> > The NIC presumably knows how to parse the firmware request and extract
> > whatever security relevant info is needed to pass to the kernel so the
> > driver can make an access control request.
>
> Not in practice, we'd have to agree on how to describe the "security relevant
> info" and that won't happen..

I think you're going to find that you need to describe the security
relevant info regardless of how you implement things, but we can leave
that discussion for below.

> > Leon mentioned that different firmware revisions would have different
> > parameters for a given opcode, and that one would need to inspect
> > those parameters to properly filter the command.  Is that not true, or
> > am I misreading or misunderstanding Leon's comments?
>
> They are ABI stable, so there will be rules about future changes that
> old software can follow to ignore or reject future things it doesn't
> understand.

Good, "ABI stable" means there is some hope :)  Based on the various
discussions I'm guessing both the ABI and any assigned numbers
are/will-be vendor specific?

> > > > The access control point itself represents the requested
> > > > operation.  This is possible because the number of networking
> > > > operations on a given packet is well defined and fairly limited; at a
> > > > high level the packet is either being sent from the node, received by
> > > > the node, or is passing through the node.
> > >
> > > I think we have the same split, fwctl send/recive analog is also very
> > > limited.
> >
> > Sure, but I thought the goal was to enforce access controls on the
> > firmware requests based on the opcodes/parameters contained within the
> > firmware request blob/mailbox?
>
> Yes, that's the goal. It is the same as iptables being able to
> identify that a send system call has a packet that is http or dns.

I think we still have a disconnect here.  A packet being a DNS or HTTP
packet is different from an opcode.  The opcode in the iptables isn't
"DNS" or "HTTP" it is "INPUT", "OUTPUT", or "FORWARD".

Most LSMs will want to know who is initiating the firmware request
(the subject), the requested operation/opcode (the action/verb), and
the target of the request (the object, which in this case is likely
the kernel or the device).

For most LSMs, I expect the subject to be the process making the fwctl call.

Similarly, the object will likely be either the kernel or the device itself.

As I understand things, the action/verb is going to be the opcode
within the firmware request.  If you believe I'm wrong about this
please help me understand why.

> > > LSM takes the secmark and determines if the access control point
> > > accept/rejects.
> >
> > At this point I think it would be helpful to write out the
> > subject-access-object triple for an example operation and explain how
> > an LSM could obtain each component of the access request.
>
> I think I am talking about this:
>
> app_1 FWCTL_RPC op_unpriv_t
> app_2 FWCTL_RPC op_priv_t
>
> - app_x broadly comes from the process executing the ioctl

Yep.  Were on the same page here.

> - FWCTL_RPC identifies the IOCTL userspace called to send the RPC
>   packet

Maybe.  That is an option.

> - op_X_t is the result of the classifier inspecting the RPC
>   packet. Admin tells the classifier to return op_X_t similar to
>   how --selctx does for iptables.

I've tried to explain how this doesn't match with secmark, but I'm
evidently doing a poor job.  If you want to continue with the secmark
comparisons it might be helpful to spend some time configuring secmark
on a SELinux system, and writing policy for it, to see how it works.

Beyond that, I think you will find that most LSMs - although not all -
define their security policy via an abstract subject-action-object.
The policy either allows or rejects a subject's ability to perform a
certain action on an object.  My concern with your example is that the
object isn't what is actually being acted upon, it's the requested
action.  The fwctl ioctl/API allows a user to act on a device, with
the actual action being specified by the fwctl payload.  From what I
can see, the classifier's output is the action, not the object.

> > > Same as for networking. Admin understands, admin defines, kernel is
> > > just a programmable classifier.
> >
> > Are you able to define all of the firmware request operations at this
> > point in time?  That is my largest concern at this point, and perhaps
> > the answer is a simple "yes", but I haven't seen it yet.
>
> We can identify all the IOCTL points where the RPC packet will be
> delivered to the kernel (send/recv/etc)
>
> We cannot pre-identify all the mlx_XXX_op_t's an admin might want to
> use.
>
> The same way secmark cannot pre-identify all the XXX_packet_t's.

Once again, I think there is a disconnect or misunderstanding, on a
SELinux system using secmark all of the packet types, e.g.
"XXX_packet_t's", *are* pre-defined in the SELinux policy.

-- 
paul-moore.com

^ permalink raw reply

* Re: [RFC PATCH 4/4] firmware: arm_ffa: check pkvm initailised when initailise ffa driver
From: Yeoreum Yun @ 2026-04-21  6:57 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, zohar,
	roberto.sassu, dmitry.kasatkin, eric.snowberg, peterhuewe, jarkko,
	jgg, sudeep.holla, oupton, joey.gouly, suzuki.poulose, yuzenghui,
	catalin.marinas, will
In-Reply-To: <87pl3vb5bm.wl-maz@kernel.org>

Hi Marc,

> On Sat, 18 Apr 2026 11:34:30 +0100,
> Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> >
> > > > @@ -2035,6 +2037,16 @@ static int __init ffa_init(void)
> > > >  	u32 buf_sz;
> > > >  	size_t rxtx_bufsz = SZ_4K;
> > > >
> > > > +	/*
> > > > +	 * When pKVM is enabled, the FF-A driver must be initialized
> > > > +	 * after pKVM initialization. Otherwise, pKVM cannot negotiate
> > > > +	 * the FF-A version or obtain RX/TX buffer information,
> > > > +	 * which leads to failures in FF-A calls.
> > > > +	 */
> > > > +	if (IS_ENABLED(CONFIG_KVM) && is_protected_kvm_enabled() &&
> > > > +	    !is_kvm_arm_initialised())
> > > > +		return -EPROBE_DEFER;
> > > > +
> > >
> > > That's still fundamentally wrong: pkvm is not ready until
> > > finalize_pkvm() has finished, and that's not indicated by
> > > is_kvm_arm_initialised().
> >
> > Thanks. I miss the TSC bit set in here.
>
> That's the least of the problems. None of the infrastructure is in
> place at this stage...
>
> > IMHO, I'd like to make an new state check function --
> > is_pkvm_arm_initialised() so that ff-a driver to know whether
> > pkvm is initialised.
>
> Doesn't sound great, TBH.
>
> > or any other suggestion?
>
> Instead of adding more esoteric predicates, I'd rather you build on an
> existing infrastructure. You have a dependency on KVM, use something
> that is designed to enforce dependencies. Device links spring to mind
> as something designed for that.
>
> Can you look into enabling this for KVM? If that's possible, then it
> should be easy enough to delay the actual KVM registration after pKVM
> is finalised.

Hmm, I also considered moving kvm_init() into finalised_pkvm() and
then handling the remaining ffa_init() work after /dev/kvm is registered.

However, this approach seems awkward in practice,
because misc has neither a bus nor a dedicated class notifier that we can hook into.

Also, the FF-A initialization is not driven by a device probe, but rather
happens as part of the bus registration itself,
so it does not fit well with a device_link or probe deferral based approach.

Instead, perhaps we could go with the idea I mentioned previously:
either introduce a notifier, or create a pseudo ffa_device
once pKVM initialization has completed, and
then let the ffa driver perform the additional initialization from there.

Am I missing something?

--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* Re: [RFC PATCH v1 00/11] Landlock: Namespace and capability control
From: Mickaël Salaün @ 2026-04-21  8:24 UTC (permalink / raw)
  To: Günther Noack
  Cc: Christian Brauner, Günther Noack, Paul Moore,
	Serge E . Hallyn, Justin Suess, Lennart Poettering,
	Mikhail Ivanov, Nicolas Bouchinet, Shervin Oloumi, Tingmao Wang,
	kernel-team, linux-fsdevel, linux-kernel, linux-security-module
In-Reply-To: <20260420.aaab9bf39ef8@gnoack.org>

On Mon, Apr 20, 2026 at 05:06:32PM +0200, Günther Noack wrote:
> Hello!
> 
> On Thu, Mar 12, 2026 at 11:04:33AM +0100, Mickaël Salaün wrote:
> > Namespaces are a fundamental building block for containers and
> > application sandboxes, but user namespace creation significantly widens
> > the kernel attack surface.  CVE-2022-0185 (filesystem mount parsing),
> > CVE-2022-25636 and CVE-2023-32233 (netfilter), and CVE-2022-0492 (cgroup
> > v1 release_agent) all demonstrate vulnerabilities exploitable only
> > through capabilities gained via user namespaces.  Some distributions
> > block user namespace creation entirely, but this removes a useful
> > isolation primitive.  Fine-grained control allows trusted programs to
> > use namespaces while preventing unnecessary exposure for programs that
> > do not need them.
> > 
> > Existing mechanisms (user.max_*_namespaces sysctls, userns_create LSM
> > hook, PR_SET_NO_NEW_PRIVS, and capset) each address part of this threat
> > but none provides per-process, fine-grained control over both namespace
> > types and capabilities.  Container runtimes resort to seccomp-based
> > clone/unshare filtering, but seccomp cannot dereference clone3's flag
> > structure, forcing runtimes to block clone3 entirely.
> > 
> > Landlock's composable layer model enables several patterns: a user
> > session manager can restrict namespace types and capabilities broadly
> > while allowing trusted programs to create the namespaces they need, and
> > each deeper layer can further restrict the allowed set.  Container
> > runtimes can similarly deny namespace creation inside managed
> > containers.
> 
> I assume we are talking about an unrestricted systemd user session
> manager, which would not itself be restricted?  (If the entire user
> session were running under Landlock, users couldn't change their
> passwords with "passwd" any more, because of the no_new_privs
> requirement.)

systemd can be use to create such session, as other init systems.
If no_new_privs is set, commands such as passwd would indeed not work,
but:
1. The process applying the Landlock restrictions (e.g. creating the
   user session) doesn't need to set no_new_privs if it has
   CAP_SYS_ADMIN in the current user namespace.
2. SUID programs can (and should probably) be replaced with proper
   client/server interfaces (i.e. for the client to not be privileged),
   see DBus services (e.g. Account) or homectl for instance.


> 
> 
> > This series adds two new permission categories to Landlock:
> > 
> > - LANDLOCK_PERM_NAMESPACE_ENTER: Restricts which namespace types a
> >   sandboxed process can acquire: both creation (unshare/clone) and entry
> >   (setns).  User namespace creation has no capability check in the
> >   kernel, so this is the only enforcement mechanism for that entry
> >   point.
> > 
> > - LANDLOCK_PERM_CAPABILITY_USE: Restricts which Linux capabilities a
> >   sandboxed process can use, regardless of how they were obtained
> >   (including through user namespace creation).
> 
> Given that you already went through multiple iterations here, I fully

It's the first public one, but it's well advanced.

> expect that I am overlooking something here, but based on the
> explanation, it's not clear to me why the capability control is needed
> in addition to the namespace control, to reduce the kernel attack
> surface.
> 
> In my understanding the "attack surface" problem with user namespaces
> is that they allow unprivileged processes to gain CAP_SYS_ADMIN within
> that namespace, which unlocks access to code paths which were
> traditionally reserved for the (top level) root user.

This capability and others.

> 
> But then, to prevent that from happening, it seems that restricting
> access to user namespace creation would be sufficient?

It would be sufficient to limit the kernel attack surface, but it would
make all the related features unusable.  As explained in this cover
letter, there are already several ways to block everything, but this
doesn't help for a lot of use cases and this Landlock feature proposes a
new fine-grained and unprivileged way to properly restrict some
capabilities.

> 
> (Also, in some cases, I suspect it might be possible to break
> assumptions that more privileged processes make about filesystem
> layout if the user can change the mount layout.  But that is not an
> issue with Landlock, as we forbid changes to mounts and also require
> no_new_privs.)
> 
> 
> > Both use new handled_perm and LANDLOCK_RULE_* constants following the
> > existing allow-list model.  The UAPI uses raw CAP_* and CLONE_NEW*
> > values directly; unknown values are silently accepted for forward
> > compatibility (the allow-list denies them by default).  The Landlock ABI
> > version is bumped from 8 to 9.
> 
> Compatibility question:
> 
> For both permission categories, when they are "handled" in the
> ruleset, they default to denying *all* types of namespaces, and *all*
> types of capabilities.
> 
> This is different to the handled_access_* rights, where we are
> requiring users to explicitly list all restricted rights as "handled",
> because the full list of available operations might be a moving
> target.
> 
> Why is this not a problem for capabilities and for namespaces?  Both
> the list of capabilities and the list of namespaces has been expanded
> in the past.  What happens if a new capability or namespace is
> invented?  If these are evolved, is that backwards compatible for the
> existing users of these Landlock permission categories?

This question is answered is the documentation (and the commit
messages), and that's the main difference between handled_access_* and
handled_perm.  In a nutshell, the permission rules uses non-Landlock
bits that naturally evolve without any Landlock-specific changes.

> 
> 
> > The handled_perm infrastructure is designed to be reusable by future
> > permission categories.  The last patch documents the design rationale
> > for the permission model and the criteria for choosing between
> > handled_access_*, handled_perm, and scoped.  A patch series to add
> > socket creation control is under review [2]; it could benefit from the
> > same permission model to achieve complete deny-by-default coverage of
> > socket creation.

See here ^

> > 
> > This series builds on Christian Brauner's namespace LSM blob RFC [1],
> > included as patch 1.
> > 
> > Christian, could you please review patch 3?  It adds a FOR_EACH_NS_TYPE
> > X-macro to ns_common_types.h and derives CLONE_NS_ALL, replacing inline
> > CLONE_NEW* flag enumerations in nsproxy.c and fork.c.
> > 
> > Paul, could you please review patch 2?  It adds LSM_AUDIT_DATA_NS, a new
> > audit record type that logs namespace_type and inum for
> > namespace-related LSM denials.
> > 
> > All four example vulnerabilities follow the same pattern: an
> > unprivileged user creates a user namespace to obtain capabilities, then
> > creates a second namespace to exercise them against vulnerable code.
> > LANDLOCK_PERM_NAMESPACE_ENTER prevents this by denying the user
> > namespace (eliminating the capability grant) or the specific namespace
> > type needed to exercise it.  LANDLOCK_PERM_CAPABILITY_USE independently
> > prevents it by denying the required capability.
> 
> Here, it is also not clear to me why LANDLOCK_PERM_CAPABILITY_USE is
> needed in addition to LANDLOCK_PERM_NAMESPACE_ENTER.

This is also explained in the documentation.

> 
> Looking at capabilities(7), my understanding is that capabilities can
> only be acquired through:
> 
> (1) user namespaces (prevented with LANDLOCK_PERM_NAMESPACE_ENTER)
> (2) execve (setuid or individual capabilities, prevented using
>     PR_SET_NO_NEW_PRIVS)
> 
> ...so if a process were to start out with no such capabilities,
> wouldn't that be enough to prevent it from gaining more?  Am I
> overlooking another way through which these can be acquired?
> 
> The Landlock capability support adds a "filter" for the use of
> capabilities, but my understanding of the capability system was that
> it already *is* that filter.  As long as we prevent the acquisition of
> new capabilities, shouldn't that be sufficient?

In a nutshell, capabilities applies to namespaces (and their type), so
it makes sense to be able to control them together, see the chroot
example.  Please take a look at the documentation.

> 
> 
> > Namespace restriction is enforced at two hook sites: namespace_alloc
> > (unshare/clone) and namespace_install (setns).  Together, these ensure a
> > process denied a namespace type cannot circumvent the restriction by
> > entering a pre-existing namespace via setns() on an inherited or passed
> > file descriptor.  When a domain handles both permissions, both must
> > independently allow the operation (e.g., unshare(CLONE_NEWNET) requires
> > both CAP_SYS_ADMIN to be allowed and CLONE_NEWNET to be allowed).
> > 
> > Design evolution:
> > 
> > The first approach added CAP_OPT flags to security_capable() to
> > distinguish namespace creation contexts.  This was too invasive and
> > would have required capability splitting (a dedicated CAP_NAMESPACE)
> > which does not help because the CAP_SYS_ADMIN fallback for backward
> > compatibility undermines the distinction.
> > 
> > The second stored the namespace creator's domain in the LSM blob and
> > used domain ancestry comparison in hook_capable() to bypass capability
> > checks for namespace management operations.  A SCOPE_NAMESPACE flag
> > restricted setns() by the namespace creator's domain, like SCOPE_SIGNAL.
> > Both were dropped: scopes should only concern Landlock properties
> > (domain relationships), not kernel namespace state; and the
> > cross-namespace heuristic (ns != cred->user_ns) did not accurately
> > identify namespace management operations.
> > 
> > The final design drops all of this.  The key insight is that
> > capabilities gained through user namespace creation are only exercisable
> > against namespaces of a specific type: creating a network namespace is
> > what makes CAP_NET_ADMIN exercisable.  LANDLOCK_PERM_NAMESPACE_ENTER
> > controls where capabilities are exercisable by restricting which
> > namespace types can be acquired.  LANDLOCK_PERM_CAPABILITY_USE controls
> > which capabilities are available, as a pure per-layer bitmask check with
> > no namespace awareness.  The two are independently enforced at their own
> > hook sites, with no interaction in hook_capable().  No scope flag is
> > added in this series.
> > 
> > Note that when Landlock filesystem restrictions are in use, mount
> > namespace creation has an inherent limitation: all mount topology
> > changes are denied when any filesystem right is handled, which is
> > optional.  A dedicated mount access control type is left for future work
> > [3].
> > 
> > https://lore.kernel.org/r/20260216-work-security-namespace-v1-1-075c28758e1f@kernel.org [1]
> > https://lore.kernel.org/r/20251118134639.3314803-1-ivanov.mikhail1@huawei-partners.com [2]
> > https://github.com/landlock-lsm/linux/issues/14 [3]
> > 
> > Christian Brauner (1):
> >   security: add LSM blob and hooks for namespaces
> > 
> > Mickaël Salaün (10):
> >   security: Add LSM_AUDIT_DATA_NS for namespace audit records
> >   nsproxy: Add FOR_EACH_NS_TYPE() X-macro and CLONE_NS_ALL
> >   landlock: Wrap per-layer access masks in struct layer_rights
> >   landlock: Enforce namespace entry restrictions
> >   landlock: Enforce capability restrictions
> >   selftests/landlock: Drain stale audit records on init
> >   selftests/landlock: Add namespace restriction tests
> >   selftests/landlock: Add capability restriction tests
> >   samples/landlock: Add capability and namespace restriction support
> >   landlock: Add documentation for capability and namespace restrictions
> > 
> >  Documentation/admin-guide/LSM/landlock.rst    |   19 +-
> >  Documentation/security/landlock.rst           |   80 +-
> >  Documentation/userspace-api/landlock.rst      |  156 +-
> >  include/linux/lsm_audit.h                     |    5 +
> >  include/linux/lsm_hook_defs.h                 |    3 +
> >  include/linux/lsm_hooks.h                     |    1 +
> >  include/linux/ns/ns_common_types.h            |   47 +-
> >  include/linux/security.h                      |   20 +
> >  include/uapi/linux/landlock.h                 |   89 +-
> >  kernel/fork.c                                 |    7 +-
> >  kernel/nscommon.c                             |   12 +
> >  kernel/nsproxy.c                              |   21 +-
> >  samples/landlock/sandboxer.c                  |  164 +-
> >  security/landlock/Makefile                    |    2 +
> >  security/landlock/access.h                    |   72 +-
> >  security/landlock/audit.c                     |    8 +
> >  security/landlock/audit.h                     |    2 +
> >  security/landlock/cap.c                       |  142 ++
> >  security/landlock/cap.h                       |   49 +
> >  security/landlock/cred.h                      |   47 +-
> >  security/landlock/limits.h                    |    9 +
> >  security/landlock/ns.c                        |  188 +++
> >  security/landlock/ns.h                        |   74 +
> >  security/landlock/ruleset.c                   |   23 +-
> >  security/landlock/ruleset.h                   |   53 +-
> >  security/landlock/setup.c                     |    4 +
> >  security/landlock/syscalls.c                  |  124 +-
> >  security/lsm_audit.c                          |    4 +
> >  security/lsm_init.c                           |    2 +
> >  security/security.c                           |   76 +
> >  tools/testing/selftests/landlock/audit.h      |   29 +-
> >  tools/testing/selftests/landlock/audit_test.c |    2 -
> >  tools/testing/selftests/landlock/base_test.c  |   20 +-
> >  tools/testing/selftests/landlock/cap_test.c   |  614 ++++++++
> >  tools/testing/selftests/landlock/common.h     |   23 +
> >  tools/testing/selftests/landlock/config       |    5 +
> >  tools/testing/selftests/landlock/ns_test.c    | 1379 +++++++++++++++++
> >  tools/testing/selftests/landlock/wrappers.h   |    6 +
> >  38 files changed, 3487 insertions(+), 94 deletions(-)
> >  create mode 100644 security/landlock/cap.c
> >  create mode 100644 security/landlock/cap.h
> >  create mode 100644 security/landlock/ns.c
> >  create mode 100644 security/landlock/ns.h
> >  create mode 100644 tools/testing/selftests/landlock/cap_test.c
> >  create mode 100644 tools/testing/selftests/landlock/ns_test.c
> > 
> > 
> > base-commit: 5dfb8077be2bbe2c3b9477da759e80fa9f98da42
> > -- 
> > 2.53.0
> > 
> 
> FWIW, I have also skimmed through some of the code and documentation
> and the code seemed very clean so far.
> 
> –Günther
> 

^ permalink raw reply

* Re: [RFC PATCH 4/4] firmware: arm_ffa: check pkvm initailised when initailise ffa driver
From: Sudeep Holla @ 2026-04-21  8:51 UTC (permalink / raw)
  To: Yeoreum Yun
  Cc: Marc Zyngier, linux-security-module, linux-kernel,
	linux-integrity, linux-arm-kernel, kvmarm, paul, jmorris, serge,
	zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, peterhuewe,
	jarkko, jgg, oupton, joey.gouly, suzuki.poulose, yuzenghui,
	catalin.marinas, will
In-Reply-To: <aecf57rWloQwDh6v@e129823.arm.com>

On Tue, Apr 21, 2026 at 07:57:43AM +0100, Yeoreum Yun wrote:

[...]

> 
> Also, the FF-A initialization is not driven by a device probe, but rather
> happens as part of the bus registration itself,
> so it does not fit well with a device_link or probe deferral based approach.
> 
> Instead, perhaps we could go with the idea I mentioned previously:
> either introduce a notifier, or create a pseudo ffa_device
> once pKVM initialization has completed, and
> then let the ffa driver perform the additional initialization from there.
> 
> Am I missing something?
> 

In order to handle/cleanup some ugliness in interrupt management in the
FF-A driver, we may introduce DT node eventually. But it will take sometime.

-- 
Regards,
Sudeep

^ permalink raw reply

* Re: [RFC PATCH 4/4] firmware: arm_ffa: check pkvm initailised when initailise ffa driver
From: Yeoreum Yun @ 2026-04-21  9:12 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Marc Zyngier, linux-security-module, linux-kernel,
	linux-integrity, linux-arm-kernel, kvmarm, paul, jmorris, serge,
	zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, peterhuewe,
	jarkko, jgg, oupton, joey.gouly, suzuki.poulose, yuzenghui,
	catalin.marinas, will
In-Reply-To: <20260421-married-liberal-ammonite-dffda8@sudeepholla>

> On Tue, Apr 21, 2026 at 07:57:43AM +0100, Yeoreum Yun wrote:
>
> [...]
>
> >
> > Also, the FF-A initialization is not driven by a device probe, but rather
> > happens as part of the bus registration itself,
> > so it does not fit well with a device_link or probe deferral based approach.
> >
> > Instead, perhaps we could go with the idea I mentioned previously:
> > either introduce a notifier, or create a pseudo ffa_device
> > once pKVM initialization has completed, and
> > then let the ffa driver perform the additional initialization from there.
> >
> > Am I missing something?
> >
>
> In order to handle/cleanup some ugliness in interrupt management in the
> FF-A driver, we may introduce DT node eventually. But it will take sometime.

Unfortunately, I think this DT node wouldn't be helpful to solve
this situation for dependency with the kvm misc device...

IMHO, current situation, the notifier seems to good option. unless
we make the initcall to recongise this dependency.

--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* Re: [RFC PATCH 1/4] security: ima: move ima_init into late_initcall_sync
From: Yeoreum Yun @ 2026-04-21  9:37 UTC (permalink / raw)
  To: Jonathan McDowell
  Cc: linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, zohar,
	roberto.sassu, dmitry.kasatkin, eric.snowberg, peterhuewe, jarkko,
	jgg, sudeep.holla, maz, oupton, joey.gouly, suzuki.poulose,
	yuzenghui, catalin.marinas, will
In-Reply-To: <aeYA2Jf7esHsfY4M@earth.li>

On Mon, Apr 20, 2026 at 11:32:56AM +0100, Jonathan McDowell wrote:
> On Fri, Apr 17, 2026 at 06:57:56PM +0100, Yeoreum Yun wrote:
> > To generate the boot_aggregate log in the IMA subsystem with TPM PCR values,
> > the TPM driver must be built as built-in and
> > must be probed before the IMA subsystem is initialized.
> >
> > However, when the TPM device operates over the FF-A protocol using
> > the CRB interface, probing fails and returns -EPROBE_DEFER if
> > the tpm_crb_ffa device — an FF-A device that provides the communication
> > interface to the tpm_crb driver — has not yet been probed.
> >
> > To ensure the TPM device operating over the FF-A protocol with
> > the CRB interface is probed before IMA initialization,
> > the following conditions must be met:
> >
> >   1. The corresponding ffa_device must be registered,
> >      which is done via ffa_init().
> >
> >   2. The tpm_crb_driver must successfully probe this device via
> >      tpm_crb_ffa_init().
> >
> >   3. The tpm_crb driver using CRB over FF-A can then
> >      be probed successfully. (See crb_acpi_add() and
> >      tpm_crb_ffa_init() for reference.)
> >
> > Unfortunately, ffa_init(), tpm_crb_ffa_init(), and crb_acpi_driver_init() are
> > all registered with device_initcall, which means crb_acpi_driver_init() may
> > be invoked before ffa_init() and tpm_crb_ffa_init() are completed.
> >
> > When this occurs, probing the TPM device is deferred.
> > However, the deferred probe can happen after the IMA subsystem
> > has already been initialized, since IMA initialization is performed
> > during late_initcall, and deferred_probe_initcall() is performed
> > at the same level.
> >
> > To resolve this, move ima_init() into late_inicall_sync level
> > so that let IMA not miss TPM PCR value when generating boot_aggregate
> > log though TPM device presents in the system.
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
>
> Awesome. This fixes the problems I saw with an SPI TPM on an NVIDIA GB200
> system and reported in
>
> https://lore.kernel.org/linux-integrity/aYXEepLhUouN5f99@earth.li/
>
> Reviewed-by: Jonathan McDowell <noodles@meta.com>
> Tested-by: Jonathan McDowell <noodles@meta.com>

Thanks!

>
> > ---
> > include/linux/lsm_hooks.h         |  2 ++
> > security/integrity/ima/ima_main.c |  2 +-
> > security/lsm_init.c               | 13 +++++++++++--
> > 3 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index d48bf0ad26f4..88fe105b7f00 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -166,6 +166,7 @@ enum lsm_order {
> >  * @initcall_fs: LSM callback for fs_initcall setup, optional
> >  * @initcall_device: LSM callback for device_initcall() setup, optional
> >  * @initcall_late: LSM callback for late_initcall() setup, optional
> > + * @initcall_late_sync: LSM callback for late_initcall_sync() setup, optional
> >  */
> > struct lsm_info {
> > 	const struct lsm_id *id;
> > @@ -181,6 +182,7 @@ struct lsm_info {
> > 	int (*initcall_fs)(void);
> > 	int (*initcall_device)(void);
> > 	int (*initcall_late)(void);
> > +	int (*initcall_late_sync)(void);
> > };
> >
> > #define DEFINE_LSM(lsm)							\
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 1d6229b156fb..ace280fa3212 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -1320,5 +1320,5 @@ DEFINE_LSM(ima) = {
> > 	.order = LSM_ORDER_LAST,
> > 	.blobs = &ima_blob_sizes,
> > 	/* Start IMA after the TPM is available */
> > -	.initcall_late = init_ima,
> > +	.initcall_late_sync = init_ima,
> > };
> > diff --git a/security/lsm_init.c b/security/lsm_init.c
> > index 573e2a7250c4..4e5c59beb82a 100644
> > --- a/security/lsm_init.c
> > +++ b/security/lsm_init.c
> > @@ -547,13 +547,22 @@ device_initcall(security_initcall_device);
> >  * security_initcall_late - Run the LSM late initcalls
> >  */
> > static int __init security_initcall_late(void)
> > +{
> > +	return lsm_initcall(late);
> > +}
> > +late_initcall(security_initcall_late);
> > +
> > +/**
> > + * security_initcall_late_sync - Run the LSM late initcalls sync
> > + */
> > +static int __init security_initcall_late_sync(void)
> > {
> > 	int rc;
> >
> > -	rc = lsm_initcall(late);
> > +	rc = lsm_initcall(late_sync);
> > 	lsm_pr_dbg("all enabled LSMs fully activated\n");
> > 	call_blocking_lsm_notifier(LSM_STARTED_ALL, NULL);
> >
> > 	return rc;
> > }
> > -late_initcall(security_initcall_late);
> > +late_initcall_sync(security_initcall_late_sync);
> > --
> > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
> >
> >
>
> J.
>
> --
> ] https://www.earth.li/~noodles/ []  "Do I scare you?" "No." "Do you   [
> ]  PGP/GPG Key @ the.earth.li    []   want me to?" -- Wayne's World.   [
> ] via keyserver, web or email.   []                                    [
> ] RSA: 4096/0x94FA372B2DA8B985   []                                    [

--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* Re: [RFC PATCH 1/4] security: ima: move ima_init into late_initcall_sync
From: Mimi Zohar @ 2026-04-21 12:00 UTC (permalink / raw)
  To: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm
  Cc: paul, jmorris, serge, roberto.sassu, dmitry.kasatkin,
	eric.snowberg, peterhuewe, jarkko, jgg, sudeep.holla, maz, oupton,
	joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas, will
In-Reply-To: <20260417175759.3191279-2-yeoreum.yun@arm.com>

On Fri, 2026-04-17 at 18:57 +0100, Yeoreum Yun wrote:
> To generate the boot_aggregate log in the IMA subsystem with TPM PCR values,
> the TPM driver must be built as built-in and
> must be probed before the IMA subsystem is initialized.
> 
> However, when the TPM device operates over the FF-A protocol using
> the CRB interface, probing fails and returns -EPROBE_DEFER if
> the tpm_crb_ffa device — an FF-A device that provides the communication
> interface to the tpm_crb driver — has not yet been probed.
> 
> To ensure the TPM device operating over the FF-A protocol with
> the CRB interface is probed before IMA initialization,
> the following conditions must be met:
> 
>    1. The corresponding ffa_device must be registered,
>       which is done via ffa_init().
> 
>    2. The tpm_crb_driver must successfully probe this device via
>       tpm_crb_ffa_init().
> 
>    3. The tpm_crb driver using CRB over FF-A can then
>       be probed successfully. (See crb_acpi_add() and
>       tpm_crb_ffa_init() for reference.)
> 
> Unfortunately, ffa_init(), tpm_crb_ffa_init(), and crb_acpi_driver_init() are
> all registered with device_initcall, which means crb_acpi_driver_init() may
> be invoked before ffa_init() and tpm_crb_ffa_init() are completed.
> 
> When this occurs, probing the TPM device is deferred.
> However, the deferred probe can happen after the IMA subsystem
> has already been initialized, since IMA initialization is performed
> during late_initcall, and deferred_probe_initcall() is performed
> at the same level.
> 
> To resolve this, move ima_init() into late_inicall_sync level
> so that let IMA not miss TPM PCR value when generating boot_aggregate
> log though TPM device presents in the system.
> 
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>

IMA should be initialized as early as possible. I'm really hesitant to defer
ima_init() to late_initcall_sync() for systems that the TPM is currently
initialized in time. For these systems, continue initializing IMA at
late_initcall(). As a compromise for those systems that the TPM isn't properly
initialized in time, define and instantiate the late_initcall_sync().

ima_init() would need to differentiate between the late_initcall and
late_initcall_sync.  On late_initcall(), instead of saying "No TPM chip found,
activating TPM-bypass!",  it should say "No TPM chip found, deferring to
late_initcall_sync" or something similar.

thanks,

Mimi

> ---
>  include/linux/lsm_hooks.h         |  2 ++
>  security/integrity/ima/ima_main.c |  2 +-
>  security/lsm_init.c               | 13 +++++++++++--
>  3 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index d48bf0ad26f4..88fe105b7f00 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -166,6 +166,7 @@ enum lsm_order {
>   * @initcall_fs: LSM callback for fs_initcall setup, optional
>   * @initcall_device: LSM callback for device_initcall() setup, optional
>   * @initcall_late: LSM callback for late_initcall() setup, optional
> + * @initcall_late_sync: LSM callback for late_initcall_sync() setup, optional
>   */
>  struct lsm_info {
>  	const struct lsm_id *id;
> @@ -181,6 +182,7 @@ struct lsm_info {
>  	int (*initcall_fs)(void);
>  	int (*initcall_device)(void);
>  	int (*initcall_late)(void);
> +	int (*initcall_late_sync)(void);
>  };
> 
>  #define DEFINE_LSM(lsm)							\
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 1d6229b156fb..ace280fa3212 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -1320,5 +1320,5 @@ DEFINE_LSM(ima) = {
>  	.order = LSM_ORDER_LAST,
>  	.blobs = &ima_blob_sizes,
>  	/* Start IMA after the TPM is available */
> -	.initcall_late = init_ima,
> +	.initcall_late_sync = init_ima,
>  };
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 573e2a7250c4..4e5c59beb82a 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -547,13 +547,22 @@ device_initcall(security_initcall_device);
>   * security_initcall_late - Run the LSM late initcalls
>   */
>  static int __init security_initcall_late(void)
> +{
> +	return lsm_initcall(late);
> +}
> +late_initcall(security_initcall_late);
> +
> +/**
> + * security_initcall_late_sync - Run the LSM late initcalls sync
> + */
> +static int __init security_initcall_late_sync(void)
>  {
>  	int rc;
> 
> -	rc = lsm_initcall(late);
> +	rc = lsm_initcall(late_sync);
>  	lsm_pr_dbg("all enabled LSMs fully activated\n");
>  	call_blocking_lsm_notifier(LSM_STARTED_ALL, NULL);
> 
>  	return rc;
>  }
> -late_initcall(security_initcall_late);
> +late_initcall_sync(security_initcall_late_sync);
> --
> LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
> 

^ permalink raw reply

* Re: [RFC PATCH 1/4] security: ima: move ima_init into late_initcall_sync
From: Yeoreum Yun @ 2026-04-21 12:50 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, peterhuewe, jarkko, jgg,
	sudeep.holla, maz, oupton, joey.gouly, suzuki.poulose, yuzenghui,
	catalin.marinas, will
In-Reply-To: <a6a0e15286c983d720de227c6827adbe976c5b9b.camel@linux.ibm.com>

Hi Mimi,

> On Fri, 2026-04-17 at 18:57 +0100, Yeoreum Yun wrote:
> > To generate the boot_aggregate log in the IMA subsystem with TPM PCR values,
> > the TPM driver must be built as built-in and
> > must be probed before the IMA subsystem is initialized.
> >
> > However, when the TPM device operates over the FF-A protocol using
> > the CRB interface, probing fails and returns -EPROBE_DEFER if
> > the tpm_crb_ffa device — an FF-A device that provides the communication
> > interface to the tpm_crb driver — has not yet been probed.
> >
> > To ensure the TPM device operating over the FF-A protocol with
> > the CRB interface is probed before IMA initialization,
> > the following conditions must be met:
> >
> >    1. The corresponding ffa_device must be registered,
> >       which is done via ffa_init().
> >
> >    2. The tpm_crb_driver must successfully probe this device via
> >       tpm_crb_ffa_init().
> >
> >    3. The tpm_crb driver using CRB over FF-A can then
> >       be probed successfully. (See crb_acpi_add() and
> >       tpm_crb_ffa_init() for reference.)
> >
> > Unfortunately, ffa_init(), tpm_crb_ffa_init(), and crb_acpi_driver_init() are
> > all registered with device_initcall, which means crb_acpi_driver_init() may
> > be invoked before ffa_init() and tpm_crb_ffa_init() are completed.
> >
> > When this occurs, probing the TPM device is deferred.
> > However, the deferred probe can happen after the IMA subsystem
> > has already been initialized, since IMA initialization is performed
> > during late_initcall, and deferred_probe_initcall() is performed
> > at the same level.
> >
> > To resolve this, move ima_init() into late_inicall_sync level
> > so that let IMA not miss TPM PCR value when generating boot_aggregate
> > log though TPM device presents in the system.
> >
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
>
> IMA should be initialized as early as possible. I'm really hesitant to defer
> ima_init() to late_initcall_sync() for systems that the TPM is currently
> initialized in time. For these systems, continue initializing IMA at
> late_initcall(). As a compromise for those systems that the TPM isn't properly
> initialized in time, define and instantiate the late_initcall_sync().
>
> ima_init() would need to differentiate between the late_initcall and
> late_initcall_sync.  On late_initcall(), instead of saying "No TPM chip found,
> activating TPM-bypass!",  it should say "No TPM chip found, deferring to
> late_initcall_sync" or something similar.

But can we really move those initialisations to be called again?

I am referring to functions such as ima_init_crypto(),
ima_add_boot_aggregate(), and ima_measure_critical_data() in ima_init()—
first without TPM, and then a second time once TPM becomes available.
I don’t think that approach would work.

In other words, unless tpm_default_chip() can differentiate between a TPM
device that is deferred and one that does not exist, we cannot distinguish
between the “defer” case and “-EEXIST”.

It might be possible if the TPM core tracked the state when a driver returns
-EPROBE_DEFER, but I am not sure that is the right approach.
For deferred probe cases, the “device initialised in time” check should
likely be done at late_initcall_sync, rather than late_initcall.

This implies that any such check performed before late_initcall_sync
does not reflect a valid state, as it cannot distinguish between “not
present” and “deferred”.

Therefore, I think the TPM check in IMA should be performed at
late_initcall_sync.


Am I missing something?

Thanks.

>
> > ---
> >  include/linux/lsm_hooks.h         |  2 ++
> >  security/integrity/ima/ima_main.c |  2 +-
> >  security/lsm_init.c               | 13 +++++++++++--
> >  3 files changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> > index d48bf0ad26f4..88fe105b7f00 100644
> > --- a/include/linux/lsm_hooks.h
> > +++ b/include/linux/lsm_hooks.h
> > @@ -166,6 +166,7 @@ enum lsm_order {
> >   * @initcall_fs: LSM callback for fs_initcall setup, optional
> >   * @initcall_device: LSM callback for device_initcall() setup, optional
> >   * @initcall_late: LSM callback for late_initcall() setup, optional
> > + * @initcall_late_sync: LSM callback for late_initcall_sync() setup, optional
> >   */
> >  struct lsm_info {
> >  	const struct lsm_id *id;
> > @@ -181,6 +182,7 @@ struct lsm_info {
> >  	int (*initcall_fs)(void);
> >  	int (*initcall_device)(void);
> >  	int (*initcall_late)(void);
> > +	int (*initcall_late_sync)(void);
> >  };
> >
> >  #define DEFINE_LSM(lsm)							\
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 1d6229b156fb..ace280fa3212 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -1320,5 +1320,5 @@ DEFINE_LSM(ima) = {
> >  	.order = LSM_ORDER_LAST,
> >  	.blobs = &ima_blob_sizes,
> >  	/* Start IMA after the TPM is available */
> > -	.initcall_late = init_ima,
> > +	.initcall_late_sync = init_ima,
> >  };
> > diff --git a/security/lsm_init.c b/security/lsm_init.c
> > index 573e2a7250c4..4e5c59beb82a 100644
> > --- a/security/lsm_init.c
> > +++ b/security/lsm_init.c
> > @@ -547,13 +547,22 @@ device_initcall(security_initcall_device);
> >   * security_initcall_late - Run the LSM late initcalls
> >   */
> >  static int __init security_initcall_late(void)
> > +{
> > +	return lsm_initcall(late);
> > +}
> > +late_initcall(security_initcall_late);
> > +
> > +/**
> > + * security_initcall_late_sync - Run the LSM late initcalls sync
> > + */
> > +static int __init security_initcall_late_sync(void)
> >  {
> >  	int rc;
> >
> > -	rc = lsm_initcall(late);
> > +	rc = lsm_initcall(late_sync);
> >  	lsm_pr_dbg("all enabled LSMs fully activated\n");
> >  	call_blocking_lsm_notifier(LSM_STARTED_ALL, NULL);
> >
> >  	return rc;
> >  }
> > -late_initcall(security_initcall_late);
> > +late_initcall_sync(security_initcall_late_sync);
> > --
> > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
> >

--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* Re: [RFC PATCH 1/4] security: ima: move ima_init into late_initcall_sync
From: Mimi Zohar @ 2026-04-21 13:26 UTC (permalink / raw)
  To: Yeoreum Yun
  Cc: linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, peterhuewe, jarkko, jgg,
	sudeep.holla, maz, oupton, joey.gouly, suzuki.poulose, yuzenghui,
	catalin.marinas, will
In-Reply-To: <aedyh61iTnQRyzMv@e129823.arm.com>

On Tue, 2026-04-21 at 13:50 +0100, Yeoreum Yun wrote:
> Hi Mimi,
> 
> > On Fri, 2026-04-17 at 18:57 +0100, Yeoreum Yun wrote:
> > > To generate the boot_aggregate log in the IMA subsystem with TPM PCR values,
> > > the TPM driver must be built as built-in and
> > > must be probed before the IMA subsystem is initialized.
> > > 
> > > However, when the TPM device operates over the FF-A protocol using
> > > the CRB interface, probing fails and returns -EPROBE_DEFER if
> > > the tpm_crb_ffa device — an FF-A device that provides the communication
> > > interface to the tpm_crb driver — has not yet been probed.
> > > 
> > > To ensure the TPM device operating over the FF-A protocol with
> > > the CRB interface is probed before IMA initialization,
> > > the following conditions must be met:
> > > 
> > >    1. The corresponding ffa_device must be registered,
> > >       which is done via ffa_init().
> > > 
> > >    2. The tpm_crb_driver must successfully probe this device via
> > >       tpm_crb_ffa_init().
> > > 
> > >    3. The tpm_crb driver using CRB over FF-A can then
> > >       be probed successfully. (See crb_acpi_add() and
> > >       tpm_crb_ffa_init() for reference.)
> > > 
> > > Unfortunately, ffa_init(), tpm_crb_ffa_init(), and crb_acpi_driver_init() are
> > > all registered with device_initcall, which means crb_acpi_driver_init() may
> > > be invoked before ffa_init() and tpm_crb_ffa_init() are completed.
> > > 
> > > When this occurs, probing the TPM device is deferred.
> > > However, the deferred probe can happen after the IMA subsystem
> > > has already been initialized, since IMA initialization is performed
> > > during late_initcall, and deferred_probe_initcall() is performed
> > > at the same level.
> > > 
> > > To resolve this, move ima_init() into late_inicall_sync level
> > > so that let IMA not miss TPM PCR value when generating boot_aggregate
> > > log though TPM device presents in the system.
> > > 
> > > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > 
> > IMA should be initialized as early as possible. I'm really hesitant to defer
> > ima_init() to late_initcall_sync() for systems that the TPM is currently
> > initialized in time. For these systems, continue initializing IMA at
> > late_initcall(). As a compromise for those systems that the TPM isn't properly
> > initialized in time, define and instantiate the late_initcall_sync().
> > 
> > ima_init() would need to differentiate between the late_initcall and
> > late_initcall_sync.  On late_initcall(), instead of saying "No TPM chip found,
> > activating TPM-bypass!",  it should say "No TPM chip found, deferring to
> > late_initcall_sync" or something similar.
> 
> But can we really move those initialisations to be called again?
> 
> I am referring to functions such as ima_init_crypto(),
> ima_add_boot_aggregate(), and ima_measure_critical_data() in ima_init()—
> first without TPM, and then a second time once TPM becomes available.
> I don’t think that approach would work.
> 
> In other words, unless tpm_default_chip() can differentiate between a TPM
> device that is deferred and one that does not exist, we cannot distinguish
> between the “defer” case and “-EEXIST”.
> 
> It might be possible if the TPM core tracked the state when a driver returns
> -EPROBE_DEFER, but I am not sure that is the right approach.
> For deferred probe cases, the “device initialised in time” check should
> likely be done at late_initcall_sync, rather than late_initcall.
> 
> This implies that any such check performed before late_initcall_sync
> does not reflect a valid state, as it cannot distinguish between “not
> present” and “deferred”.
> 
> Therefore, I think the TPM check in IMA should be performed at
> late_initcall_sync.
> 
> 
> Am I missing something?

In ima_init() you short circuit out, when called by late_initcall(), if the TPM
hasn't been initialized.  So the rest of the ima_init() isn't called.  Roughly
something like this (needs some cleanup):

int __init ima_init(void)
{
        static int first = 1;
        int rc;

        if (ima_tpm_chip)
                return 0;

        ima_tpm_chip = tpm_default_chip();
        if (!ima_tpm_chip && first) {
                pr_info("No TPM chip found, deferring te late_initcall_sync()\n");
                first = 0;
                return 0;
        }

The rest of the code remains the same.

Mimi

^ permalink raw reply

* Re: [RFC PATCH 1/4] security: ima: move ima_init into late_initcall_sync
From: Yeoreum Yun @ 2026-04-21 14:09 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
	dmitry.kasatkin, eric.snowberg, peterhuewe, jarkko, jgg,
	sudeep.holla, maz, oupton, joey.gouly, suzuki.poulose, yuzenghui,
	catalin.marinas, will
In-Reply-To: <d6bcc9ef98a1e86887c5a79ff2822e70b5534343.camel@linux.ibm.com>

Hi Mimi,

> On Tue, 2026-04-21 at 13:50 +0100, Yeoreum Yun wrote:
> > Hi Mimi,
> >
> > > On Fri, 2026-04-17 at 18:57 +0100, Yeoreum Yun wrote:
> > > > To generate the boot_aggregate log in the IMA subsystem with TPM PCR values,
> > > > the TPM driver must be built as built-in and
> > > > must be probed before the IMA subsystem is initialized.
> > > >
> > > > However, when the TPM device operates over the FF-A protocol using
> > > > the CRB interface, probing fails and returns -EPROBE_DEFER if
> > > > the tpm_crb_ffa device — an FF-A device that provides the communication
> > > > interface to the tpm_crb driver — has not yet been probed.
> > > >
> > > > To ensure the TPM device operating over the FF-A protocol with
> > > > the CRB interface is probed before IMA initialization,
> > > > the following conditions must be met:
> > > >
> > > >    1. The corresponding ffa_device must be registered,
> > > >       which is done via ffa_init().
> > > >
> > > >    2. The tpm_crb_driver must successfully probe this device via
> > > >       tpm_crb_ffa_init().
> > > >
> > > >    3. The tpm_crb driver using CRB over FF-A can then
> > > >       be probed successfully. (See crb_acpi_add() and
> > > >       tpm_crb_ffa_init() for reference.)
> > > >
> > > > Unfortunately, ffa_init(), tpm_crb_ffa_init(), and crb_acpi_driver_init() are
> > > > all registered with device_initcall, which means crb_acpi_driver_init() may
> > > > be invoked before ffa_init() and tpm_crb_ffa_init() are completed.
> > > >
> > > > When this occurs, probing the TPM device is deferred.
> > > > However, the deferred probe can happen after the IMA subsystem
> > > > has already been initialized, since IMA initialization is performed
> > > > during late_initcall, and deferred_probe_initcall() is performed
> > > > at the same level.
> > > >
> > > > To resolve this, move ima_init() into late_inicall_sync level
> > > > so that let IMA not miss TPM PCR value when generating boot_aggregate
> > > > log though TPM device presents in the system.
> > > >
> > > > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > >
> > > IMA should be initialized as early as possible. I'm really hesitant to defer
> > > ima_init() to late_initcall_sync() for systems that the TPM is currently
> > > initialized in time. For these systems, continue initializing IMA at
> > > late_initcall(). As a compromise for those systems that the TPM isn't properly
> > > initialized in time, define and instantiate the late_initcall_sync().
> > >
> > > ima_init() would need to differentiate between the late_initcall and
> > > late_initcall_sync.  On late_initcall(), instead of saying "No TPM chip found,
> > > activating TPM-bypass!",  it should say "No TPM chip found, deferring to
> > > late_initcall_sync" or something similar.
> >
> > But can we really move those initialisations to be called again?
> >
> > I am referring to functions such as ima_init_crypto(),
> > ima_add_boot_aggregate(), and ima_measure_critical_data() in ima_init()—
> > first without TPM, and then a second time once TPM becomes available.
> > I don’t think that approach would work.
> >
> > In other words, unless tpm_default_chip() can differentiate between a TPM
> > device that is deferred and one that does not exist, we cannot distinguish
> > between the “defer” case and “-EEXIST”.
> >
> > It might be possible if the TPM core tracked the state when a driver returns
> > -EPROBE_DEFER, but I am not sure that is the right approach.
> > For deferred probe cases, the “device initialised in time” check should
> > likely be done at late_initcall_sync, rather than late_initcall.
> >
> > This implies that any such check performed before late_initcall_sync
> > does not reflect a valid state, as it cannot distinguish between “not
> > present” and “deferred”.
> >
> > Therefore, I think the TPM check in IMA should be performed at
> > late_initcall_sync.
> >
> >
> > Am I missing something?
>
> In ima_init() you short circuit out, when called by late_initcall(), if the TPM
> hasn't been initialized.  So the rest of the ima_init() isn't called.  Roughly
> something like this (needs some cleanup):
>
> int __init ima_init(void)
> {
>         static int first = 1;
>         int rc;
>
>         if (ima_tpm_chip)
>                 return 0;
>
>         ima_tpm_chip = tpm_default_chip();
>         if (!ima_tpm_chip && first) {
>                 pr_info("No TPM chip found, deferring te late_initcall_sync()\n");
>                 first = 0;
>                 return 0;
>         }

I see. then I'll respin in v2.

Thanks!

--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* Re: [PATCH AUTOSEL 7.0-6.18] ima: Define and use a digest_size field in the ima_algo_desc structure
From: Mimi Zohar @ 2026-04-21 18:49 UTC (permalink / raw)
  To: Sasha Levin, patches, stable
  Cc: Roberto Sassu, dmitry.kasatkin, paul, jmorris, serge,
	linux-integrity, linux-security-module, linux-kernel
In-Reply-To: <20260420132314.1023554-102-sashal@kernel.org>

On Mon, 2026-04-20 at 09:18 -0400, Sasha Levin wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> [ Upstream commit a74d7197ebe5b1b8028911d47e78c119d9aaf193 ]
> 
> Add the digest_size field to the ima_algo_desc structure to determine the
> digest size from the correct source.
> 
> If the hash algorithm is among allocated PCR banks, take the value from the
> TPM bank info (equal to the value from the crypto subsystem if the TPM
> algorithm is supported by it; otherwise, not exceding the size of the
> digest buffer in the tpm_digest structure, used by IMA).
> 
> If the hash algorithm is SHA1, use the predefined value. Lastly, if the
> hash algorithm is the default one but not among the PCR banks, take the
> digest size from the crypto subsystem (the default hash algorithm is
> checked when parsing the ima_hash= command line option).
> 
> Finally, use the new information to correctly show the template digest in
> ima_measurements_show() and ima_ascii_measurements_show().
> 
> Link: https://github.com/linux-integrity/linux/issues/14
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> 
> LLM Generated explanations, may be completely bogus:
> 
> Now I have complete information. Let me provide the final analysis.
> 
> ## Phase 1: Commit Message Forensics
> 
> ### Step 1.1: Subject Line
> **Record:** Subsystem: `ima:` (Integrity Measurement Architecture, a
> security subsystem). Action verb: "Define and use" - sounds like
> enhancement, but functionally equivalent to "fix digest size source".
> One-line summary: Add a digest_size field to ima_algo_desc for accurate
> template digest output.
> 
> ### Step 1.2: Tags
> **Record:**
> - `Link: https://github.com/linux-integrity/linux/issues/14` (bug
>   tracker for the linux-integrity subsystem)
> - `Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>` (author)
> - `Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>` (IMA maintainer -
>   applied the patch)
> - No `Fixes:`, no `Cc: stable`, no `Reported-by:` tag.
> 
> ### Step 1.3: Commit Body Analysis
> **Record:** The message describes three cases for digest size:
> 1. TPM bank-allocated algos: take from TPM bank info (which may differ
>    from crypto subsystem size)
> 2. SHA1: use predefined value
> 3. Default hash algo not among banks: use crypto subsystem's size
> 
> Author's framing is additive/improvement ("Add the ... field"), but the
> Link points to GitHub issue #14 titled "Out of bound when creating per-
> algo measurement list interfaces" - describing a KASAN out-of-bounds
> read when TPM has unsupported algorithms (e.g., SHA3_256).
> 
> ### Step 1.4: Hidden Bug Fix Detection
> **Record:** This IS a hidden bug fix. The old code used
> `hash_digest_size[algo]` where `algo` can be `HASH_ALGO__LAST` (for
> unsupported TPM algos). Since `hash_digest_size` is declared
> `[HASH_ALGO__LAST]`, that access is out-of-bounds. The new code uses the
> TPM bank's `digest_size` (always valid) or a known constant.
> 
> ## Phase 2: Diff Analysis
> 
> ### Step 2.1: Inventory
> **Record:** 3 files changed:
> - `security/integrity/ima/ima.h` (+1)
> - `security/integrity/ima/ima_crypto.c` (+6)
> - `security/integrity/ima/ima_fs.c` (+6/-12)
> 
> Total: 13 insertions, 12 deletions. Scope: single-subsystem surgical
> change.
> 
> ### Step 2.2-2.3: Code Flow and Bug Mechanism
> **Record:** Bug category: **Out-of-bounds read** (KASAN-detectable).
> 
> Before fix: `ima_putc(m, e->digests[algo_idx].digest,
> hash_digest_size[algo])` where `algo = ima_algo_array[algo_idx].algo`.
> If the TPM has an algorithm not supported by the kernel's crypto
> subsystem (e.g., SHA3_256 which was not yet in `tpm2_hash_map`), `algo
> == HASH_ALGO__LAST`, and `hash_digest_size[HASH_ALGO__LAST]` is an OOB
> read of the `[HASH_ALGO__LAST]`-sized array.
> 
> After fix: `ima_putc(m, e->digests[algo_idx].digest,
> ima_algo_array[algo_idx].digest_size)`. `digest_size` is populated from
> `tpm_bank_info.digest_size` (which is filled via `tpm2_pcr_read` for
> unknown algos, or `hash_digest_size[crypto_algo]` for known ones),
> `SHA1_DIGEST_SIZE`, or `hash_digest_size[ima_hash_algo]` - all safe
> indexes.
> 
> ### Step 2.4: Fix Quality
> **Record:** Fix is obviously correct, minimal, and well-contained. The
> new `digest_size` field is populated once during init (`__init`), then
> only read later. Regression risk is low - the change is semantically
> equivalent to the old code when the TPM algo is supported, and correct
> when it isn't.
> 
> ## Phase 3: Git History
> 
> ### Step 3.1-3.2: Blame and Fixes target
> **Record:** The buggy line `ima_putc(m, e->digests[algo_idx].digest,
> hash_digest_size[algo])` was introduced by commit `9fa8e76250082a`
> ("ima: add crypto agility support for template-hash algorithm", by
> Enrico Bravi, merged in v6.10). This code is present in every stable
> tree from v6.10 onwards (so 6.12.y and newer).
> 
> ### Step 3.3: Related Commits
> **Record:** Companion commit `d7bd8cf0b348d` ("ima_fs: Correctly create
> securityfs files for unsupported hash algos") was applied 12 days after
> this one, sharing the same `Link:` to issue #14. That commit has an
> explicit `Fixes: 9fa8e7625008` tag and includes a KASAN dump showing
> `create_securityfs_measurement_lists+0x396/0x440` OOB in
> `hash_algo_name`. The two commits address two sides of the same bug:
> `a74d7197ebe5b` fixes OOB in `hash_digest_size[algo]` (runtime, at file
> read), `d7bd8cf0b348d` fixes OOB in `hash_algo_name[algo]` (boot, at
> file creation).
> 
> ### Step 3.4: Author Context
> **Record:** Roberto Sassu is a long-term IMA contributor. Mimi Zohar is
> the IMA subsystem maintainer who merged the patch.
> 
> ### Step 3.5: Dependencies
> **Record:** The fix depends on `tpm_bank_info.digest_size` being
> available, which has existed since commit `879b589210a9a` (2019). No new
> dependencies. Applies to any stable tree containing `9fa8e76250082a`
> (v6.10+).
> 
> ## Phase 4: Mailing List Research
> 
> ### Step 4.1-4.4: Patch Discussion
> **Record:**
> - `b4 dig -c a74d7197ebe5b` found single v1 submission at `https://lore.
>   kernel.org/all/20260225125301.87996-1-roberto.sassu@huaweicloud.com/`
> - Discussion thread contains 3 messages from Mimi Zohar (maintainer) and
>   Roberto Sassu. Mimi requested title rename and asked for a note about
>   the design change (from crypto subsystem's digest size to TPM's).
> - No explicit stable nomination, no mention of KASAN in discussion
>   thread itself.
> - GitHub issue #14 (referenced via Link: tag) explicitly documents the
>   OOB bug this is fixing: "If a TPM algorithm is not supported the PCR
>   bank info is initialized with HASH_ALGO__LAST, which passed to
>   hash_algo_name[] causes an out of bound."
> - No v2, applied as single revision.
> 
> ### Step 4.5: Stable Discussion
> **Record:** No prior stable mailing list discussion found for this
> specific commit.
> 
> ## Phase 5: Code Semantic Analysis
> 
> ### Step 5.1-5.4: Call Paths
> **Record:** `ima_measurements_show()` is called when a userspace process
> reads `/sys/kernel/security/ima/binary_runtime_measurements*`.
> `ima_ascii_measurements_show()` similarly for ASCII files. These files
> are readable by root. The path is reachable from userspace via a simple
> `read()` syscall against the securityfs files. `ima_init_crypto()` is
> called once at boot via initcall.
> 
> ### Step 5.5: Similar Patterns
> **Record:** The sister commit `d7bd8cf0b348d` addresses the same pattern
> (`hash_algo_name[algo]` with `algo == HASH_ALGO__LAST`) in the file-
> creation path.
> 
> ## Phase 6: Stable Tree Cross-Reference
> 
> ### Step 6.1-6.3: Applicability
> **Record:**
> - Buggy code exists in 6.12.y (verified via `git blame stable-
>   push/linux-6.12.y` showing line 184 originated from 9fa8e76250082a).
>   Also in 6.15, 6.17, 6.18, 6.19, 7.0.
> - 6.1.y and 6.6.y don't have the crypto agility code
>   (`hash_digest_size[algo]` usage) - the fix is NOT applicable/needed
>   there. 6.6.y uses `TPM_DIGEST_SIZE`.
> - Backport difficulty to 6.12.y: minor rework needed (ima_algo_array
>   allocation uses `kcalloc` instead of `kzalloc_objs` in newer tree, but
>   that's not affected by this patch - the field addition and assignments
>   apply straightforwardly).
> - Neither this commit nor `d7bd8cf0b348d` is yet in 6.12.y (verified via
>   `git log stable-push/linux-6.12.y`).
> 
> ## Phase 7: Subsystem Context
> 
> ### Step 7.1-7.2
> **Record:** Subsystem: IMA (security/integrity/ima/). Criticality:
> IMPORTANT - used for measured boot/attestation on enterprise/embedded
> systems. Activity: active subsystem with regular fixes. The code is only
> reachable when CONFIG_IMA is enabled AND a TPM is present, further
> narrowing impact to TPM-equipped systems.
> 
> ## Phase 8: Impact and Risk
> 
> ### Step 8.1: Affected Users
> **Record:** Users with IMA enabled + TPM 2.0 chip that exposes an
> algorithm not in the kernel's `tpm2_hash_map`. The KASAN dump in
> d7bd8cf0b348d shows this was hit on real hardware (SHA3_256-capable
> TPM).
> 
> ### Step 8.2: Trigger
> **Record:** The secondary OOB fixed by THIS commit
> (hash_digest_size[HASH_ALGO__LAST]) triggers when:
> 1. A TPM exposes an unsupported algorithm (e.g., SHA3_256)
> 2. A user (root) reads the unsupported-algo measurements file
> Root privilege required - not a remote attack vector, but reproducible
> with specific hardware. The primary OOB (in create_securityfs) hits
> every boot with such TPMs, which is what the KASAN report showed.
> 
> ### Step 8.3: Failure Mode
> **Record:** Out-of-bounds read from kernel memory. Under KASAN: reported
> as BUG. Without KASAN: may return garbage digest size, which could cause
> excessive data to be read from `e->digests[algo_idx].digest` (a fixed-
> size `[TPM2_MAX_DIGEST_SIZE]` buffer) or leak a few bytes past the
> `hash_digest_size` array. Severity: **MEDIUM-HIGH** (OOB read is KASAN-
> reportable security-relevant behavior, not a guaranteed crash without
> KASAN but can leak info or cause incorrect behavior).
> 
> ### Step 8.4: Risk vs Benefit
> **Record:**
> - **Benefit: MEDIUM** - Fixes one half of a KASAN-reportable OOB read
>   with real-hardware reproducer.
> - **Risk: LOW** - 13-line structural change, all within the IMA init
>   path + two show functions, no change of external behavior for
>   supported TPM algos.
> - **Ratio: Favorable for backport** - but only valuable when paired with
>   d7bd8cf0b348d (the boot-time crash fix).
> 
> ## Phase 9: Final Synthesis
> 
> ### Step 9.1-9.4: Evidence Summary
> 
> **For backport:**
> - Small, contained (13/12 lines, 3 files)
> - Fixes real OOB read (hash_digest_size[HASH_ALGO__LAST])
> - Obviously correct - reviewed by IMA maintainer (Mimi Zohar)
> - Low regression risk
> - Reachable from userspace (root reads securityfs file)
> - Bug has real-hardware reproducer (SHA3_256 TPMs)
> - Companion commit d7bd8cf0b348d has `Fixes:` tag and will be auto-
>   selected; backporting only d7bd8cf0b348d leaves a latent OOB in the
>   read path
> 
> **Against backport:**
> - No explicit `Fixes:` tag, no `Cc: stable`
> - Framed as enhancement, not bug fix
> - Alone doesn't fix the primary crash (boot-time OOB in
>   `create_securityfs_measurement_lists`) - that's d7bd8cf0b348d
> - Design change (TPM's size vs crypto's size) noted by maintainer in
>   review
> 
> **Exception Category:** Not a device ID/quirk/DT/build/doc. Standard bug
> fix evaluation.
> 
> **Stable Rules Check:**
> 1. Obviously correct: YES (reviewed, simple struct field addition + safe
>    sources)
> 2. Fixes real bug: YES (OOB read)
> 3. Important issue: MEDIUM (KASAN-reportable OOB with real hardware)
> 4. Small and contained: YES (13/12 lines)
> 5. No new features/APIs: YES (internal struct field, not user-visible)
> 6. Applies to stable: YES (6.12.y+ with minor/no conflicts)
> 
> ## Verification
> 
> - [Phase 1] Parsed tags: no Fixes:, no Cc: stable, has `Link:
>   github.com/linux-integrity/linux/issues/14`
> - [Phase 1] Hidden bug fix confirmed: commit uses additive language but
>   eliminates OOB read
> - [Phase 2] Diff analysis: `git show a74d7197ebe5b` confirmed +13/-12
>   across 3 files; key change is replacing `hash_digest_size[algo]` with
>   `ima_algo_array[algo_idx].digest_size`
> - [Phase 3] `git blame stable-push/linux-6.12.y
>   security/integrity/ima/ima_fs.c`: buggy line 184 was introduced by
>   9fa8e76250082a (v6.10)
> - [Phase 3] `git show 9fa8e76250082`: confirmed merged in v6.10
>   (2024-04-12)
> - [Phase 3] Found companion commit d7bd8cf0b348d sharing the same GitHub
>   issue link
> - [Phase 3] Verified `include/linux/tpm.h` defines
>   `tpm_bank_info.digest_size` (dependency satisfied in all target stable
>   trees since 2019 via commit 879b589210a9a)
> - [Phase 4] `b4 dig -c a74d7197ebe5b`: found single v1 submission
> - [Phase 4] `b4 dig -c a74d7197ebe5b -a`: only v1, no earlier revisions
> - [Phase 4] `b4 dig -c a74d7197ebe5b -w`: maintainer Mimi Zohar was a
>   recipient
> - [Phase 4] Read `/tmp/thread.mbx`: confirmed maintainer review, no
>   stable nomination, no KASAN mention in thread
> - [Phase 4] WebFetch github issue #14: confirmed OOB bug rationale
> - [Phase 4] WebFetch marc.info original patch: confirmed patch content
>   matches commit
> - [Phase 5] Verified `ima_measurements_show` is reachable via read() on
>   `/sys/kernel/security/ima/*_runtime_measurements*`
> - [Phase 6] `git show stable-push/linux-6.12.y:.../ima_fs.c`: confirmed
>   buggy line present at 184
> - [Phase 6] `git show stable-push/linux-6.6.y:.../ima_fs.c`: uses
>   `TPM_DIGEST_SIZE`, no crypto agility, bug doesn't exist there
> - [Phase 6] `git show stable-push/linux-6.1.y`: ima_fs.c does not have
>   the buggy code
> - [Phase 6] `git log stable-push/linux-6.12.y`: neither this commit nor
>   d7bd8cf0b348d is yet in 6.12.y
> - [Phase 8] Failure mode: OOB read of
>   `hash_digest_size[HASH_ALGO__LAST]` - triggered by reading per-algo
>   measurement file on TPM with unsupported algo
> - UNVERIFIED: Whether d7bd8cf0b348d has been auto-selected by Fixes:-tag
>   tooling; whether user-visible symptoms (without KASAN) would be
>   observable in the field.
> 
> ## Conclusion
> 
> This commit is a small, surgical fix for a real out-of-bounds read bug
> that affects stable trees from v6.12 onwards. While the commit message
> frames it as an "add a field" improvement rather than a bug fix, the
> code change explicitly eliminates an OOB read in the user-reachable path
> `ima_measurements_show()` / `ima_ascii_measurements_show()` when a TPM
> exposes an algorithm not supported by the kernel crypto subsystem. The
> companion commit `d7bd8cf0b348d` addresses the parallel boot-time OOB
> (in `create_securityfs_measurement_lists`) and has a `Fixes:` tag, so it
> will likely be auto-selected. If d7bd8cf0b348d reaches stable (as it
> should), this commit is needed to plug the remaining runtime OOB on the
> same hardware.
> 
> **YES**

Thanks, Sasha!

The AI's conclusion is correct.  This patch is needed when backporting commit
d7bd8cf0b348 ("ima_fs: Correctly create securityfs files for unsupported hash
algos") is correct.

Mimi

^ permalink raw reply

* Re: [PATCH RFC bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
From: Frederick Lawler @ 2026-04-21 21:07 UTC (permalink / raw)
  To: Paul Moore, James Morris, Serge E. Hallyn, Eric Paris,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, Mickaël Salaün, Günther Noack
  Cc: linux-kernel, linux-security-module, audit, bpf, linux-kselftest,
	kernel-team
In-Reply-To: <20260311-bpf-auditd-send-message-v1-0-10a62db5c92f@cloudflare.com>

Hi folks,

I was accepted to speak a little bit about this patch series at Linux
Security Summit this May [1]. I'm going to use this opportunity to
re-iterate some of the motivation, what can be done today with BPF,
drawbacks, and wrap up with discussion topics. I'd love to hear feedback
from audit, BPF, and security folks to work towards a viable solution that
addresses shortcomings to allow for better integration with BPF.

Best,
Fred

[1]: https://lssna2026.sched.com/event/2KEc3/bridging-bpf-lsm-and-the-linux-audit-subsystem-frederick-lawler-cloudflare?iframe=yes&w=100%&sidebar=yes&bg=no

^ permalink raw reply

* Re: [PATCH RFC bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
From: Alexei Starovoitov @ 2026-04-21 21:12 UTC (permalink / raw)
  To: Frederick Lawler
  Cc: Paul Moore, James Morris, Serge E. Hallyn, Eric Paris,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, Mickaël Salaün, Günther Noack, LKML,
	LSM List, audit, bpf, open list:KERNEL SELFTEST FRAMEWORK,
	kernel-team
In-Reply-To: <aefnIw1Tx_2r5nkS@CMGLRV3>

On Tue, Apr 21, 2026 at 2:07 PM Frederick Lawler <fred@cloudflare.com> wrote:
>
> Hi folks,
>
> I was accepted to speak a little bit about this patch series at Linux
> Security Summit this May [1]. I'm going to use this opportunity to
> re-iterate some of the motivation, what can be done today with BPF,
> drawbacks, and wrap up with discussion topics. I'd love to hear feedback
> from audit, BPF, and security folks to work towards a viable solution that
> addresses shortcomings to allow for better integration with BPF.

I don't think any bpf folks will be there.
Also giving a talk about it doesn't make it acceptable.
It's still Nack.

^ permalink raw reply

* Re: [PATCH RFC bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
From: Paul Moore @ 2026-04-21 22:10 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Frederick Lawler, James Morris, Serge E. Hallyn, Eric Paris,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, Mickaël Salaün, Günther Noack, LKML,
	LSM List, audit, bpf, open list:KERNEL SELFTEST FRAMEWORK,
	kernel-team
In-Reply-To: <CAADnVQJQxa+fHM9X8COc60ZMcxF0DMhWbtArg_gPynpbbg55TQ@mail.gmail.com>

On Tue, Apr 21, 2026 at 5:12 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Tue, Apr 21, 2026 at 2:07 PM Frederick Lawler <fred@cloudflare.com> wrote:
> >
> > Hi folks,
> >
> > I was accepted to speak a little bit about this patch series at Linux
> > Security Summit this May [1]. I'm going to use this opportunity to
> > re-iterate some of the motivation, what can be done today with BPF,
> > drawbacks, and wrap up with discussion topics. I'd love to hear feedback
> > from audit, BPF, and security folks to work towards a viable solution that
> > addresses shortcomings to allow for better integration with BPF.
>
> I don't think any bpf folks will be there.
> Also giving a talk about it doesn't make it acceptable.

I think it's valid to have a discussion with the LSS folks as they are
the ones who will most likely be interested in using the
functionality.  From my perspective there is still value in validating
the basic ideas in Fred's patchset and checking the value amongst the
LSS audience.

> It's still Nack.

Based solely on the diffstat and a quick look, this appears to be an
LSM patchset, not necessarily a BPF patchset; yes, there are kfuncs,
but they are LSM/audit kfuncs and not core BPF functionality.
Regardless, I want to see how the LSS presentation is received before
worrying about this too much, but your NACK has been noted.

-- 
paul-moore.com

^ permalink raw reply

* Re: [PATCH RFC bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
From: Alexei Starovoitov @ 2026-04-21 22:14 UTC (permalink / raw)
  To: Paul Moore, Linus Torvalds
  Cc: Frederick Lawler, James Morris, Serge E. Hallyn, Eric Paris,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, Mickaël Salaün, Günther Noack, LKML,
	LSM List, audit, bpf, open list:KERNEL SELFTEST FRAMEWORK,
	kernel-team
In-Reply-To: <CAHC9VhSo-M1wK2s2P0B+2F0pJqNguW7Kj-g63anp1xCMSBLa6Q@mail.gmail.com>

On Tue, Apr 21, 2026 at 3:10 PM Paul Moore <paul@paul-moore.com> wrote:
>
>
> > It's still Nack.
>
> Based solely on the diffstat and a quick look, this appears to be an
> LSM patchset, not necessarily a BPF patchset; yes, there are kfuncs,
> but they are LSM/audit kfuncs and not core BPF functionality.
> Regardless, I want to see how the LSS presentation is received before
> worrying about this too much, but your NACK has been noted.

Paul,

told you countless times LSM cannot touch BPF bits without
explicit Ack.

So, no, you cannot add bpf kfuncs in random places in the kernel
And, no, you cannot call bpf internals through bpf_map_ops, etc.

^ permalink raw reply

* Re: [PATCH RFC bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
From: Paul Moore @ 2026-04-21 22:49 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Linus Torvalds, Frederick Lawler, James Morris, Serge E. Hallyn,
	Eric Paris, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, Mickaël Salaün, Günther Noack, LKML,
	LSM List, audit, bpf, open list:KERNEL SELFTEST FRAMEWORK,
	kernel-team
In-Reply-To: <CAADnVQL33TOWFmxcWod3BCgmMXSQ=cEp6NCcxgQaJL0C7+Z9zA@mail.gmail.com>

On Tue, Apr 21, 2026 at 6:15 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Tue, Apr 21, 2026 at 3:10 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > > It's still Nack.
> >
> > Based solely on the diffstat and a quick look, this appears to be an
> > LSM patchset, not necessarily a BPF patchset; yes, there are kfuncs,
> > but they are LSM/audit kfuncs and not core BPF functionality.
> > Regardless, I want to see how the LSS presentation is received before
> > worrying about this too much, but your NACK has been noted.
>
> Paul,
>
> told you countless times LSM cannot touch BPF bits without
> explicit Ack.

Based on a quick glance it doesn't appear that Fred's patches touch
BPF bits, they simply supply kfuncs for users to use in their BPF
programs.

> So, no, you cannot add bpf kfuncs in random places in the kernel

I see plenty of places outside of kernel/bpf that define kfuncs.

> And, no, you cannot call bpf internals through bpf_map_ops, etc.

It doesn't appear that Fred is doing that in his patches.

-- 
paul-moore.com

^ permalink raw reply

* Re: [PATCH RFC bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
From: Alexei Starovoitov @ 2026-04-21 23:08 UTC (permalink / raw)
  To: Paul Moore
  Cc: Linus Torvalds, Frederick Lawler, James Morris, Serge E. Hallyn,
	Eric Paris, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, Mickaël Salaün, Günther Noack, LKML,
	LSM List, audit, bpf, open list:KERNEL SELFTEST FRAMEWORK,
	kernel-team
In-Reply-To: <CAHC9VhSKvC+p5j5Pe1YKB-=vRkRA81K3kp97Q6-H5K+kvYa0AQ@mail.gmail.com>

On Tue, Apr 21, 2026 at 3:50 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Tue, Apr 21, 2026 at 6:15 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> > On Tue, Apr 21, 2026 at 3:10 PM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > > It's still Nack.
> > >
> > > Based solely on the diffstat and a quick look, this appears to be an
> > > LSM patchset, not necessarily a BPF patchset; yes, there are kfuncs,
> > > but they are LSM/audit kfuncs and not core BPF functionality.
> > > Regardless, I want to see how the LSS presentation is received before
> > > worrying about this too much, but your NACK has been noted.
> >
> > Paul,
> >
> > told you countless times LSM cannot touch BPF bits without
> > explicit Ack.
>
> Based on a quick glance it doesn't appear that Fred's patches touch
> BPF bits, they simply supply kfuncs for users to use in their BPF
> programs.
>
> > So, no, you cannot add bpf kfuncs in random places in the kernel
>
> I see plenty of places outside of kernel/bpf that define kfuncs.

We discussed this already.
All those places were explicitly acked by BPF maintainers.

Every time somebody adds a kfunc it breaks safety, because
people don't read or don't understand Documentation/bpf/kfuncs.rst.
kfuncs are not export_symbol.
Object ownership model needs to be thought through.
Calling context needs to be analyzed and so on.
Just because something "works for me" it doesn't mean
that it's safe.

^ permalink raw reply

* Re: [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Feng Yang @ 2026-04-22  6:39 UTC (permalink / raw)
  To: stephen.smalley.work
  Cc: jmorris, linux-kernel, linux-security-module, paul, serge,
	yangfeng59949
In-Reply-To: <CAEjxPJ6jPh9_P=56VfAhGQG-Y=njosFuV_Kys9kzhkm7PxRPLg@mail.gmail.com>

On Mon, 13 Apr 2026 13:39:18 -0400, Stephen Smalley wrote:

[...]
> > This BUG_ON was first mentioned in [1], but I could not find any explanatory record of why this check is needed.
> >
> > [1] https://lore.kernel.org/all/Pine.LNX.4.64.0607122149070.573@d.namei/
> >
> > In the existing LSM_HOOK_INIT(xfrm_decode_session, selinux_xfrm_decode_session),
> > when the `ckall` parameter of the `selinux_xfrm_decode_session` function is 0,
> > it can only return 0 and will not trigger BUG_ON.
> > Therefore, remove the BUG_ON check to fix this issue.
> >
> > Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> > Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> > Closes: https://lore.kernel.org/all/4c4d04ba.6c12b.19c039b69e6.Coremail.kaiyanm@hust.edu.cn/
> > Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
> 
> Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
> 
> With the proviso that we likely ought to follow up with a clean-up
> that introduces a separate skb_classify_flow LSM hook that returns
> void so we don't awkwardly ignore errors below and defer handling to
> the individual security module.

Thank you for your review. Should the follow-up cleanup optimization for
adding a separate skb_classify_flow hook be submitted after this patch is merged?

> > ---
> >  security/security.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/security/security.c b/security/security.c
> > index 67af9228c4e9..198f650070da 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -4991,10 +4991,7 @@ int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid)
> >
> >  void security_skb_classify_flow(struct sk_buff *skb, struct flowi_common *flic)
> >  {
> > -       int rc = call_int_hook(xfrm_decode_session, skb, &flic->flowic_secid,
> > -                              0);
> > -
> > -       BUG_ON(rc);
> > +       call_int_hook(xfrm_decode_session, skb, &flic->flowic_secid, 0);
> >  }
> >  EXPORT_SYMBOL(security_skb_classify_flow);
> >  #endif /* CONFIG_SECURITY_NETWORK_XFRM */
> > --
> > 2.43.0


^ permalink raw reply

* Re: [PATCH] apparmor: Fix two bugs of aa_setup_dfa_engine's fail handling
From: GONG Ruiqi @ 2026-04-22  7:03 UTC (permalink / raw)
  To: John Johansen, Paul Moore, James Morris, Serge E . Hallyn
  Cc: apparmor, linux-security-module, linux-kernel, lujialin4
In-Reply-To: <20260403035119.2132418-1-gongruiqi1@huawei.com>

Kindly ping.

On 4/3/2026 11:51 AM, GONG Ruiqi wrote:
> First, aa_dfa_unpack returns ERR_PTR not NULL when it fails, but
> aa_put_dfa only checks NULL for its input, which would cause invalid
> memory access in aa_put_dfa. Set nulldfa to NULL explicitly to fix that.
> 
> Second, aa_put_pdb calls aa_pdb_free_kref -> aa_free_pdb -> aa_put_dfa,
> i.e.  it will free nullpdb->dfa. But there's another aa_put_dfa(nulldfa)
> after aa_put_pdb(nullpdb), which would cause double free. Remove that
> redundant aa_put_dfa to fix that.
> 
> Fixes: 98b824ff8984 ("apparmor: refcount the pdb")
> Signed-off-by: GONG Ruiqi <gongruiqi1@huawei.com>
> ---
>  security/apparmor/lsm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index c1d42fc72fdb..be82ec1b9fd9 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -2465,6 +2465,7 @@ static int __init aa_setup_dfa_engine(void)
>  			    TO_ACCEPT2_FLAG(YYTD_DATA32));
>  	if (IS_ERR(nulldfa)) {
>  		error = PTR_ERR(nulldfa);
> +		nulldfa = NULL;
>  		goto fail;
>  	}
>  	nullpdb->dfa = aa_get_dfa(nulldfa);
> @@ -2486,7 +2487,6 @@ static int __init aa_setup_dfa_engine(void)
>  
>  fail:
>  	aa_put_pdb(nullpdb);
> -	aa_put_dfa(nulldfa);
>  	nullpdb = NULL;
>  	nulldfa = NULL;
>  	stacksplitdfa = NULL;


^ permalink raw reply

* Re: [RFC PATCH 4/4] firmware: arm_ffa: check pkvm initailised when initailise ffa driver
From: Yeoreum Yun @ 2026-04-22 13:32 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Marc Zyngier, linux-security-module, linux-kernel,
	linux-integrity, linux-arm-kernel, kvmarm, paul, jmorris, zohar,
	roberto.sassu, dmitry.kasatkin, eric.snowberg, jarkko, oupton,
	joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas, will,
	sebastianene
In-Reply-To: <aec/YSxYO1bOhXhn@e129823.arm.com>

Hi All,

> > On Tue, Apr 21, 2026 at 07:57:43AM +0100, Yeoreum Yun wrote:
> >
> > [...]
> >
> > >
> > > Also, the FF-A initialization is not driven by a device probe, but rather
> > > happens as part of the bus registration itself,
> > > so it does not fit well with a device_link or probe deferral based approach.
> > >
> > > Instead, perhaps we could go with the idea I mentioned previously:
> > > either introduce a notifier, or create a pseudo ffa_device
> > > once pKVM initialization has completed, and
> > > then let the ffa driver perform the additional initialization from there.
> > >
> > > Am I missing something?
> > >
> >
> > In order to handle/cleanup some ugliness in interrupt management in the
> > FF-A driver, we may introduce DT node eventually. But it will take sometime.
>
> Unfortunately, I think this DT node wouldn't be helpful to solve
> this situation for dependency with the kvm misc device...
>
> IMHO, current situation, the notifier seems to good option. unless
> we make the initcall to recongise this dependency.
>

I think the best approach for now is to introduce a notifier to handle this situation.
If there are no further suggestions, I’ll send a v2 based on:
  - https://lore.kernel.org/all/aeS4rAeVQ0yJIPYw@e129823.arm.com/

Thanks!

--
Sincerely,
Yeoreum Yun

^ permalink raw reply

* Re: [PATCH RFC bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
From: Paul Moore @ 2026-04-22 14:33 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Linus Torvalds, Frederick Lawler, James Morris, Serge E. Hallyn,
	Eric Paris, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Shuah Khan, Mickaël Salaün, Günther Noack, LKML,
	LSM List, audit, bpf, open list:KERNEL SELFTEST FRAMEWORK,
	kernel-team
In-Reply-To: <CAADnVQJBOk8w89kj95M3GH-eFdC7njZt=zMEU=skB-DTb3k-bw@mail.gmail.com>

On Tue, Apr 21, 2026 at 7:08 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Tue, Apr 21, 2026 at 3:50 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Tue, Apr 21, 2026 at 6:15 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > > On Tue, Apr 21, 2026 at 3:10 PM Paul Moore <paul@paul-moore.com> wrote:
> > > >
> > > > > It's still Nack.
> > > >
> > > > Based solely on the diffstat and a quick look, this appears to be an
> > > > LSM patchset, not necessarily a BPF patchset; yes, there are kfuncs,
> > > > but they are LSM/audit kfuncs and not core BPF functionality.
> > > > Regardless, I want to see how the LSS presentation is received before
> > > > worrying about this too much, but your NACK has been noted.
> > >
> > > Paul,
> > >
> > > told you countless times LSM cannot touch BPF bits without
> > > explicit Ack.
> >
> > Based on a quick glance it doesn't appear that Fred's patches touch
> > BPF bits, they simply supply kfuncs for users to use in their BPF
> > programs.
> >
> > > So, no, you cannot add bpf kfuncs in random places in the kernel
> >
> > I see plenty of places outside of kernel/bpf that define kfuncs.
>
> We discussed this already.
> All those places were explicitly acked by BPF maintainers.

Perhaps.  However, we don't have to look further than your first
response in this thread to notice a prejudice against existing
in-kernel security/auditing mechanisms that large numbers of users
rely on every day.  While I'll continue to try working with you, and
encourage others to do the same, you've forced me into a position
where I need to consider sending patches to Linus that you've NACK'd.
I don't like it, it indicates a cultural breakdown and the formation
of rigid silos/fiefdoms within the kernel community, but I have a
responsibility to the other developers and users to represent and
advocate for their security needs.

> Every time somebody adds a kfunc it breaks safety, because
> people don't read or don't understand Documentation/bpf/kfuncs.rst.
> kfuncs are not export_symbol.
> Object ownership model needs to be thought through.
> Calling context needs to be analyzed and so on.
> Just because something "works for me" it doesn't mean
> that it's safe.

Unfortunately that isn't the review you provided Fred in this thread.
There were no comments about object ownership, calling context,
safety, etc., only a dismissive comment telling Fred to use something
else for logging.  If you want to provide proper feedback, something
along the lines of Kumar's constructive review, I think Fred would
welcome that.

-- 
paul-moore.com

^ permalink raw reply

* [RFC PATCH v2 0/4] fix FF-A call failed with pKVM when ff-a driver is built-in
From: Yeoreum Yun @ 2026-04-22 16:24 UTC (permalink / raw)
  To: linux-security-module, linux-kernel, linux-integrity,
	linux-arm-kernel, kvmarm
  Cc: paul, jmorris, serge, zohar, roberto.sassu, dmitry.kasatkin,
	eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
	suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
	sebastianene, Yeoreum Yun

commit 0e0546eabcd6 ("firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall")
changed the initcall level of ffa_init() to rootfs_initcall to address
an issue where IMA could not properly recognize the TPM device
when FF-A driver is built as built-in.

However, this introduces another problem: pKVM fails to handle FF-A calls
because it cannot trap the FFA_VERSION call invoked by ffa_init().

To ensure the TPM device is recognized when present in the system,
it is preferable to invoke again ima_init() at a later stage.
Deferred probing is resolved by deferred_probe_initcall(),
which runs at the late_initcall level.
Therefore, introduce an LSM initcall at late_initcall_sync and
invode ima_init() again to this level in case of TPM is probed yet
at the late_initcall stage.

With this change, revert the initcall level of ffa_init() back to
device_initcall. Additionally, to handle the case where ffa_init() runs
before kvm_init(), check whether pKVM has been initialized during ffa_init().
If not, defer initialization to prevent failures of FF-A calls
due to the inability to trap FFA_VERSION and FFA_RXTX_MAP in pKVM.

This patch is based on v7.0

Patch History
=============
from v1 to v2:
  - add notifier to make ffa-driver pkvm initialised.
  - modify to try initailisation again when IMA coudln't find proper TPM device.
  - https://lore.kernel.org/all/20260417175759.3191279-1-yeoreum.yun@arm.com/#t

Yeoreum Yun (4):
  security: ima: call ima_init() again at late_initcall_sync for defered
    TPM
  tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in
  firmware: arm_ffa: revert ffa_init() initcall level to device_initcall
  firmware: arm_ffa: check pkvm initailised when initailise ffa driver

 arch/arm64/include/asm/virt.h       | 11 +++++
 arch/arm64/kvm/arm.c                | 21 ++++++++
 arch/arm64/kvm/pkvm.c               |  2 +
 drivers/char/tpm/tpm_crb_ffa.c      | 18 ++-----
 drivers/firmware/arm_ffa/common.h   |  4 +-
 drivers/firmware/arm_ffa/driver.c   | 38 ++++++++++++++-
 drivers/firmware/arm_ffa/smccc.c    |  2 +-
 include/linux/lsm_hooks.h           |  2 +
 security/integrity/ima/ima.h        |  4 +-
 security/integrity/ima/ima_init.c   | 10 +++-
 security/integrity/ima/ima_main.c   | 76 +++++++++++++++++++++++------
 security/integrity/ima/ima_policy.c |  3 ++
 security/lsm_init.c                 | 13 ++++-
 13 files changed, 163 insertions(+), 41 deletions(-)


base-commit: 028ef9c96e96197026887c0f092424679298aae8
--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox