* [PATCH bpf-next v5 3/8] libbpf: Drop in-loader metadata check for load-time verification
From: Daniel Borkmann @ 2026-07-07 19:30 UTC (permalink / raw)
To: ast
Cc: kpsingh, James.Bottomley, paul, bboscaccy, memxor, torvalds,
a.s.protopopov, bpf, linux-security-module
In-Reply-To: <20260707193104.350066-1-daniel@iogearbox.net>
The signed gen_loader used to police its own metadata map from within
BPF: emit_signature_match() read the kernel-cached map->sha[] back
through hardcoded struct bpf_map offsets and compared it against a hash
that compute_sha_update_offsets() baked into the signed instructions,
after a BPF_OBJ_GET_INFO_BY_FD round-trip to populate map->sha[].
The kernel now verifies the metadata at BPF_PROG_LOAD time by folding
the frozen contents of the loader's exclusive fd_array maps into the
signature, so the loader no longer checks anything itself. Generated
loaders thus carry no verification logic of their own anymore: Nothing
in the signing chain depends on emitted loader bytecode doing the right
thing.
On the loading side, skel_internal.h now sets fd_array_cnt for a signed
load so the kernel scans fd_array for the exclusive metadata map -
still frozen, as the kernel requires - and the BPF_OBJ_GET_INFO_BY_FD
round-trip to populate map->sha[] is gone. The struct bpf_map layout
BUILD_BUG_ON()s on the kernel side are removed as well: they only
pinned the ABI for the in-BPF read of map->sha[] that is no longer
needed. Same for the map->excl member. Note: gen_hash is retained; it
still marks a loader as signed so an untrusted host cannot re-dimension
maps or override initial values now covered by the signature.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/linux/bpf.h | 1 -
kernel/bpf/syscall.c | 7 ---
tools/lib/bpf/bpf_gen_internal.h | 1 -
tools/lib/bpf/gen_loader.c | 76 +++-----------------------------
tools/lib/bpf/libbpf_internal.h | 1 -
tools/lib/bpf/skel_internal.h | 31 +------------
6 files changed, 9 insertions(+), 108 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index adf53f7edf28..c1a98fa36738 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -299,7 +299,6 @@ struct bpf_map_owner {
struct bpf_map {
u8 sha[SHA256_DIGEST_SIZE];
- u32 excl;
const struct bpf_map_ops *ops;
struct bpf_map *inner_map_meta;
#ifdef CONFIG_SECURITY
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index e898fad01aaf..358f2b0ce2bd 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1598,13 +1598,6 @@ static int map_create_alloc(union bpf_attr *attr, bpfptr_t uattr, struct bpf_ver
err = -EFAULT;
goto free_map;
}
-
- /* See libbpf: emit_signature_match() */
- BUILD_BUG_ON(offsetof(struct bpf_map, excl) != SHA256_DIGEST_SIZE);
- BUILD_BUG_ON(!__same_type(map->excl, u32));
- BUILD_BUG_ON(offsetof(struct bpf_map, sha) != 0);
- BUILD_BUG_ON(!__same_type(map->sha, u8[SHA256_DIGEST_SIZE]));
- map->excl = 1;
} else if (attr->excl_prog_hash_size) {
bpf_log(log, "Invalid excl_prog_hash_size.\n");
err = -EINVAL;
diff --git a/tools/lib/bpf/bpf_gen_internal.h b/tools/lib/bpf/bpf_gen_internal.h
index 49af4260b8e6..042569187752 100644
--- a/tools/lib/bpf/bpf_gen_internal.h
+++ b/tools/lib/bpf/bpf_gen_internal.h
@@ -51,7 +51,6 @@ struct bpf_gen {
__u32 nr_ksyms;
int fd_array;
int nr_fd_array;
- int hash_insn_offset[SHA256_DWORD_SIZE];
};
void bpf_gen__init(struct bpf_gen *gen, int log_level, int nr_progs, int nr_maps);
diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
index c7f2d2ac7bb3..6e3dd5242761 100644
--- a/tools/lib/bpf/gen_loader.c
+++ b/tools/lib/bpf/gen_loader.c
@@ -111,7 +111,6 @@ static void emit2(struct bpf_gen *gen, struct bpf_insn insn1, struct bpf_insn in
static int add_data(struct bpf_gen *gen, const void *data, __u32 size);
static void emit_sys_close_blob(struct bpf_gen *gen, int blob_off);
-static void emit_signature_match(struct bpf_gen *gen);
void bpf_gen__init(struct bpf_gen *gen, int log_level, int nr_progs, int nr_maps)
{
@@ -154,8 +153,6 @@ void bpf_gen__init(struct bpf_gen *gen, int log_level, int nr_progs, int nr_maps
/* R7 contains the error code from sys_bpf. Copy it into R0 and exit. */
emit(gen, BPF_MOV64_REG(BPF_REG_0, BPF_REG_7));
emit(gen, BPF_EXIT_INSN());
- if (OPTS_GET(gen->opts, gen_hash, false))
- emit_signature_match(gen);
}
static int add_data(struct bpf_gen *gen, const void *data, __u32 size)
@@ -377,8 +374,6 @@ static void emit_sys_close_blob(struct bpf_gen *gen, int blob_off)
__emit_sys_close(gen);
}
-static void compute_sha_update_offsets(struct bpf_gen *gen);
-
int bpf_gen__finish(struct bpf_gen *gen, int nr_progs, int nr_maps)
{
int i;
@@ -408,9 +403,6 @@ int bpf_gen__finish(struct bpf_gen *gen, int nr_progs, int nr_maps)
if (!gen->error) {
struct gen_loader_opts *opts = gen->opts;
- if (OPTS_GET(opts, gen_hash, false))
- compute_sha_update_offsets(gen);
-
opts->insns = gen->insn_start;
opts->insns_sz = gen->insn_cur - gen->insn_start;
opts->data = gen->data_start;
@@ -460,22 +452,6 @@ void bpf_gen__free(struct bpf_gen *gen)
_val; \
})
-static void compute_sha_update_offsets(struct bpf_gen *gen)
-{
- __u64 sha[SHA256_DWORD_SIZE];
- __u64 sha_dw;
- int i;
-
- libbpf_sha256(gen->data_start, gen->data_cur - gen->data_start, (__u8 *)sha);
- for (i = 0; i < SHA256_DWORD_SIZE; i++) {
- struct bpf_insn *insn =
- (struct bpf_insn *)(gen->insn_start + gen->hash_insn_offset[i]);
- sha_dw = tgt_endian(sha[i]);
- insn[0].imm = (__u32)sha_dw;
- insn[1].imm = sha_dw >> 32;
- }
-}
-
void bpf_gen__load_btf(struct bpf_gen *gen, const void *btf_raw_data,
__u32 btf_raw_size)
{
@@ -557,8 +533,9 @@ void bpf_gen__map_create(struct bpf_gen *gen,
* Conditionally update max_entries from the host-supplied loader
* ctx. This sizes the map at runtime, but for a signed loader
* (gen_hash) it would let an untrusted host re-dimension the
- * program's maps after emit_signature_match(), outside what the
- * signature attests to. Keep the signer-provided max_entries
+ * program's maps, outside what the signature attests to: the
+ * metadata blob is covered by the program signature and verified
+ * by the kernel at load time. Keep the signer-provided max_entries
* baked into the blob in that case.
*/
if (map_idx >= 0 && !OPTS_GET(gen->opts, gen_hash, false))
@@ -596,45 +573,6 @@ void bpf_gen__map_create(struct bpf_gen *gen,
emit_sys_close_stack(gen, stack_off(inner_map_fd));
}
-static void emit_signature_match(struct bpf_gen *gen)
-{
- __s64 off;
- int i;
-
- /*
- * Reject if the metadata map is not exclusive. Without exclusivity
- * the cached map->sha[] verified above can be stale: another BPF
- * program with map access could have mutated the contents between
- * BPF_OBJ_GET_INFO_BY_FD and loader execution.
- */
- emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_1, BPF_PSEUDO_MAP_IDX,
- 0, 0, 0, 0));
- emit(gen, BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, SHA256_DIGEST_LENGTH));
- off = -(gen->insn_cur - gen->insn_start - gen->cleanup_label) / 8 - 2;
- if (is_simm16(off)) {
- emit(gen, BPF_MOV64_IMM(BPF_REG_7, -EINVAL));
- emit(gen, BPF_JMP_IMM(BPF_JNE, BPF_REG_2, 1, off));
- } else {
- gen->error = -ERANGE;
- }
-
- for (i = 0; i < SHA256_DWORD_SIZE; i++) {
- emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_1, BPF_PSEUDO_MAP_IDX,
- 0, 0, 0, 0));
- emit(gen, BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_1, i * sizeof(__u64)));
- gen->hash_insn_offset[i] = gen->insn_cur - gen->insn_start;
- emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_3, 0, 0, 0, 0, 0));
-
- off = -(gen->insn_cur - gen->insn_start - gen->cleanup_label) / 8 - 2;
- if (is_simm16(off)) {
- emit(gen, BPF_MOV64_IMM(BPF_REG_7, -EINVAL));
- emit(gen, BPF_JMP_REG(BPF_JNE, BPF_REG_2, BPF_REG_3, off));
- } else {
- gen->error = -ERANGE;
- }
- }
-}
-
void bpf_gen__record_attach_target(struct bpf_gen *gen, const char *attach_name,
enum bpf_attach_type type)
{
@@ -1211,10 +1149,10 @@ void bpf_gen__map_update_elem(struct bpf_gen *gen, int map_idx, void *pvalue,
* }
*
* The runtime initial_value comes from the host-supplied loader
- * ctx and would overwrite the blob value after emit_signature_match()
- * has already validated map->sha[]. For a signed loader (gen_hash)
- * the attested blob value must be authoritative, so skip the override
- * and leave the hashed value in place.
+ * ctx and would overwrite the blob value that the program signature
+ * covers and the kernel verifies at load time. For a signed loader
+ * (gen_hash) the attested blob value must be authoritative, so skip
+ * the override and leave the signed value in place.
*/
if (!OPTS_GET(gen->opts, gen_hash, false)) {
emit(gen, BPF_LDX_MEM(BPF_DW, BPF_REG_3, BPF_REG_6,
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 04cd303fb5a8..d5b7db703b3f 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -768,7 +768,6 @@ int elf_resolve_pattern_offsets(const char *binary_path, const char *pattern,
int probe_fd(int fd);
#define SHA256_DIGEST_LENGTH 32
-#define SHA256_DWORD_SIZE SHA256_DIGEST_LENGTH / sizeof(__u64)
void libbpf_sha256(const void *data, size_t len, __u8 out[SHA256_DIGEST_LENGTH]);
int probe_sys_bpf_ext(void);
diff --git a/tools/lib/bpf/skel_internal.h b/tools/lib/bpf/skel_internal.h
index 74503d358bc8..53fee53d36d5 100644
--- a/tools/lib/bpf/skel_internal.h
+++ b/tools/lib/bpf/skel_internal.h
@@ -18,10 +18,6 @@
#include "bpf.h"
#endif
-#ifndef SHA256_DIGEST_LENGTH
-#define SHA256_DIGEST_LENGTH 32
-#endif
-
#ifndef __NR_bpf
# if defined(__mips__) && defined(_ABIO32)
# define __NR_bpf 4355
@@ -320,25 +316,6 @@ static inline int skel_link_create(int prog_fd, int target_fd,
return skel_sys_bpf(BPF_LINK_CREATE, &attr, attr_sz);
}
-static inline int skel_obj_get_info_by_fd(int fd)
-{
- const size_t attr_sz = offsetofend(union bpf_attr, info);
- __u8 sha[SHA256_DIGEST_LENGTH];
- struct bpf_map_info info;
- __u32 info_len = sizeof(info);
- union bpf_attr attr;
-
- memset(&info, 0, sizeof(info));
- info.hash = (long) &sha;
- info.hash_size = SHA256_DIGEST_LENGTH;
-
- memset(&attr, 0, attr_sz);
- attr.info.bpf_fd = fd;
- attr.info.info = (long) &info;
- attr.info.info_len = info_len;
- return skel_sys_bpf(BPF_OBJ_GET_INFO_BY_FD, &attr, attr_sz);
-}
-
static inline int skel_map_freeze(int fd)
{
const size_t attr_sz = offsetofend(union bpf_attr, map_fd);
@@ -384,12 +361,6 @@ static inline int bpf_load_and_run(struct bpf_load_and_run_opts *opts)
set_err;
goto out;
}
- err = skel_obj_get_info_by_fd(map_fd);
- if (err < 0) {
- opts->errstr = "failed to fetch obj info";
- set_err;
- goto out;
- }
#endif
memset(&attr, 0, prog_load_attr_sz);
@@ -400,6 +371,8 @@ static inline int bpf_load_and_run(struct bpf_load_and_run_opts *opts)
#ifndef __KERNEL__
attr.signature = (long) opts->signature;
attr.signature_size = opts->signature_sz;
+ if (opts->signature)
+ attr.fd_array_cnt = 1;
#else
if (opts->signature || opts->signature_sz)
pr_warn("signatures are not supported from bpf_preload\n");
--
2.43.0
^ permalink raw reply related
* [PATCH bpf-next v5 2/8] bpf: Verify signed loader metadata at load time
From: Daniel Borkmann @ 2026-07-07 19:30 UTC (permalink / raw)
To: ast
Cc: kpsingh, James.Bottomley, paul, bboscaccy, memxor, torvalds,
a.s.protopopov, bpf, linux-security-module
In-Reply-To: <20260707193104.350066-1-daniel@iogearbox.net>
A signed gen_loader program carries the programs, maps and relocations
it installs in a metadata array map. The loader instructions are covered
by the PKCS#7 signature, but the metadata map is not: Today the loader
compares the map contents from within BPF against a hash baked into its
(signed) instructions, using the kernel-cached map hash. The kernel
itself never actually attests that the metadata the loader installs is
the metadata that was signed.
This split is the core of the long-standing objection to the BPF signing
scheme from the LSM / integrity side: the integrity check of a light
skeleton only completes once the loader program runs, that is, after the
security_bpf_prog_load() hook, so at admission time an LSM observes a
program whose payload has not yet been verified. Auditing the chain
link is also not a purely cryptographic operation: whoever signs or
reviews an lskel has to disassemble the loader's preamble to convince
themselves that the embedded hash check is present and correct [0][1].
Two acceptable fixes were identified in those threads: Complete the
integrity check before the admission hook fires, or add a second hook
that collects the verification result after the loader ran [2]. Covering
both the loader and its maps directly with the PKCS#7 signature is what
Blaise Boscaccy's patchsets proposed in several forms. Let's implement
the former, without growing the UAPI, and in particular as a single
unified scheme where the signature spans the raw bytes rather than
derived hashes.
A signed loader binds its metadata map(s) through the existing fd_array,
and an exclusive map is already bound to a program digest (excl_prog_hash).
So when a signature is present, collect the exclusive maps from fd_array
and append their frozen contents to the instructions before verification:
The signature now covers insns || metadata_0 || metadata_1 || [...] in
the fd_array order, and verification completes in bpf_check(), once the
fd_array maps are resolved into used_maps, before the LSM admission hook
and the rest of verification. A program is either BPF_SIG_UNSIGNED or
BPF_SIG_VERIFIED, with nothing in between. While folding the fd_array
maps, a non-exclusive map bound to a signed program is rejected, so every
map folded into the signature is exclusive. A signed loader that fails
to cover its metadata thus does not load, and BPF_SIG_VERIFIED always
means the instructions and every exclusive map are authentic. The maps
must be frozen so the hashed bytes cannot change before the loader runs;
the map <-> program digest binding is enforced by the verifier for every
used map. Binding maps through fd_array_cnt makes the verifier resolve
and excl-check them (excl_prog_sha vs prog->digest) before it would
otherwise compute the digest, so compute prog->digest up front in
bpf_check(), over the unmodified instructions the signature covers, for
a load that folds metadata.
Unsigned programs are not affected by the signature path; for them the
LSM admission hook merely moves below fd_array resolution, with minimal
bounded work in between. Note, signed loaders generated by older libbpf/
bpftool versions need to be regenerated; some of the recent fixes we've
had on the signed loader side require the latter already to close gaps.
Finally, some remarks around the security_bpf_prog_load() placement
given there was discussion on whether a new hook is needed or the existing
security_bpf_prog() hook should be reused [3]: For a new hook it would
mean that just for loading a single BPF program it has to pass through
four layers of LSM hooks:
1) security_bpf (cmd=PROG_LOAD): for gating various bpf subcmds
2) security_bpf_prog_load: historical admission hook (CAP/token,
prog_type, attach point), pre-verification
3) security_bpf_prog_verify_signature: newly asked admission hook,
same role as 2), plus the BPF signature verdict
4) security_bpf_prog: gate handing the prog fd back to userspace,
verification done & signature verified
The use-cases of 2) and 3) conflate, thus BPF community prefers to just
keep a total of 3 LSM hooks (as-is today): 3) makes 2) incoherent given
they are the /same class/ of hook, that is, access-control admission on
the load and split only by _what_ they can see. Worse, with the split,
for a signed BPF program security_bpf_prog_load 2) admits a program whose
signature has not been checked, so a policy gating at 2) is structurally
unable to express "admit only verified" and every such policy is forced
onto 3) *anyway*. In other words, one doesn't get two complementary hooks,
but rather, one real admission hook aka 3) plus a now-degraded /legacy/
hook 2) that can't answer the question operators actually want to ask.
Reusing security_bpf_prog() 4) for admission is no alternative either:
it fires only after the entire verifier (and JIT) pipeline ran, so
denying a not-yet-verified program at that point burns exactly the
work a denial is supposed to avoid, and by then the program has an id
assigned and the kallsyms/perf/audit load events fired. Policies are
free to also consume the signature verdict at 4), but admission control
belongs into security_bpf_prog_load(). Hence the latter remains the only
admission hook, merely moved past signature verification; with moving
large allocations further down into the BPF verifier, there is now only
minimal work between the old and new location: The preparation work in
bpf_check() is reordered such that only the minimally necessary setup
happens up front: Allocating the env, initializing the verifier log and
resolving the fd_array that a signed BPF metadata map needs. The worst
case allocation up until security_bpf_prog_load() is ~90K which is the
env itself (~54K) plus the continuous fd_array cache (at most 32K). The
insn_aux_data array is moved into a later stage in the verification.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/2f71d6c03698eb17d51f7247efde777627ee578a.camel@HansenPartnership.com [0]
Link: https://lore.kernel.org/lkml/ecf0521ed302db672672ebfbc670ecfba36a6e00.camel@HansenPartnership.com [1]
Link: https://lore.kernel.org/bpf/88703f00d5b7a779728451008626efa45e42db3d.camel@HansenPartnership.com [2]
Link: https://lore.kernel.org/bpf/DJOFY21DYUI4.19WKQ3NPZ4H5R@gmail.com [3]
---
include/linux/bpf_verifier.h | 1 +
kernel/bpf/syscall.c | 76 +-----------
kernel/bpf/verifier.c | 228 +++++++++++++++++++++++++++++++----
3 files changed, 209 insertions(+), 96 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index bb57773cde37..317e99b9acc0 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -947,6 +947,7 @@ struct bpf_verifier_env {
bool bypass_spec_v4;
bool seen_direct_write;
bool seen_exception;
+ bool signature;
struct bpf_insn_aux_data *insn_aux_data; /* array of per-insn state */
const struct bpf_line_info *prev_linfo;
struct bpf_verifier_log log;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 6db306d23b47..e898fad01aaf 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -40,7 +40,6 @@
#include <linux/tracepoint.h>
#include <linux/overflow.h>
#include <linux/cookie.h>
-#include <linux/verification.h>
#include <linux/btf_ids.h>
#include <net/netfilter/nf_bpf_link.h>
@@ -2886,64 +2885,6 @@ static bool is_perfmon_prog_type(enum bpf_prog_type prog_type)
}
}
-static enum bpf_sig_keyring bpf_classify_keyring(s32 keyring_id)
-{
- switch (keyring_id) {
- case 0:
- return BPF_SIG_KEYRING_BUILTIN;
- case (s32)(unsigned long)VERIFY_USE_SECONDARY_KEYRING:
- return BPF_SIG_KEYRING_SECONDARY;
- case (s32)(unsigned long)VERIFY_USE_PLATFORM_KEYRING:
- return BPF_SIG_KEYRING_PLATFORM;
- default:
- return BPF_SIG_KEYRING_USER;
- }
-}
-
-static int bpf_prog_verify_signature(struct bpf_prog *prog, union bpf_attr *attr,
- bool is_kernel, s32 *keyring_serial)
-{
- bpfptr_t usig = make_bpfptr(attr->signature, is_kernel);
- struct bpf_dynptr_kern sig_ptr, insns_ptr;
- struct bpf_key *key = NULL;
- void *sig;
- int err = 0;
-
- /*
- * Don't attempt to use kmalloc_large or vmalloc for signatures.
- * Practical signature for BPF program should be below this limit.
- */
- if (attr->signature_size > KMALLOC_MAX_CACHE_SIZE)
- return -EINVAL;
-
- if (system_keyring_id_check(attr->keyring_id) == 0)
- key = bpf_lookup_system_key(attr->keyring_id);
- else
- key = bpf_lookup_user_key(attr->keyring_id, 0);
-
- if (!key)
- return -EINVAL;
-
- sig = kvmemdup_bpfptr(usig, attr->signature_size);
- if (IS_ERR(sig)) {
- bpf_key_put(key);
- return PTR_ERR(sig);
- }
-
- bpf_dynptr_init(&sig_ptr, sig, BPF_DYNPTR_TYPE_LOCAL, 0,
- attr->signature_size);
- bpf_dynptr_init(&insns_ptr, prog->insnsi, BPF_DYNPTR_TYPE_LOCAL, 0,
- prog->len * sizeof(struct bpf_insn));
-
- err = bpf_verify_pkcs7_signature((struct bpf_dynptr *)&insns_ptr,
- (struct bpf_dynptr *)&sig_ptr, key);
- if (!err)
- *keyring_serial = bpf_key_serial(key);
- bpf_key_put(key);
- kvfree(sig);
- return err;
-}
-
static int bpf_prog_mark_insn_arrays_ready(struct bpf_prog *prog)
{
int err;
@@ -3133,17 +3074,8 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, struct bpf_log_at
/* eBPF programs must be GPL compatible to use GPL-ed functions */
prog->gpl_compatible = license_is_gpl_compatible(license) ? 1 : 0;
- if (attr->signature) {
- err = bpf_prog_verify_signature(prog, attr, uattr.is_kernel,
- &prog->aux->sig.keyring_serial);
- if (err)
- goto free_prog;
- prog->aux->sig.keyring_type = bpf_classify_keyring(attr->keyring_id);
- prog->aux->sig.verdict = BPF_SIG_VERIFIED;
- } else {
- prog->aux->sig.keyring_type = BPF_SIG_KEYRING_NONE;
- prog->aux->sig.verdict = BPF_SIG_UNSIGNED;
- }
+ prog->aux->sig.keyring_type = BPF_SIG_KEYRING_NONE;
+ prog->aux->sig.verdict = BPF_SIG_UNSIGNED;
prog->orig_prog = NULL;
prog->jited = 0;
@@ -3189,10 +3121,6 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, struct bpf_log_at
if (err < 0)
goto free_prog;
- err = security_bpf_prog_load(prog, attr, token, uattr.is_kernel);
- if (err)
- goto free_prog;
-
/* run eBPF verifier */
err = bpf_check(&prog, attr, uattr, attr_log);
if (err < 0)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 2e9e65297c70..462a1f8544b9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -22,6 +22,8 @@
#include <linux/ctype.h>
#include <linux/error-injection.h>
#include <linux/bpf_lsm.h>
+#include <linux/security.h>
+#include <linux/verification.h>
#include <linux/btf_ids.h>
#include <linux/poison.h>
#include <linux/module.h>
@@ -2554,6 +2556,10 @@ fd_array_get_btf_sparse(struct bpf_verifier_env *env, u32 idx)
static struct btf *fd_array_get_btf(struct bpf_verifier_env *env, u32 idx)
{
+ if (env->signature) {
+ verbose(env, "signed program cannot bind any BTF\n");
+ return ERR_PTR(-EACCES);
+ }
if (env->fd_array)
return fd_array_get_btf_continuous(env, idx);
if (!bpfptr_is_null(env->fd_array_raw))
@@ -17646,6 +17652,11 @@ static int __add_used_btf(struct bpf_verifier_env *env, struct btf *btf)
if (env->used_btfs[i].btf == btf)
goto ret_put;
+ if (env->signature) {
+ verbose(env, "signed program cannot bind any BTF\n");
+ ret = -EACCES;
+ goto ret_put;
+ }
if (env->used_btf_cnt >= MAX_USED_BTFS) {
verbose(env, "The total number of btfs per program has reached the limit of %u\n",
MAX_USED_BTFS);
@@ -17928,6 +17939,12 @@ static int __add_used_map(struct bpf_verifier_env *env, struct bpf_map *map)
if (env->used_maps[i] == map)
return i;
+ if (env->signature &&
+ env->prog->aux->sig.verdict == BPF_SIG_VERIFIED) {
+ verbose(env, "signed program cannot bind map '%s' not covered by the signature\n",
+ map->name);
+ return -EACCES;
+ }
if (env->used_map_cnt >= MAX_USED_MAPS) {
verbose(env, "The total number of maps per program has reached the limit of %u\n",
MAX_USED_MAPS);
@@ -18011,6 +18028,10 @@ static int fd_array_get_map_idx(struct bpf_verifier_env *env, u32 idx)
{
if (env->fd_array)
return fd_array_get_map_idx_continuous(env, idx);
+ if (env->signature) {
+ verbose(env, "signed program must bind maps via a continuous fd_array (fd_array_cnt)\n");
+ return -EACCES;
+ }
if (!bpfptr_is_null(env->fd_array_raw))
return fd_array_get_map_idx_sparse(env, idx);
@@ -18289,6 +18310,10 @@ static int check_and_resolve_insns(struct bpf_verifier_env *env)
map_idx = fd_array_get_map_idx(env, insn[0].imm);
break;
default:
+ if (env->signature) {
+ verbose(env, "signed program cannot reference a map by fd, only via fd_array index\n");
+ return -EINVAL;
+ }
map_idx = add_used_map(env, insn[0].imm);
break;
}
@@ -19870,6 +19895,140 @@ int bpf_fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
return 0;
}
+static enum bpf_sig_keyring bpf_classify_keyring(s32 keyring_id)
+{
+ switch (keyring_id) {
+ case 0:
+ return BPF_SIG_KEYRING_BUILTIN;
+ case (s32)(unsigned long)VERIFY_USE_SECONDARY_KEYRING:
+ return BPF_SIG_KEYRING_SECONDARY;
+ case (s32)(unsigned long)VERIFY_USE_PLATFORM_KEYRING:
+ return BPF_SIG_KEYRING_PLATFORM;
+ default:
+ return BPF_SIG_KEYRING_USER;
+ }
+}
+
+/*
+ * Verify the PKCS#7 signature of a loaded program. Called from bpf_check()
+ * once the program's metadata maps have been resolved into used_maps, so
+ * the exact maps folded into the signature are the ones the program binds.
+ *
+ * The signature covers the instructions followed by the frozen contents of
+ * each map, in @maps order: insns || map_0 || map_1 || [...]. On success the
+ * verdict and keyring info are recorded on prog->aux.
+ */
+static int bpf_prog_verify_signature(struct bpf_verifier_env *env,
+ union bpf_attr *attr, bool is_kernel)
+{
+ bpfptr_t usig = make_bpfptr(attr->signature, is_kernel);
+ struct bpf_dynptr_kern sig_ptr, data_ptr;
+ struct bpf_prog *prog = env->prog;
+ struct bpf_map **maps = env->used_maps;
+ struct bpf_key *key = NULL;
+ void *sig, *data = NULL;
+ u32 map_cnt = env->used_map_cnt;
+ u32 i, off, insns_sz;
+ u64 data_sz;
+ int err = 0;
+
+ /*
+ * Don't attempt to use kmalloc_large or vmalloc for signatures.
+ * Practical signature for BPF program should be below this limit.
+ */
+ if (!attr->signature_size ||
+ attr->signature_size > KMALLOC_MAX_CACHE_SIZE)
+ return -EINVAL;
+ if (system_keyring_id_check(attr->keyring_id) == 0)
+ key = bpf_lookup_system_key(attr->keyring_id);
+ else
+ key = bpf_lookup_user_key(attr->keyring_id, 0);
+ if (!key) {
+ verbose(env, "cannot resolve signing keyring with keyring_id %d\n",
+ attr->keyring_id);
+ return -EINVAL;
+ }
+
+ sig = kvmemdup_bpfptr(usig, attr->signature_size);
+ if (IS_ERR(sig)) {
+ bpf_key_put(key);
+ return PTR_ERR(sig);
+ }
+
+ insns_sz = prog->len * sizeof(struct bpf_insn);
+ data_sz = insns_sz;
+ for (i = 0; i < map_cnt; i++) {
+ struct bpf_map *map = maps[i];
+
+ if (map->map_type != BPF_MAP_TYPE_ARRAY ||
+ !map->ops->map_direct_value_addr) {
+ verbose(env, "signed program metadata map '%s' must be an array\n",
+ map->name);
+ err = -EINVAL;
+ goto out;
+ }
+ if (!READ_ONCE(map->frozen)) {
+ verbose(env, "signed program metadata map '%s' must be frozen\n",
+ map->name);
+ err = -EPERM;
+ goto out;
+ }
+ if (!map->excl_prog_sha) {
+ verbose(env, "signed program metadata map '%s' must be exclusive\n",
+ map->name);
+ err = -EPERM;
+ goto out;
+ }
+ data_sz += map->value_size;
+ }
+ if (bpf_dynptr_check_size(data_sz)) {
+ verbose(env, "signed payload too large: %llu bytes\n", data_sz);
+ err = -E2BIG;
+ goto out;
+ }
+ data = kvmalloc(data_sz, GFP_KERNEL_ACCOUNT | __GFP_ZERO);
+ if (!data) {
+ err = -ENOMEM;
+ goto out;
+ }
+ memcpy(data, prog->insnsi, insns_sz);
+ off = insns_sz;
+ for (i = 0; i < map_cnt; i++) {
+ struct bpf_map *map = maps[i];
+ u64 addr;
+
+ err = map->ops->map_direct_value_addr(map, &addr, 0);
+ if (err) {
+ verbose(env, "failed to read signed metadata map '%s': %d\n",
+ map->name, err);
+ goto out;
+ }
+ memcpy(data + off, (void *)(unsigned long)addr,
+ map->value_size);
+ off += map->value_size;
+ }
+
+ bpf_dynptr_init(&data_ptr, data, BPF_DYNPTR_TYPE_LOCAL, 0, data_sz);
+ bpf_dynptr_init(&sig_ptr, sig, BPF_DYNPTR_TYPE_LOCAL, 0,
+ attr->signature_size);
+
+ err = bpf_verify_pkcs7_signature((struct bpf_dynptr *)&data_ptr,
+ (struct bpf_dynptr *)&sig_ptr, key);
+ if (err) {
+ verbose(env, "signature verification failed: %d\n", err);
+ } else {
+ verbose(env, "signature verification passed\n");
+ prog->aux->sig.keyring_serial = bpf_key_serial(key);
+ prog->aux->sig.keyring_type = bpf_classify_keyring(attr->keyring_id);
+ prog->aux->sig.verdict = BPF_SIG_VERIFIED;
+ }
+out:
+ kvfree(data);
+ bpf_key_put(key);
+ kvfree(sig);
+ return err;
+}
+
int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
struct bpf_log_attr *attr_log)
{
@@ -19892,18 +20051,6 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
return -ENOMEM;
env->bt.env = env;
-
- len = (*prog)->len;
- env->insn_aux_data =
- vzalloc(array_size(sizeof(struct bpf_insn_aux_data), len));
- ret = -ENOMEM;
- if (!env->insn_aux_data)
- goto err_free_env;
- for (i = 0; i < len; i++)
- env->insn_aux_data[i].orig_idx = i;
- env->succ = bpf_iarray_realloc(NULL, 2);
- if (!env->succ)
- goto err_free_env;
env->prog = *prog;
env->ops = bpf_verifier_ops[env->prog->type];
@@ -19912,22 +20059,51 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
env->bypass_spec_v1 = bpf_bypass_spec_v1(env->prog->aux->token);
env->bypass_spec_v4 = bpf_bypass_spec_v4(env->prog->aux->token);
env->bpf_capable = is_priv = bpf_token_capable(env->prog->aux->token, CAP_BPF);
-
- bpf_get_btf_vmlinux();
-
- /* grab the mutex to protect few globals used by verifier */
- if (!is_priv)
- mutex_lock(&bpf_verifier_lock);
+ env->signature = attr->signature;
/* user could have requested verbose verifier output
* and supplied buffer to store the verification trace
*/
ret = bpf_vlog_init(&env->log, attr_log->level, attr_log->ubuf, attr_log->size);
if (ret)
- goto err_unlock;
+ goto err_free_env;
+ if (env->signature) {
+ ret = bpf_prog_calc_tag(env->prog);
+ if (ret < 0)
+ goto err_prep;
+ }
ret = process_fd_array(env, attr, uattr);
if (ret)
+ goto err_prep;
+
+ if (env->signature) {
+ ret = bpf_prog_verify_signature(env, attr, uattr.is_kernel);
+ if (ret)
+ goto err_prep;
+ }
+
+ ret = security_bpf_prog_load(env->prog, attr, env->prog->aux->token,
+ uattr.is_kernel);
+ if (ret)
+ goto err_prep;
+
+ bpf_get_btf_vmlinux();
+
+ /* grab the mutex to protect few globals used by verifier */
+ if (!is_priv)
+ mutex_lock(&bpf_verifier_lock);
+
+ len = env->prog->len;
+ env->insn_aux_data =
+ vzalloc(array_size(sizeof(struct bpf_insn_aux_data), len));
+ ret = -ENOMEM;
+ if (!env->insn_aux_data)
+ goto skip_full_check;
+ for (i = 0; i < len; i++)
+ env->insn_aux_data[i].orig_idx = i;
+ env->succ = bpf_iarray_realloc(NULL, 2);
+ if (!env->succ)
goto skip_full_check;
mark_verifier_state_clean(env);
@@ -20151,18 +20327,26 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
*prog = env->prog;
module_put(env->attach_btf_mod);
-err_unlock:
if (!is_priv)
mutex_unlock(&bpf_verifier_lock);
- bpf_clear_insn_aux_data(env, 0, env->prog->len);
+ goto err_free_env;
+err_prep:
+ err = bpf_log_attr_finalize(attr_log, &env->log);
+ if (err)
+ ret = err;
+ release_insn_arrays(env);
+ release_maps(env);
+ release_btfs(env);
err_free_env:
+ if (env->insn_aux_data)
+ bpf_clear_insn_aux_data(env, 0, env->prog->len);
+ vfree(env->insn_aux_data);
kvfree(env->fd_array);
bpf_stack_liveness_free(env);
kvfree(env->cfg.insn_postorder);
kvfree(env->scc_info);
kvfree(env->succ);
kvfree(env->gotox_tmp_buf);
- vfree(env->insn_aux_data);
kvfree(env);
return ret;
}
--
2.43.0
^ permalink raw reply related
* [PATCH bpf-next v5 1/8] bpf: Resolve and cache fd_array objects at load time
From: Daniel Borkmann @ 2026-07-07 19:30 UTC (permalink / raw)
To: ast
Cc: kpsingh, James.Bottomley, paul, bboscaccy, memxor, torvalds,
a.s.protopopov, bpf, linux-security-module
In-Reply-To: <20260707193104.350066-1-daniel@iogearbox.net>
The fd_array passed to BPF_PROG_LOAD carries the map and module BTF file
descriptors a program binds. The verifier reads it more than once during
a load: process_fd_array() walks it to bind the maps and BTFs, and
check_and_resolve_insns() and the kfunc BTF resolver later read it again
to resolve the program's BPF_PSEUDO_MAP_IDX* and module kfunc refs.
For signed BPF, we need these upfront in memory, thus resolve each fd to
its object once and cache it by fd_array index, then bind that cached
object for the rest of the load. env->fd_array becomes a small per-slot
{map, btf} cache rather than a bpfptr_t; every later reference is then
an in-bounds lookup of an already-resolved object, and an index outside
the cache is rejected instead of read from user memory:
- continuous (fd_array_cnt given): the caller declares the length and
every entry is resolved and bound up front (used also by the BPF
signed loader)
- sparse (no fd_array_cnt): left as the legacy path with no fd_array
cache; each reference reads its fd from the caller's fd_array and
resolves it on the spot. Deduplication in used_maps and the kfunc BTF
table keeps this correct, and only unsigned programs use this shape.
Split these into separate helpers to make it easier to follow.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Anton Protopopov <a.s.protopopov@gmail.com>
---
include/linux/bpf_verifier.h | 22 +++-
kernel/bpf/verifier.c | 223 +++++++++++++++++++++++++++--------
2 files changed, 193 insertions(+), 52 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 76b8b7627a10..bb57773cde37 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -898,6 +898,14 @@ struct bpf_scc_info {
struct bpf_liveness;
+struct bpf_fd_array {
+ union {
+ struct bpf_map *map;
+ struct btf *btf;
+ unsigned long val;
+ };
+};
+
/* single container for all structs
* one verifier_env per bpf_check() call
*/
@@ -989,7 +997,19 @@ struct bpf_verifier_env {
u32 free_list_size;
u32 explored_states_size;
u32 num_backedges;
- bpfptr_t fd_array;
+ /*
+ * The program's fd_array comes in two shapes, told apart by whether
+ * the caller passed fd_array_cnt. They are mutually exclusive:
+ * - continuous (fd_array_cnt given): ->fd_array holds every entry
+ * resolved to its object up front, indexed by fd_array position,
+ * with ->fd_array_cnt slots; ->fd_array_raw is unused.
+ * - sparse (no fd_array_cnt): ->fd_array is NULL, and entries are
+ * read from ->fd_array_raw (the caller's fd_array) and resolved
+ * on the spot at each reference.
+ */
+ struct bpf_fd_array *fd_array;
+ u32 fd_array_cnt;
+ bpfptr_t fd_array_raw;
/* bit mask to keep track of whether a register has been accessed
* since the last time the function state was printed
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 3193b473762b..2e9e65297c70 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2490,6 +2490,79 @@ int bpf_get_kfunc_addr(const struct bpf_prog *prog, u32 func_id,
return 0;
}
+#define BPF_FD_SLOT_BTF 1UL
+
+static void fd_slot_set_map(struct bpf_fd_array *slot, struct bpf_map *map)
+{
+ slot->val = (unsigned long)map;
+}
+
+static void fd_slot_set_btf(struct bpf_fd_array *slot, struct btf *btf)
+{
+ slot->val = (unsigned long)btf | BPF_FD_SLOT_BTF;
+}
+
+static struct bpf_map *fd_slot_map(struct bpf_fd_array slot)
+{
+ if (slot.val & BPF_FD_SLOT_BTF)
+ return NULL;
+ return (struct bpf_map *)slot.val;
+}
+
+static struct btf *fd_slot_btf(struct bpf_fd_array slot)
+{
+ if (!(slot.val & BPF_FD_SLOT_BTF))
+ return NULL;
+ return (struct btf *)(slot.val & ~BPF_FD_SLOT_BTF);
+}
+
+static struct btf *
+fd_array_get_btf_continuous(struct bpf_verifier_env *env, u32 idx)
+{
+ struct btf *btf;
+
+ if (idx >= env->fd_array_cnt) {
+ verbose(env, "kfunc fd_idx %u out of bounds, fd_array_cnt %u\n",
+ idx, env->fd_array_cnt);
+ return ERR_PTR(-EINVAL);
+ }
+ btf = fd_slot_btf(env->fd_array[idx]);
+ if (!btf) {
+ verbose(env, "kfunc fd_idx %u is not a module BTF\n", idx);
+ return ERR_PTR(-EINVAL);
+ }
+ btf_get(btf);
+ return btf;
+}
+
+static struct btf *
+fd_array_get_btf_sparse(struct bpf_verifier_env *env, u32 idx)
+{
+ struct btf *btf;
+ int btf_fd;
+
+ if (copy_from_bpfptr_offset(&btf_fd, env->fd_array_raw,
+ (size_t)idx * sizeof(btf_fd), sizeof(btf_fd)))
+ return ERR_PTR(-EFAULT);
+ btf = btf_get_by_fd(btf_fd);
+ if (IS_ERR(btf)) {
+ verbose(env, "invalid module BTF fd specified\n");
+ return btf;
+ }
+ return btf;
+}
+
+static struct btf *fd_array_get_btf(struct bpf_verifier_env *env, u32 idx)
+{
+ if (env->fd_array)
+ return fd_array_get_btf_continuous(env, idx);
+ if (!bpfptr_is_null(env->fd_array_raw))
+ return fd_array_get_btf_sparse(env, idx);
+
+ verbose(env, "kfunc offset > 0 without fd_array is invalid\n");
+ return ERR_PTR(-EPROTO);
+}
+
static struct btf *__find_kfunc_desc_btf(struct bpf_verifier_env *env,
s16 offset)
{
@@ -2498,7 +2571,6 @@ static struct btf *__find_kfunc_desc_btf(struct bpf_verifier_env *env,
struct bpf_kfunc_btf *b;
struct module *mod;
struct btf *btf;
- int btf_fd;
tab = env->prog->aux->kfunc_btf_tab;
b = bsearch(&kf_btf, tab->descs, tab->nr_descs,
@@ -2509,22 +2581,9 @@ static struct btf *__find_kfunc_desc_btf(struct bpf_verifier_env *env,
return ERR_PTR(-E2BIG);
}
- if (bpfptr_is_null(env->fd_array)) {
- verbose(env, "kfunc offset > 0 without fd_array is invalid\n");
- return ERR_PTR(-EPROTO);
- }
-
- if (copy_from_bpfptr_offset(&btf_fd, env->fd_array,
- offset * sizeof(btf_fd),
- sizeof(btf_fd)))
- return ERR_PTR(-EFAULT);
-
- btf = btf_get_by_fd(btf_fd);
- if (IS_ERR(btf)) {
- verbose(env, "invalid module BTF fd specified\n");
+ btf = fd_array_get_btf(env, offset);
+ if (IS_ERR(btf))
return btf;
- }
-
if (!btf_is_module(btf)) {
verbose(env, "BTF fd for kfunc is not a module BTF\n");
btf_put(btf);
@@ -17921,6 +17980,44 @@ static int add_used_map(struct bpf_verifier_env *env, int fd)
return __add_used_map(env, map);
}
+static int fd_array_get_map_idx_continuous(struct bpf_verifier_env *env, u32 idx)
+{
+ struct bpf_map *map;
+
+ if (idx >= env->fd_array_cnt) {
+ verbose(env, "fd_idx %u out of bounds, fd_array_cnt %u\n",
+ idx, env->fd_array_cnt);
+ return -EINVAL;
+ }
+ map = fd_slot_map(env->fd_array[idx]);
+ if (!map) {
+ verbose(env, "fd_idx %u is not a map\n", idx);
+ return -EINVAL;
+ }
+ return __add_used_map(env, map);
+}
+
+static int fd_array_get_map_idx_sparse(struct bpf_verifier_env *env, u32 idx)
+{
+ int fd;
+
+ if (copy_from_bpfptr_offset(&fd, env->fd_array_raw,
+ (size_t)idx * sizeof(fd), sizeof(fd)))
+ return -EFAULT;
+ return add_used_map(env, fd);
+}
+
+static int fd_array_get_map_idx(struct bpf_verifier_env *env, u32 idx)
+{
+ if (env->fd_array)
+ return fd_array_get_map_idx_continuous(env, idx);
+ if (!bpfptr_is_null(env->fd_array_raw))
+ return fd_array_get_map_idx_sparse(env, idx);
+
+ verbose(env, "fd_idx without fd_array is invalid\n");
+ return -EPROTO;
+}
+
static int check_alu_fields(struct bpf_verifier_env *env, struct bpf_insn *insn)
{
u8 class = BPF_CLASS(insn->code);
@@ -18138,7 +18235,6 @@ static int check_and_resolve_insns(struct bpf_verifier_env *env)
struct bpf_map *map;
int map_idx;
u64 addr;
- u32 fd;
if (i == insn_cnt - 1 || insn[1].code != 0 ||
insn[1].dst_reg != 0 || insn[1].src_reg != 0 ||
@@ -18190,21 +18286,13 @@ static int check_and_resolve_insns(struct bpf_verifier_env *env)
switch (insn[0].src_reg) {
case BPF_PSEUDO_MAP_IDX_VALUE:
case BPF_PSEUDO_MAP_IDX:
- if (bpfptr_is_null(env->fd_array)) {
- verbose(env, "fd_idx without fd_array is invalid\n");
- return -EPROTO;
- }
- if (copy_from_bpfptr_offset(&fd, env->fd_array,
- insn[0].imm * sizeof(fd),
- sizeof(fd)))
- return -EFAULT;
+ map_idx = fd_array_get_map_idx(env, insn[0].imm);
break;
default:
- fd = insn[0].imm;
+ map_idx = add_used_map(env, insn[0].imm);
break;
}
- map_idx = add_used_map(env, fd);
if (map_idx < 0)
return map_idx;
map = env->used_maps[map_idx];
@@ -19479,7 +19567,7 @@ struct btf *bpf_get_btf_vmlinux(void)
* this case expect that every file descriptor in the array is either a map or
* a BTF. Everything else is considered to be trash.
*/
-static int add_fd_from_fd_array(struct bpf_verifier_env *env, int fd)
+static int add_fd_from_fd_array(struct bpf_verifier_env *env, u32 idx, int fd)
{
struct bpf_map *map;
struct btf *btf;
@@ -19491,51 +19579,83 @@ static int add_fd_from_fd_array(struct bpf_verifier_env *env, int fd)
err = __add_used_map(env, map);
if (err < 0)
return err;
+ fd_slot_set_map(&env->fd_array[idx], map);
return 0;
}
btf = __btf_get_by_fd(f);
if (!IS_ERR(btf)) {
btf_get(btf);
- return __add_used_btf(env, btf);
+ err = __add_used_btf(env, btf);
+ if (err < 0)
+ return err;
+ fd_slot_set_btf(&env->fd_array[idx], btf);
+ return 0;
}
verbose(env, "fd %d is not pointing to valid bpf_map or btf\n", fd);
return PTR_ERR(map);
}
-static int process_fd_array(struct bpf_verifier_env *env, union bpf_attr *attr, bpfptr_t uattr)
+/*
+ * A continuous fd_array is resolved into an in-memory cache with one slot
+ * per entry. The bound here is deliberately generous and not derived from
+ * the per-program object limits: Duplicate entries /are/ permitted, and
+ * the number of distinct maps and BTFs a program can bind is enforced when
+ * each entry is resolved by __add_used_map() and __add_used_btf().
+ */
+#define MAX_FD_ARRAY_CNT 4096
+
+static int process_fd_array_continuous(struct bpf_verifier_env *env,
+ bpfptr_t fd_array, u32 cnt)
{
- size_t size = sizeof(int);
- int ret;
- int fd;
+ int fd, ret;
u32 i;
- env->fd_array = make_bpfptr(attr->fd_array, uattr.is_kernel);
-
- /*
- * The only difference between old (no fd_array_cnt is given) and new
- * APIs is that in the latter case the fd_array is expected to be
- * continuous and is scanned for map fds right away
- */
- if (!attr->fd_array_cnt)
- return 0;
-
- /* Check for integer overflow */
- if (attr->fd_array_cnt >= (U32_MAX / size)) {
- verbose(env, "fd_array_cnt is too big (%u)\n", attr->fd_array_cnt);
- return -EINVAL;
+ if (cnt > MAX_FD_ARRAY_CNT) {
+ verbose(env, "fd_array has too many entries (%u, max %u)\n",
+ cnt, MAX_FD_ARRAY_CNT);
+ return -E2BIG;
}
- for (i = 0; i < attr->fd_array_cnt; i++) {
- if (copy_from_bpfptr_offset(&fd, env->fd_array, i * size, size))
+ env->fd_array = kvcalloc(cnt, sizeof(*env->fd_array),
+ GFP_KERNEL_ACCOUNT);
+ if (!env->fd_array)
+ return -ENOMEM;
+ env->fd_array_cnt = cnt;
+ for (i = 0; i < cnt; i++) {
+ if (copy_from_bpfptr_offset(&fd, fd_array,
+ (size_t)i * sizeof(fd), sizeof(fd)))
return -EFAULT;
-
- ret = add_fd_from_fd_array(env, fd);
+ ret = add_fd_from_fd_array(env, i, fd);
if (ret)
return ret;
}
+ return 0;
+}
+static int process_fd_array(struct bpf_verifier_env *env,
+ union bpf_attr *attr, bpfptr_t uattr)
+{
+ bpfptr_t fd_array = make_bpfptr(attr->fd_array, uattr.is_kernel);
+
+ if (bpfptr_is_null(fd_array)) {
+ if (attr->fd_array_cnt) {
+ verbose(env, "fd_array_cnt %u without fd_array is invalid\n",
+ attr->fd_array_cnt);
+ return -EINVAL;
+ }
+ return 0;
+ }
+ /*
+ * New API: the caller passes fd_array_cnt and a continuous array that
+ * is resolved and bound up front. Legacy API (no fd_array_cnt): keep
+ * the caller's array and resolve entries on the spot at each reference.
+ */
+ if (attr->fd_array_cnt)
+ return process_fd_array_continuous(env, fd_array,
+ attr->fd_array_cnt);
+ env->fd_array_raw = fd_array;
return 0;
}
@@ -20036,6 +20156,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
mutex_unlock(&bpf_verifier_lock);
bpf_clear_insn_aux_data(env, 0, env->prog->len);
err_free_env:
+ kvfree(env->fd_array);
bpf_stack_liveness_free(env);
kvfree(env->cfg.insn_postorder);
kvfree(env->scc_info);
--
2.43.0
^ permalink raw reply related
* [PATCH bpf-next v5 0/8] Verify BPF signed loader at load time
From: Daniel Borkmann @ 2026-07-07 19:30 UTC (permalink / raw)
To: ast
Cc: kpsingh, James.Bottomley, paul, bboscaccy, memxor, torvalds,
a.s.protopopov, bpf, linux-security-module
The BPF signing scheme signs a light skeleton's loader program and lets
the loader vouch for everything else: bpftool bakes the SHA256 of the
metadata map into the loader's instructions, signs the instructions, and
the loader compares the (frozen, exclusive) map against that hash from
within BPF once it runs. The construction is sound as a trusted hash
chain, but the kernel itself never attests the metadata, and that split
has been the recurring objection from the LSM / integrity side since the
scheme was proposed.
This proposal closes both gaps by having the kernel verify the metadata
at BPF_PROG_LOAD time, before the LSM admission hook and before the
verifier, /without/ growing the UAPI. A signed loader binds its metadata
map(s) through the existing fd_array/fd_array_cnt, and exclusive maps
are already bound to the loader's digest via excl_prog_hash. When a
signature is present, the kernel collects the exclusive maps from the
fd_array and appends their frozen contents to the instructions before
PKCS#7 verification, so the signature covers ...
insns || metadata_0 || metadata_1 || [...]
... in fd_array order. The in-loader hash check is dropped from the
gen_loader entirely: generated loaders carry no verification logic
anymore, and signing or verifying a skeleton becomes an ordinary CMS
operation over bytes that sit verbatim in the skeleton, reproducible
offline. A signed program is either BPF_SIG_UNSIGNED or BPF_SIG_VERIFIED
with nothing in between.
There is no new UAPI, we now have a single signature scheme, no LSM
code reaching into BPF internals, no new LSM hook, and unsigned loads
are completely unaffected. It is also less complex since the loader
does not need to deal with BTF, an extra kfunc, etc, as proposed in
an earlier series [0]. Tested against full BPF CI which came back
green. For more details and examples, see the documentation patch in
this series.
[0] https://lore.kernel.org/bpf/20260522023234.3778588-1-kpsingh@kernel.org/
v4 -> v5:
- Squashed patch 2/9 and 3/9 together and dropped Nack (Paul)
- Added Anton's Acked-by on the fd_array cache patch
- Add !attr->signature_size test to reject invalid param
and added BPF selftest for this case
v3 -> v4:
- Fix upper limit in MAX_FD_ARRAY_CNT (Anton)
- Reject !fd_array && attr->fd_array_cnt (Anton)
- Add bpftool patch wrt ignored return value of EVP_Digest() (sashiko)
- Fix setting of gen_loader_fixture_init (sashiko)
- Fix unused map_fd cleanup branch in selftest (bot+bpf-ci)
- Remove now unused map->excl member and adjust selftests
- Added more BPF signed_loader corner case selftest coverage
- Added Paul's Nack wrt bpf_prog_load LSM hook dispute
- Added patch 2 to move bigger allocations below fd_array
resolution (Paul)
v2 -> v3:
- Added first commit to cache and work on objects in fd_array
which was the most recent issue sashiko rightfully complained
- Added more BPF signed_loader selftest coverage to cover that
usage of sparse fd_array or map fds gets rejected
- I left the security_bpf_prog_load as in v2 given preference
from BPF side over adding new hook
v1 -> v2:
- Addressed both sashiko complaints, the TOCTOU bug regarding
fd_array processing, as well as exclusive map checking to
only allow array maps. The validation is now moved into the
verifier before the main verification work happens. This also
gives the opportunity to utilize the verifier log.
Daniel Borkmann (8):
bpf: Resolve and cache fd_array objects at load time
bpf: Verify signed loader metadata at load time
libbpf: Drop in-loader metadata check for load-time verification
bpftool: Check EVP_Digest when computing excl_prog_hash
bpftool: Cover loader metadata with the program signature
selftests/bpf: Adjust bpf_map layout in verifier_map_ptr
selftests/bpf: Verify load-time signed loader metadata
Documentation/bpf: Add BPF signing and enforcement doc
Documentation/bpf/index.rst | 1 +
Documentation/bpf/signing.rst | 497 ++++++++
include/linux/bpf.h | 1 -
include/linux/bpf_verifier.h | 23 +-
kernel/bpf/syscall.c | 83 +-
kernel/bpf/verifier.c | 451 ++++++--
tools/bpf/bpftool/gen.c | 2 +
tools/bpf/bpftool/sign.c | 24 +-
tools/lib/bpf/bpf_gen_internal.h | 1 -
tools/lib/bpf/gen_loader.c | 76 +-
tools/lib/bpf/libbpf_internal.h | 1 -
tools/lib/bpf/skel_internal.h | 31 +-
.../selftests/bpf/prog_tests/signed_loader.c | 1025 ++++++++++++++---
.../selftests/bpf/progs/test_signed_loader.c | 9 +-
.../selftests/bpf/progs/verifier_map_ptr.c | 23 +-
15 files changed, 1809 insertions(+), 439 deletions(-)
create mode 100644 Documentation/bpf/signing.rst
--
2.43.0
^ permalink raw reply
* Re: [PATCH v2] selftests/lsm: Fix memory leak in attr_lsm_count
From: Paul Moore @ 2026-07-07 19:13 UTC (permalink / raw)
To: Wang Yan, bill.c.roberts
Cc: casey, jmorris, linux-kernel, linux-kselftest,
linux-security-module, mic, serge, shuah, wangyan01
In-Reply-To: <20260703120951.1622491-1-wangyan01@kylinos.cn>
On Jul 3, 2026 Wang Yan <wangyan01@kylinos.cn> wrote:
>
> The calloc-allocated buffer in attr_lsm_count() is never released on
> any exit path, including both the normal return path and the early
> return when read_sysfs_lsms fails, resulting in a heap memory leak.
>
> Add free() for the buffer on all return branches to fix the leak.
>
> Fixes: d3d929a8b0cd ("LSM: selftests for Linux Security Module syscalls")
> Signed-off-by: Wang Yan <wangyan01@kylinos.cn>
> Reviewed-by: William Roberts <bill.c.roberts@gmail.com>
> Tested-by: William Roberts <bill.c.roberts@gmail.com>
> ---
> tools/testing/selftests/lsm/common.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Merged into lsm/dev, thanks!
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v2] NFSv4.2: fix nfs4_listxattr size accounting
From: Anna Schumaker @ 2026-07-07 19:11 UTC (permalink / raw)
To: Paul Moore, Achilles Gaikwad
Cc: Trond Myklebust, stephen.smalley.work, linux-nfs,
linux-security-module, selinux
In-Reply-To: <CAHC9VhSWWhMjs282cOTT45gn0pa8bDSxD0H24_is7k4tXmGJxQ@mail.gmail.com>
On Tue, Jul 7, 2026, at 2:48 PM, Paul Moore wrote:
> On Tue, Jul 7, 2026 at 11:24 AM Achilles Gaikwad
> <achillesgaikwad@gmail.com> wrote:
>>
>> A call to listxattr() with a buffer size of 0 returns the actual
>> size of the buffer needed for a subsequent call. On an NFSv4.2
>> mount this triggers the following oops:
>>
>> [ 399.768687] BUG: kernel NULL pointer dereference, address: 0000000000000000
>> [ 399.768705] RIP: 0010:_copy_from_pages+0x44/0xe0
>> [ 399.768722] Call Trace:
>> [ 399.768723] nfs4_xattr_alloc_entry+0x1bf/0x1e0
>> [ 399.768730] nfs4_xattr_cache_set_list+0x43/0x1f0
>> [ 399.768731] nfs4_listxattr+0x21f/0x250
>> [ 399.768733] vfs_listxattr+0x55/0xa0
>> [ 399.768736] listxattr+0x23/0x160
>> [ 399.768737] path_listxattrat+0xba/0x1e0
>> [ 399.768739] do_syscall_64+0xe2/0x680
>>
>> security_inode_listsecurity() (via the xattr_list_one() helper) now
>> decrements the remaining size even when the buffer pointer is NULL, so
>> in the size-query case, 'left' underflows to a huge size_t value. As a
>> result, nfs4_listxattr_nfs4_user() treats the NULL buffer as a real one,
>> leading to a NULL pointer dereference in _copy_from_pages().
>>
>> security_inode_listsecurity() does not return the number of bytes
>> it added to the list, so the code derived it as
>> 'size - error - left'. That is also wrong in the size-query case:
>> the generic_listxattr() contribution is only subtracted from 'left'
>> when a buffer is present. Thus, the query result comes up short by
>> exactly that contribution (e.g., "system.nfs4_acl" on a mount with
>> ACL support), and a caller that allocates the returned size gets
>> -ERANGE on the subsequent call.
>>
>> Declare 'left' as ssize_t, use a scratch copy to measure security
>> hook consumption, and only decrement 'left' if a buffer is present.
>>
>> Fixes: f71ece9712b7 ("security,fs,nfs,net: update security_inode_listsecurity() interface")
>> Suggested-by: Paul Moore <paul@paul-moore.com>
>> Signed-off-by: Achilles Gaikwad <achillesgaikwad@gmail.com>
>> ---
>> Changes in v2:
>> - Use a scratch variable to track security label size directly,
>> replacing the old formula that undercounted the size-query case.
>> - Drop the now-unneeded NULL-buffer special case for
>> nfs4_listxattr_nfs4_user().
>> - Retitled from "fix nfs4_listxattr NULL pointer dereference"
>> (the same accounting bug caused both the oops and the undercount).
>> v1: https://lore.kernel.org/linux-nfs/20260703102759.9626-1-achillesgaikwad@gmail.com/
>> fs/nfs/nfs4proc.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> [CC'd the LSM and SELinux lists for visibility]
>
> Unfortunately my testing was unsuccessful due to an NFS problem that
> started with the v7.2 merge window that I haven't had the time to
> bisect yet. Assuming the NFS folks are okay with this change, I
> figure they will want to send it up to Linus via their tree, if not
> let me know and I can send this up via the LSM tree.
Yeah, we'll send it through the NFS tree. I'll be curious to hear
what problem you're hitting, and what patch is the culprit once you
do that bisect!
Anna
>
> Reviewed-by: Paul Moore <paul@paul-moore.com>
>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 1360409d8de9..a3415082d610 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -10585,7 +10585,8 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
>> static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size)
>> {
>> ssize_t error, error2, error3;
>> - size_t left = size;
>> + ssize_t left = size;
>> + ssize_t left2;
>>
>> error = generic_listxattr(dentry, list, left);
>> if (error < 0)
>> @@ -10595,10 +10596,13 @@ static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size)
>> left -= error;
>> }
>>
>> - error2 = security_inode_listsecurity(d_inode(dentry), &list, &left);
>> + left2 = left;
>> + error2 = security_inode_listsecurity(d_inode(dentry), &list, &left2);
>> if (error2 < 0)
>> return error2;
>> - error2 = size - error - left;
>> + error2 = left - left2;
>> + if (list)
>> + left -= error2;
>>
>> error3 = nfs4_listxattr_nfs4_user(d_inode(dentry), list, left);
>> if (error3 < 0)
>>
>> base-commit: 6eb8711ece2ce27e52e327a5b7a628ed39b97f45
>> --
>> 2.55.0
>
> --
> paul-moore.com
^ permalink raw reply
* Re: [PATCH v5 2/2] cred: delete task_euid()
From: Paul Moore @ 2026-07-07 19:10 UTC (permalink / raw)
To: Alice Ryhl, Serge Hallyn, Jonathan Corbet, Greg Kroah-Hartman,
Shuah Khan, Alex Shi, Yanteng Si, Dongliang Mu
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Jann Horn, linux-security-module, linux-doc, linux-kernel,
rust-for-linux, Alice Ryhl
In-Reply-To: <20260703-remove-task-euid-v5-2-c90c7e2ddf54@google.com>
On Jul 3, 2026 Alice Ryhl <aliceryhl@google.com> wrote:
>
> task_euid() is a very weird operation. You can see how weird it is by
> grepping for task_euid() - binder is its only user. task_euid() obtains
> the objective effective UID - it looks at the credentials of the task
> for purposes of acting on it as an object, but then accesses the
> effective UID (which the credentials.7 man page describes as "[...] used
> by the kernel to determine the permissions that the process will have
> when accessing shared resources [...]").
>
> Since usage in Binder has now been removed, get rid of the resulting
> dead code.
>
> Changes to the zh_CN translation was carried out with the help of
> Gemini and Google Translate, and since adjusted as per Alex Shi's
> feedback.
>
> Suggested-by: Jann Horn <jannh@google.com>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Documentation/security/credentials.rst | 6 ++----
> Documentation/translations/zh_CN/security/credentials.rst | 4 +---
> include/linux/cred.h | 1 -
> rust/helpers/task.c | 5 -----
> rust/kernel/task.rs | 10 ----------
> 5 files changed, 3 insertions(+), 23 deletions(-)
Merged into lsm/dev, thanks!
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v5 1/2] rust: task: clarify comments on task UID accessors
From: Paul Moore @ 2026-07-07 19:10 UTC (permalink / raw)
To: Alice Ryhl, Serge Hallyn, Jonathan Corbet, Greg Kroah-Hartman,
Shuah Khan, Alex Shi, Yanteng Si, Dongliang Mu
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Jann Horn, linux-security-module, linux-doc, linux-kernel,
rust-for-linux, Alice Ryhl
In-Reply-To: <20260703-remove-task-euid-v5-1-c90c7e2ddf54@google.com>
On Jul 3, 2026 Alice Ryhl <aliceryhl@google.com> wrote:
>
> Linux has separate subjective and objective task credentials, see the
> comment above `struct cred`. Clarify which accessor functions operate on
> which set of credentials.
>
> Also document that Task::euid() is a very weird operation. You can see how
> weird it is by grepping for task_euid() in the history - binder was its
> only user. Task::euid() obtains the objective effective UID - it looks
> at the credentials of the task for purposes of acting on it as an
> object, but then accesses the effective UID (which the credentials.7 man
> page describes as "[...] used by the kernel to determine the permissions
> that the process will have when accessing shared resources [...]").
>
> For context:
> Arguably, binder's use of task_euid() is a theoretical security problem,
> which only has no impact on Android because Android has no setuid binaries
> executable by apps.
> commit 29bc22ac5e5b ("binder: use euid from cred instead of using task")
> originally fixed that by removing that only user of task_euid(), but the
> fix got reverted in commit c21a80ca0684 ("binder: fix test regression
> due to sender_euid change") because some Android test started failing.
> It was since fixed again by commit 65b672152289 ("binder: use
> current_euid() for transaction sender identity"), which uses
> current_euid() instead.
>
> Signed-off-by: Jann Horn <jannh@google.com>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Originally sent as:
> https://lore.kernel.org/r/20260212-rust-uid-v1-1-deff4214c766@google.com
> ---
> rust/kernel/task.rs | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
Merged into lsm/dev, thanks!
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v2] NFSv4.2: fix nfs4_listxattr size accounting
From: Paul Moore @ 2026-07-07 18:48 UTC (permalink / raw)
To: Achilles Gaikwad
Cc: trondmy, anna, stephen.smalley.work, linux-nfs,
linux-security-module, selinux
In-Reply-To: <20260707152305.15324-1-achillesgaikwad@gmail.com>
On Tue, Jul 7, 2026 at 11:24 AM Achilles Gaikwad
<achillesgaikwad@gmail.com> wrote:
>
> A call to listxattr() with a buffer size of 0 returns the actual
> size of the buffer needed for a subsequent call. On an NFSv4.2
> mount this triggers the following oops:
>
> [ 399.768687] BUG: kernel NULL pointer dereference, address: 0000000000000000
> [ 399.768705] RIP: 0010:_copy_from_pages+0x44/0xe0
> [ 399.768722] Call Trace:
> [ 399.768723] nfs4_xattr_alloc_entry+0x1bf/0x1e0
> [ 399.768730] nfs4_xattr_cache_set_list+0x43/0x1f0
> [ 399.768731] nfs4_listxattr+0x21f/0x250
> [ 399.768733] vfs_listxattr+0x55/0xa0
> [ 399.768736] listxattr+0x23/0x160
> [ 399.768737] path_listxattrat+0xba/0x1e0
> [ 399.768739] do_syscall_64+0xe2/0x680
>
> security_inode_listsecurity() (via the xattr_list_one() helper) now
> decrements the remaining size even when the buffer pointer is NULL, so
> in the size-query case, 'left' underflows to a huge size_t value. As a
> result, nfs4_listxattr_nfs4_user() treats the NULL buffer as a real one,
> leading to a NULL pointer dereference in _copy_from_pages().
>
> security_inode_listsecurity() does not return the number of bytes
> it added to the list, so the code derived it as
> 'size - error - left'. That is also wrong in the size-query case:
> the generic_listxattr() contribution is only subtracted from 'left'
> when a buffer is present. Thus, the query result comes up short by
> exactly that contribution (e.g., "system.nfs4_acl" on a mount with
> ACL support), and a caller that allocates the returned size gets
> -ERANGE on the subsequent call.
>
> Declare 'left' as ssize_t, use a scratch copy to measure security
> hook consumption, and only decrement 'left' if a buffer is present.
>
> Fixes: f71ece9712b7 ("security,fs,nfs,net: update security_inode_listsecurity() interface")
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Achilles Gaikwad <achillesgaikwad@gmail.com>
> ---
> Changes in v2:
> - Use a scratch variable to track security label size directly,
> replacing the old formula that undercounted the size-query case.
> - Drop the now-unneeded NULL-buffer special case for
> nfs4_listxattr_nfs4_user().
> - Retitled from "fix nfs4_listxattr NULL pointer dereference"
> (the same accounting bug caused both the oops and the undercount).
> v1: https://lore.kernel.org/linux-nfs/20260703102759.9626-1-achillesgaikwad@gmail.com/
> fs/nfs/nfs4proc.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
[CC'd the LSM and SELinux lists for visibility]
Unfortunately my testing was unsuccessful due to an NFS problem that
started with the v7.2 merge window that I haven't had the time to
bisect yet. Assuming the NFS folks are okay with this change, I
figure they will want to send it up to Linus via their tree, if not
let me know and I can send this up via the LSM tree.
Reviewed-by: Paul Moore <paul@paul-moore.com>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 1360409d8de9..a3415082d610 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -10585,7 +10585,8 @@ const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
> static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size)
> {
> ssize_t error, error2, error3;
> - size_t left = size;
> + ssize_t left = size;
> + ssize_t left2;
>
> error = generic_listxattr(dentry, list, left);
> if (error < 0)
> @@ -10595,10 +10596,13 @@ static ssize_t nfs4_listxattr(struct dentry *dentry, char *list, size_t size)
> left -= error;
> }
>
> - error2 = security_inode_listsecurity(d_inode(dentry), &list, &left);
> + left2 = left;
> + error2 = security_inode_listsecurity(d_inode(dentry), &list, &left2);
> if (error2 < 0)
> return error2;
> - error2 = size - error - left;
> + error2 = left - left2;
> + if (list)
> + left -= error2;
>
> error3 = nfs4_listxattr_nfs4_user(d_inode(dentry), list, left);
> if (error3 < 0)
>
> base-commit: 6eb8711ece2ce27e52e327a5b7a628ed39b97f45
> --
> 2.55.0
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] Documentation: landlock: Document fs.resolve_unix audit blocker
From: Mickaël Salaün @ 2026-07-07 13:33 UTC (permalink / raw)
To: Doehyun Baek
Cc: Jonathan Corbet, Shuah Khan, Sebastian Andrzej Siewior,
linux-security-module, linux-doc, linux-kernel,
Günther Noack
In-Reply-To: <CAN-j9UrCAkNgzj+xG8bRaOnducE16_O909Msi3EFVZjEJf9oAw@mail.gmail.com>
It's applied, thanks!
On Tue, Jul 07, 2026 at 11:40:51AM +0200, Doehyun Baek wrote:
> Hi Mickaël,
>
> Gentle ping on this documentation fix.
>
> Günther reviewed it:
>
> Reviewed-by: Günther Noack <gnoack@google.com>
>
> The patch still applies cleanly. Could this be picked up via the
> Landlock tree?
>
> Thanks,
> Doehyun
^ permalink raw reply
* Re: [PATCH v19 3/8] rust: implement `ForeignOwnable` for `Owned`
From: Alice Ryhl @ 2026-07-07 13:04 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Danilo Krummrich, Lorenzo Stoakes, Vlastimil Babka,
Liam R. Howlett, Uladzislau Rezki, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Trevor Gross,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, Lyude Paul, Greg Kroah-Hartman,
Arve Hjønnevåg, Todd Kjos, Christian Brauner,
Carlos Llamas, Rafael J. Wysocki, Dave Ertman, Ira Weiny,
Leon Romanovsky, Paul Moore, Serge Hallyn, David Airlie,
Simona Vetter, Alexander Viro, Jan Kara, Igor Korotin,
Viresh Kumar, Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
Krzysztof Wilczyński, Pavel Tikhomirov, Michal Wilczynski,
Philipp Stanner, rust-for-linux, linux-kernel, linux-mm,
driver-core, linux-block, linux-security-module, dri-devel,
linux-fsdevel, linux-pm, linux-pci, linux-pwm
In-Reply-To: <20260626-unique-ref-v19-3-2607ca88dfdf@kernel.org>
On Fri, Jun 26, 2026 at 1:55 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Implement `ForeignOwnable` for `Owned<T>`. This allows use of `Owned<T>` in
> places such as the `XArray`.
>
> Note that `T` does not need to implement `ForeignOwnable` for `Owned<T>` to
> implement `ForeignOwnable`.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
^ permalink raw reply
* Re: [PATCH v19 4/8] rust: page: convert to `Ownable`
From: Alice Ryhl @ 2026-07-07 13:04 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Danilo Krummrich, Lorenzo Stoakes, Vlastimil Babka,
Liam R. Howlett, Uladzislau Rezki, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Trevor Gross,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, Lyude Paul, Greg Kroah-Hartman,
Arve Hjønnevåg, Todd Kjos, Christian Brauner,
Carlos Llamas, Rafael J. Wysocki, Dave Ertman, Ira Weiny,
Leon Romanovsky, Paul Moore, Serge Hallyn, David Airlie,
Simona Vetter, Alexander Viro, Jan Kara, Igor Korotin,
Viresh Kumar, Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
Krzysztof Wilczyński, Pavel Tikhomirov, Michal Wilczynski,
Philipp Stanner, rust-for-linux, linux-kernel, linux-mm,
driver-core, linux-block, linux-security-module, dri-devel,
linux-fsdevel, linux-pm, linux-pci, linux-pwm, Asahi Lina
In-Reply-To: <20260626-unique-ref-v19-4-2607ca88dfdf@kernel.org>
On Fri, Jun 26, 2026 at 1:56 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> From: Asahi Lina <lina@asahilina.net>
>
> This allows Page references to be returned as borrowed references,
> without necessarily owning the struct page.
>
> Remove `BorrowedPage` and update users to use `Owned<Page>`.
>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> [ Andreas: Fix formatting and add a safety comment, update users. ]
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> /// Returns a raw pointer to the page.
> pub fn as_ptr(&self) -> *mut bindings::page {
> - self.page.as_ptr()
> + Opaque::cast_into(&self.page)
cast_into() is mainly used when you have a raw pointer to opaque. When
you have a reference, you can just do self.page.get()
Alice
^ permalink raw reply
* Re: [PATCH] Documentation: landlock: Document fs.resolve_unix audit blocker
From: Doehyun Baek @ 2026-07-07 9:40 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Jonathan Corbet, Shuah Khan, Sebastian Andrzej Siewior,
linux-security-module, linux-doc, linux-kernel,
Günther Noack
In-Reply-To: <aj0fldJZ2dl0gas1@google.com>
Hi Mickaël,
Gentle ping on this documentation fix.
Günther reviewed it:
Reviewed-by: Günther Noack <gnoack@google.com>
The patch still applies cleanly. Could this be picked up via the
Landlock tree?
Thanks,
Doehyun
^ permalink raw reply
* Re: [PATCH v3 4/5] xfs: replace ns_capable_noaudit
From: Carlos Maiolino @ 2026-07-07 9:26 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Jan Kara, Christoph Hellwig, Serge E. Hallyn, Dave Chinner,
Eric Sandeen, linux-xfs, linux-fsdevel, linux-security-module,
linux-kernel
In-Reply-To: <20260702155856.GF9392@frogsfrogsfrogs>
On Thu, Jul 02, 2026 at 08:58:56AM -0700, Darrick J. Wong wrote:
> On Thu, Jul 02, 2026 at 11:33:23AM +0200, cem@kernel.org wrote:
> > From: Carlos Maiolino <cem@kernel.org>
> >
> > Now that capable_noaudit() is available, we don't need to keep
> > using ns_capable_noaudit() and specifying the usernaspace every single
> > time.
> >
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > Cc: Jan Kara <jack@suse.cz>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Serge E. Hallyn <serge@hallyn.com>
> > Cc: Darrick J. Wong <djwong@kernel.org>
> > Cc: Dave Chinner <david@fromorbit.com>
> > Cc: Eric Sandeen <sandeen@redhat.com>
> > Cc: Dr. Thomas Orgis" <thomas.orgis@uni-hamburg.de>
> > Cc: linux-xfs@vger.kernel.org
> > Cc: linux-fsdevel@vger.kernel.org
> > Cc: linux-security-module@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > ---
> > fs/xfs/xfs_fsmap.c | 3 +--
> > fs/xfs/xfs_ioctl.c | 2 +-
> > fs/xfs/xfs_iops.c | 2 +-
> > 3 files changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c
> > index 7c79fbe0a74c..041bb2105ec6 100644
> > --- a/fs/xfs/xfs_fsmap.c
> > +++ b/fs/xfs/xfs_fsmap.c
> > @@ -1174,8 +1174,7 @@ xfs_getfsmap(
> > if (!xfs_getfsmap_check_keys(&head->fmh_keys[0], &head->fmh_keys[1]))
> > return -EINVAL;
> >
> > - use_rmap = xfs_has_rmapbt(mp) &&
> > - ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN);
> > + use_rmap = xfs_has_rmapbt(mp) && capable_noaudit(CAP_SYS_ADMIN);
> > head->fmh_entries = 0;
> >
> > /* Set up our device handlers. */
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index 1a8af827dde1..374b488f0416 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -647,7 +647,7 @@ xfs_ioctl_setattr_get_trans(
> > goto out_error;
> >
> > error = xfs_trans_alloc_ichange(ip, NULL, NULL, pdqp,
> > - ns_capable_noaudit(&init_user_ns, CAP_FOWNER), &tp);
> > + capable_noaudit(CAP_FOWNER), &tp);
>
> Not sure why the indentation changed, otherwise the patch looks fine to
> me.
Purely my screw up. I fixed it already to send to the next version. Was
just waiting for more comments
>
> --D
>
> > if (error)
> > goto out_error;
> >
> > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> > index 205fe2dae732..ce9f8b8468fc 100644
> > --- a/fs/xfs/xfs_iops.c
> > +++ b/fs/xfs/xfs_iops.c
> > @@ -835,7 +835,7 @@ xfs_setattr_nonsize(
> > }
> >
> > error = xfs_trans_alloc_ichange(ip, udqp, gdqp, NULL,
> > - ns_capable_noaudit(&init_user_ns, CAP_FOWNER),
> > + capable_noaudit(CAP_FOWNER),
> > &tp);
> > if (error)
> > goto out_dqrele;
> > --
> > 2.54.0
> >
> >
>
^ permalink raw reply
* Re: [PATCH v3 2/5] capability: Add new capable_noaudit
From: Carlos Maiolino @ 2026-07-07 9:25 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Jan Kara, Dave Chinner, Eric Sandeen, linux-xfs, linux-fsdevel,
linux-security-module, linux-kernel, Christoph Hellwig,
Serge Hallyn
In-Reply-To: <20260702155605.GE9392@frogsfrogsfrogs>
On Thu, Jul 02, 2026 at 08:56:05AM -0700, Darrick J. Wong wrote:
> On Thu, Jul 02, 2026 at 11:33:19AM +0200, cem@kernel.org wrote:
> > From: Carlos Maiolino <cem@kernel.org>
> >
> > In some situations (quota enforcement bypass in this case) we'd like to
> > check for a specific capability without triggering spurious audit
> > messages from security modules like selinux.
> >
> > Add a new helper so we don't need to use ns_capable_noaudit() directly.
> >
> > V3: remove the extern declaration
> >
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > Cc: Jan Kara <jack@suse.cz>
> > Cc: Darrick J. Wong <djwong@kernel.org>
> > Cc: Dave Chinner <david@fromorbit.com>
> > Cc: Eric Sandeen <sandeen@redhat.com>
> > Cc: Dr. Thomas Orgis" <thomas.orgis@uni-hamburg.de>
> > Cc: linux-xfs@vger.kernel.org
> > Cc: linux-fsdevel@vger.kernel.org
> > Cc: linux-security-module@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > Reviewed-by: Serge Hallyn <serge@hallyn.com>
> > ---
> > include/linux/capability.h | 5 +++++
> > kernel/capability.c | 17 +++++++++++++++++
> > 2 files changed, 22 insertions(+)
> >
> > diff --git a/include/linux/capability.h b/include/linux/capability.h
> > index 37db92b3d6f8..f8532d92fcad 100644
> > --- a/include/linux/capability.h
> > +++ b/include/linux/capability.h
> > @@ -145,6 +145,7 @@ extern bool has_capability_noaudit(struct task_struct *t, int cap);
> > extern bool has_ns_capability_noaudit(struct task_struct *t,
> > struct user_namespace *ns, int cap);
> > extern bool capable(int cap);
> > +bool capable_noaudit(int cap);
> > extern bool ns_capable(struct user_namespace *ns, int cap);
> > extern bool ns_capable_noaudit(struct user_namespace *ns, int cap);
> > extern bool ns_capable_setid(struct user_namespace *ns, int cap);
> > @@ -167,6 +168,10 @@ static inline bool capable(int cap)
> > {
> > return true;
> > }
> > +static inline bool capable_noaudit(int cap)
> > +{
> > + return true;
> > +}
> > static inline bool ns_capable(struct user_namespace *ns, int cap)
> > {
> > return true;
> > diff --git a/kernel/capability.c b/kernel/capability.c
> > index 829f49ae07b9..2c2d1e8300bd 100644
> > --- a/kernel/capability.c
> > +++ b/kernel/capability.c
> > @@ -416,6 +416,23 @@ bool capable(int cap)
> > return ns_capable(&init_user_ns, cap);
> > }
> > EXPORT_SYMBOL(capable);
> > +
> > +/**
> > + * capable_noaudit - Determine if the current task has a superior
> > + * capability in effect (unaudited).
> > + * @cap: The capability to be tested for
> > + *
> > + * This is the same as capable(), except it uses CAP_OPT_NOAUDIT as to prevent
> > + * issuing spurious audit messages.
> > + *
> > + * This sets PF_SUPERPRIV on the task if the capability is available on the
> > + * assumption that it's about to be used.
>
> Same complaint about the documentation as last time:
> https://lore.kernel.org/linux-fsdevel/20260626151656.GT6078@frogsfrogsfrogs/
Right. I assumed "capability in effect" would suffice that, but I can
add some extra comment here.
>
> --D
>
> > + */
> > +bool capable_noaudit(int cap)
> > +{
> > + return ns_capable_noaudit(&init_user_ns, cap);
> > +}
> > +EXPORT_SYMBOL(capable_noaudit);
> > #endif /* CONFIG_MULTIUSER */
> >
> > /**
> > --
> > 2.54.0
> >
> >
>
^ permalink raw reply
* [PATCH v2 -next 2/2] security: Fix call security_backing_file_free second time
From: Cai Xinchen @ 2026-07-07 8:06 UTC (permalink / raw)
To: paul, jmorris, serge, amir73il, brauner, caixinchen1
Cc: linux-security-module, linux-kernel, lujialin4
In-Reply-To: <20260707080615.1235830-1-caixinchen1@huawei.com>
I found the following path:
alloc_empty_backing-file
init_file(&ff->file, xxx)
-> file_ref_init(&f->f_ref, 1); // only 1
error = init_backing_file
-> security_backing_file_alloc
-> rc = call_int_hook(backing_file_alloc, ...)
if (unlikely(rc))
security_backing_file_free(backing_file); // first call
if (unlikely(error)) {
fput(&ff->file);
-> if (unlikely(file_ref_put(&file->f_ref))) // zero
__fput_deferred(file);
-> ____fput -> __fput -> file_free(file);
-> backing_file_free(backing_file(f));
-> security_backing_file_free(&ff->file); // second call
Currently, only SELinux has the lsm backing_file_alloc hook, and it always
return 0. When security_backing_file_free is called for the first time,
the blobs pointer is set to NULL. Therefore, double free will not occur in
the code.
Fixes: 6af36aeb147a ("lsm: add backing_file LSM hooks")
Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
security/security.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/security/security.c b/security/security.c
index 71aea8fdf014..bec2f4ebea34 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2486,9 +2486,8 @@ void security_backing_file_free(struct file *backing_file)
{
void *blob = backing_file_security(backing_file);
- call_void_hook(backing_file_free, backing_file);
-
if (blob) {
+ call_void_hook(backing_file_free, backing_file);
backing_file_set_security(backing_file, NULL);
kmem_cache_free(lsm_backing_file_cache, blob);
}
--
2.34.1
^ permalink raw reply related
* [PATCH v2 -next 0/2] security: Fix call security_backing_file_free second time
From: Cai Xinchen @ 2026-07-07 8:06 UTC (permalink / raw)
To: paul, jmorris, serge, amir73il, brauner, caixinchen1
Cc: linux-security-module, linux-kernel, lujialin4
v2: Move the call_void_hook(backing_file_free, ...) call in
security_backing_file_free() into the if-statment true block before we
set the backing file's LSM blob pointer to NULL and free the LSM blob.
I found the following path:
alloc_empty_backing-file
init_file(&ff->file, xxx)
-> file_ref_init(&f->f_ref, 1); // only 1
error = init_backing_file
-> security_backing_file_alloc
-> rc = call_int_hook(backing_file_alloc, ...)
if (unlikely(rc))
security_backing_file_free(backing_file); // first call
if (unlikely(error)) {
fput(&ff->file);
-> if (unlikely(file_ref_put(&file->f_ref))) // zero
__fput_deferred(file);
-> ____fput -> __fput -> file_free(file);
-> backing_file_free(backing_file(f));
-> security_backing_file_free(&ff->file); // second call
Currently, only SELinux has the lsm backing_file_alloc hook, and it always
return 0. When security_backing_file_free is called for the first time,
the blobs pointer is set to NULL. Therefore, double free will not occur in
the code.
Cai Xinchen (2):
security: Delete dumplicate assignment
security: Fix call security_backing_file_free second time
security/lsm_init.c | 1 -
security/security.c | 3 +--
2 files changed, 1 insertion(+), 3 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH v2 -next 1/2] security: Delete dumplicate assignment
From: Cai Xinchen @ 2026-07-07 8:06 UTC (permalink / raw)
To: paul, jmorris, serge, amir73il, brauner, caixinchen1
Cc: linux-security-module, linux-kernel, lujialin4
In-Reply-To: <20260707080615.1235830-1-caixinchen1@huawei.com>
Delete a blobs variable with duplicate assignment.
Signed-off-by: Cai Xinchen <caixinchen1@huawei.com>
---
security/lsm_init.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/security/lsm_init.c b/security/lsm_init.c
index 7c0fd17f1601..d7384866e3a5 100644
--- a/security/lsm_init.c
+++ b/security/lsm_init.c
@@ -290,7 +290,6 @@ static void __init lsm_prepare(struct lsm_info *lsm)
return;
/* Register the LSM blob sizes. */
- blobs = lsm->blobs;
lsm_blob_size_update(&blobs->lbs_cred, &blob_sizes.lbs_cred);
lsm_blob_size_update(&blobs->lbs_file, &blob_sizes.lbs_file);
lsm_blob_size_update(&blobs->lbs_backing_file,
--
2.34.1
^ permalink raw reply related
* [PATCH v6 8/8] lsm: Remove security_sb_mount and security_move_mount
From: Song Liu @ 2026-07-06 23:50 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260706235053.4104951-1-song@kernel.org>
Now that all LSMs have been converted to granular mount hooks and
fs/namespace.c calls the new hooks, remove the old hooks:
- security_sb_mount(): removed from lsm_hook_defs.h, security.h,
security.c.
- security_move_mount(): removed from lsm_hook_defs.h, security.h,
security.c, and bpf_lsm.c.
Code generated with the assistance of Claude, reviewed by human.
Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com> # for selinux only
Signed-off-by: Song Liu <song@kernel.org>
---
include/linux/lsm_hook_defs.h | 4 ----
include/linux/security.h | 16 ---------------
kernel/bpf/bpf_lsm.c | 2 --
security/security.c | 38 -----------------------------------
4 files changed, 60 deletions(-)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 06c904cb478e..4015f6b579c3 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -69,8 +69,6 @@ LSM_HOOK(int, 0, sb_remount, struct super_block *sb, void *mnt_opts)
LSM_HOOK(int, 0, sb_kern_mount, const struct super_block *sb)
LSM_HOOK(int, 0, sb_show_options, struct seq_file *m, struct super_block *sb)
LSM_HOOK(int, 0, sb_statfs, struct dentry *dentry)
-LSM_HOOK(int, 0, sb_mount, const char *dev_name, const struct path *path,
- const char *type, unsigned long flags, void *data)
LSM_HOOK(int, 0, sb_umount, struct vfsmount *mnt, int flags)
LSM_HOOK(int, 0, sb_pivotroot, const struct path *old_path,
const struct path *new_path)
@@ -79,8 +77,6 @@ LSM_HOOK(int, 0, sb_set_mnt_opts, struct super_block *sb, void *mnt_opts,
LSM_HOOK(int, 0, sb_clone_mnt_opts, const struct super_block *oldsb,
struct super_block *newsb, unsigned long kern_flags,
unsigned long *set_kern_flags)
-LSM_HOOK(int, 0, move_mount, const struct path *from_path,
- const struct path *to_path)
LSM_HOOK(int, 0, mount_bind, const struct path *from, const struct path *to,
bool recurse)
LSM_HOOK(int, 0, mount_new, struct fs_context *fc, const struct path *mp,
diff --git a/include/linux/security.h b/include/linux/security.h
index 5e207e38c004..ff16831e8064 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -373,8 +373,6 @@ int security_sb_remount(struct super_block *sb, void *mnt_opts);
int security_sb_kern_mount(const struct super_block *sb);
int security_sb_show_options(struct seq_file *m, struct super_block *sb);
int security_sb_statfs(struct dentry *dentry);
-int security_sb_mount(const char *dev_name, const struct path *path,
- const char *type, unsigned long flags, void *data);
int security_sb_umount(struct vfsmount *mnt, int flags);
int security_sb_pivotroot(const struct path *old_path, const struct path *new_path);
int security_sb_set_mnt_opts(struct super_block *sb,
@@ -385,7 +383,6 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
struct super_block *newsb,
unsigned long kern_flags,
unsigned long *set_kern_flags);
-int security_move_mount(const struct path *from_path, const struct path *to_path);
int security_mount_bind(const struct path *from, const struct path *to,
bool recurse);
int security_mount_new(struct fs_context *fc, const struct path *mp,
@@ -826,13 +823,6 @@ static inline int security_sb_statfs(struct dentry *dentry)
return 0;
}
-static inline int security_sb_mount(const char *dev_name, const struct path *path,
- const char *type, unsigned long flags,
- void *data)
-{
- return 0;
-}
-
static inline int security_sb_umount(struct vfsmount *mnt, int flags)
{
return 0;
@@ -860,12 +850,6 @@ static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
return 0;
}
-static inline int security_move_mount(const struct path *from_path,
- const struct path *to_path)
-{
- return 0;
-}
-
static inline int security_mount_bind(const struct path *from,
const struct path *to, bool recurse)
{
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index 4b1ee9c67d6f..9cece66a0c49 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -353,7 +353,6 @@ BTF_ID(func, bpf_lsm_release_secctx)
BTF_ID(func, bpf_lsm_sb_alloc_security)
BTF_ID(func, bpf_lsm_sb_eat_lsm_opts)
BTF_ID(func, bpf_lsm_sb_kern_mount)
-BTF_ID(func, bpf_lsm_sb_mount)
BTF_ID(func, bpf_lsm_sb_remount)
BTF_ID(func, bpf_lsm_sb_set_mnt_opts)
BTF_ID(func, bpf_lsm_sb_show_options)
@@ -385,7 +384,6 @@ BTF_ID(func, bpf_lsm_task_setscheduler)
BTF_ID(func, bpf_lsm_userns_create)
BTF_ID(func, bpf_lsm_bdev_alloc_security)
BTF_ID(func, bpf_lsm_bdev_setintegrity)
-BTF_ID(func, bpf_lsm_move_mount)
BTF_ID(func, bpf_lsm_mount_bind)
BTF_ID(func, bpf_lsm_mount_new)
BTF_ID(func, bpf_lsm_mount_remount)
diff --git a/security/security.c b/security/security.c
index 1ede060db3ca..0456680738f2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1065,29 +1065,6 @@ int security_sb_statfs(struct dentry *dentry)
return call_int_hook(sb_statfs, dentry);
}
-/**
- * security_sb_mount() - Check permission for mounting a filesystem
- * @dev_name: filesystem backing device
- * @path: mount point
- * @type: filesystem type
- * @flags: mount flags
- * @data: filesystem specific data
- *
- * Check permission before an object specified by @dev_name is mounted on the
- * mount point named by @nd. For an ordinary mount, @dev_name identifies a
- * device if the file system type requires a device. For a remount
- * (@flags & MS_REMOUNT), @dev_name is irrelevant. For a loopback/bind mount
- * (@flags & MS_BIND), @dev_name identifies the pathname of the object being
- * mounted.
- *
- * Return: Returns 0 if permission is granted.
- */
-int security_sb_mount(const char *dev_name, const struct path *path,
- const char *type, unsigned long flags, void *data)
-{
- return call_int_hook(sb_mount, dev_name, path, type, flags, data);
-}
-
/**
* security_sb_umount() - Check permission for unmounting a filesystem
* @mnt: mounted filesystem
@@ -1167,21 +1144,6 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
}
EXPORT_SYMBOL(security_sb_clone_mnt_opts);
-/**
- * security_move_mount() - Check permissions for moving a mount
- * @from_path: source mount point
- * @to_path: destination mount point
- *
- * Check permission before a mount is moved.
- *
- * Return: Returns 0 if permission is granted.
- */
-int security_move_mount(const struct path *from_path,
- const struct path *to_path)
-{
- return call_int_hook(move_mount, from_path, to_path);
-}
-
/**
* security_mount_bind() - Check permissions for a bind mount
* @from: source path
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v6 7/8] vfs: Replace security_sb_mount/security_move_mount with granular hooks
From: Song Liu @ 2026-07-06 23:50 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260706235053.4104951-1-song@kernel.org>
Replace the monolithic security_sb_mount() call in path_mount() and
the security_move_mount() call on the mount-move paths with the new
granular mount hooks:
- do_loopback(): call security_mount_bind()
- do_new_mount(): call security_mount_new()
- do_remount(): call security_mount_remount()
- do_reconfigure_mnt(): call security_mount_reconfigure()
- do_change_type(): call security_mount_change_type()
- do_move_mount(): call security_mount_move()
- do_set_group(): call security_mount_move()
The new hooks are called at the individual operation level with
appropriate context (resolved paths, fs_context), rather than at
the top of path_mount() with raw string arguments.
security_mount_bind() and security_mount_move() are called under the
namespace semaphore, after the mountpoint has been resolved and pinned
by LOCK_MOUNT(). This ensures the LSM inspects the mount that will
actually be used, instead of a mountpoint that could be overmounted
between the check and the operation. Placing the move hook in
do_move_mount() and do_set_group() also unifies the old mount(2)
MS_MOVE path (do_move_mount_old()) and the move_mount(2) path
(vfs_move_mount()) on a single call site.
For MOVE_MOUNT_BENEATH, security_mount_move() additionally receives the
mount that ends up on top, so an LSM can tell a plain move (target ==
top) apart from a mount-beneath operation.
Code generated with the assistance of Claude, reviewed by human.
Signed-off-by: Song Liu <song@kernel.org>
---
fs/namespace.c | 77 +++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 61 insertions(+), 16 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 3d5cd5bf3b05..bc9401a2efac 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2893,6 +2893,10 @@ static int do_change_type(const struct path *path, int ms_flags)
if (!type)
return -EINVAL;
+ err = security_mount_change_type(path, ms_flags);
+ if (err)
+ return err;
+
guard(namespace_excl)();
err = may_change_propagation(mnt);
@@ -3003,6 +3007,7 @@ static int do_loopback(const struct path *path, const char *old_name,
{
struct path old_path __free(path_put) = {};
struct mount *mnt = NULL;
+ struct path dest;
int err;
if (!old_name || !*old_name)
@@ -3021,6 +3026,17 @@ static int do_loopback(const struct path *path, const char *old_name,
if (!check_mnt(mp.parent))
return -EINVAL;
+ /*
+ * Check permission against the mountpoint that was actually pinned
+ * under the namespace semaphore, rather than the caller-supplied
+ * @path which may have been overmounted before the lock was taken.
+ */
+ dest.mnt = &mp.parent->mnt;
+ dest.dentry = mp.mp->m_dentry;
+ err = security_mount_bind(&old_path, &dest, recurse);
+ if (err)
+ return err;
+
mnt = __do_loopback(&old_path, recurse, CL_COPY_MNT_NS_FILE);
if (IS_ERR(mnt))
return PTR_ERR(mnt);
@@ -3335,7 +3351,8 @@ static void mnt_warn_timestamp_expiry(const struct path *mountpoint,
* superblock it refers to. This is triggered by specifying MS_REMOUNT|MS_BIND
* to mount(2).
*/
-static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
+static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags,
+ unsigned long flags)
{
struct super_block *sb = path->mnt->mnt_sb;
struct mount *mnt = real_mount(path->mnt);
@@ -3350,6 +3367,10 @@ static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
if (!can_change_locked_flags(mnt, mnt_flags))
return -EPERM;
+ ret = security_mount_reconfigure(path, mnt_flags, flags);
+ if (ret)
+ return ret;
+
/*
* We're only checking whether the superblock is read-only not
* changing it, so only take down_read(&sb->s_umount).
@@ -3373,7 +3394,7 @@ static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
* on it - tough luck.
*/
static int do_remount(const struct path *path, int sb_flags,
- int mnt_flags, void *data)
+ int mnt_flags, void *data, unsigned long flags)
{
int err;
struct super_block *sb = path->mnt->mnt_sb;
@@ -3400,6 +3421,9 @@ static int do_remount(const struct path *path, int sb_flags,
fc->oldapi = true;
err = parse_monolithic_mount_data(fc, data);
+ if (!err)
+ err = security_mount_remount(fc, path, mnt_flags, flags,
+ data);
if (!err) {
down_write(&sb->s_umount);
err = -EPERM;
@@ -3438,6 +3462,16 @@ static int do_set_group(const struct path *from_path, const struct path *to_path
guard(namespace_excl)();
+ /*
+ * Setting a sharing group does not overmount anything, so the
+ * source, target and top mount all refer to @to_path. The check
+ * runs under the namespace semaphore for the same reason as the
+ * move case.
+ */
+ err = security_mount_move(from_path, to_path, to_path);
+ if (err)
+ return err;
+
err = may_change_propagation(from);
if (err)
return err;
@@ -3634,6 +3668,7 @@ static int do_move_mount(const struct path *old_path,
enum mnt_tree_flags_t flags)
{
struct mount *old = real_mount(old_path->mnt);
+ struct path target, top;
int err;
bool beneath = flags & MNT_TREE_BENEATH;
@@ -3647,6 +3682,17 @@ static int do_move_mount(const struct path *old_path,
if (IS_ERR(mp.parent))
return PTR_ERR(mp.parent);
+ /*
+ * The destination that was actually pinned under the namespace
+ * semaphore. For a plain move the source is attached on top of
+ * @target, so @target is also the mount that ends up on top; for
+ * MOVE_MOUNT_BENEATH the source is inserted below the existing top
+ * mount, which is reported separately below.
+ */
+ target.mnt = &mp.parent->mnt;
+ target.dentry = mp.mp->m_dentry;
+ top = target;
+
if (check_mnt(old)) {
/* if the source is in our namespace... */
/* ... it should be detachable from parent */
@@ -3687,8 +3733,14 @@ static int do_move_mount(const struct path *old_path,
err = can_move_mount_beneath(old, over, &mp);
if (err)
return err;
+ top.mnt = &over->mnt;
+ top.dentry = over->mnt.mnt_root;
}
+ err = security_mount_move(old_path, &target, &top);
+ if (err)
+ return err;
+
/*
* Don't move a mount tree containing unbindable mounts to a destination
* mount which is shared.
@@ -3793,7 +3845,7 @@ static int do_new_mount_fc(struct fs_context *fc, const struct path *mountpoint,
*/
static int do_new_mount(const struct path *path, const char *fstype,
int sb_flags, int mnt_flags,
- const char *name, void *data)
+ const char *name, void *data, unsigned long flags)
{
struct file_system_type *type;
struct fs_context *fc;
@@ -3837,6 +3889,9 @@ static int do_new_mount(const struct path *path, const char *fstype,
err = parse_monolithic_mount_data(fc, data);
if (!err && !mount_capable(fc))
err = -EPERM;
+
+ if (!err)
+ err = security_mount_new(fc, path, mnt_flags, flags, data);
if (!err)
err = do_new_mount_fc(fc, path, mnt_flags);
@@ -4087,7 +4142,6 @@ int path_mount(const char *dev_name, const struct path *path,
const char *type_page, unsigned long flags, void *data_page)
{
unsigned int mnt_flags = 0, sb_flags;
- int ret;
/* Discard magic */
if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
@@ -4100,9 +4154,6 @@ int path_mount(const char *dev_name, const struct path *path,
if (flags & MS_NOUSER)
return -EINVAL;
- ret = security_sb_mount(dev_name, path, type_page, flags, data_page);
- if (ret)
- return ret;
if (!may_mount())
return -EPERM;
if (flags & SB_MANDLOCK)
@@ -4148,9 +4199,9 @@ int path_mount(const char *dev_name, const struct path *path,
SB_I_VERSION);
if ((flags & (MS_REMOUNT | MS_BIND)) == (MS_REMOUNT | MS_BIND))
- return do_reconfigure_mnt(path, mnt_flags);
+ return do_reconfigure_mnt(path, mnt_flags, flags);
if (flags & MS_REMOUNT)
- return do_remount(path, sb_flags, mnt_flags, data_page);
+ return do_remount(path, sb_flags, mnt_flags, data_page, flags);
if (flags & MS_BIND)
return do_loopback(path, dev_name, flags & MS_REC);
if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
@@ -4159,7 +4210,7 @@ int path_mount(const char *dev_name, const struct path *path,
return do_move_mount_old(path, dev_name);
return do_new_mount(path, type_page, sb_flags, mnt_flags, dev_name,
- data_page);
+ data_page, flags);
}
int do_mount(const char *dev_name, const char __user *dir_name,
@@ -4554,12 +4605,6 @@ static inline int vfs_move_mount(const struct path *from_path,
const struct path *to_path,
enum mnt_tree_flags_t mflags)
{
- int ret;
-
- ret = security_move_mount(from_path, to_path);
- if (ret)
- return ret;
-
if (mflags & MNT_TREE_PROPAGATION)
return do_set_group(from_path, to_path);
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v6 6/8] tomoyo: Convert from sb_mount to granular mount hooks
From: Song Liu @ 2026-07-06 23:50 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260706235053.4104951-1-song@kernel.org>
Replace tomoyo_sb_mount() with granular mount hooks. Each hook
reconstructs the MS_* flags expected by tomoyo_mount_permission()
using the original flags parameter where available.
Key changes:
- mount_bind: passes the pre-resolved source path to
tomoyo_mount_acl() via a new dev_path parameter, instead of
re-resolving dev_name via kern_path(). This eliminates a TOCTOU
vulnerability.
- mount_new, mount_remount, mount_reconfigure: use the original
mount(2) flags for policy matching.
- mount_move: passes pre-resolved paths for both source and
destination; the new top_path argument is unused.
- mount_change_type: passes raw ms_flags directly.
Also removes the unused data_page parameter from
tomoyo_mount_permission().
Code generated with the assistance of Claude, reviewed by human.
Acked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Song Liu <song@kernel.org>
---
security/tomoyo/common.h | 2 +-
security/tomoyo/mount.c | 31 +++++++----
security/tomoyo/tomoyo.c | 111 +++++++++++++++++++++++++++++++++++----
3 files changed, 123 insertions(+), 21 deletions(-)
diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h
index d098cf8aae61..9241034cfede 100644
--- a/security/tomoyo/common.h
+++ b/security/tomoyo/common.h
@@ -1013,7 +1013,7 @@ int tomoyo_mkdev_perm(const u8 operation, const struct path *path,
const unsigned int mode, unsigned int dev);
int tomoyo_mount_permission(const char *dev_name, const struct path *path,
const char *type, unsigned long flags,
- void *data_page);
+ const struct path *dev_path);
int tomoyo_open_control(const u8 type, struct file *file);
int tomoyo_path2_perm(const u8 operation, const struct path *path1,
const struct path *path2);
diff --git a/security/tomoyo/mount.c b/security/tomoyo/mount.c
index 322dfd188ada..82ffe7d02814 100644
--- a/security/tomoyo/mount.c
+++ b/security/tomoyo/mount.c
@@ -70,6 +70,7 @@ static bool tomoyo_check_mount_acl(struct tomoyo_request_info *r,
* @dir: Pointer to "struct path".
* @type: Name of filesystem type.
* @flags: Mount options.
+ * @dev_path: Pre-resolved device/source path. Maybe NULL.
*
* Returns 0 on success, negative value otherwise.
*
@@ -78,11 +79,11 @@ static bool tomoyo_check_mount_acl(struct tomoyo_request_info *r,
static int tomoyo_mount_acl(struct tomoyo_request_info *r,
const char *dev_name,
const struct path *dir, const char *type,
- unsigned long flags)
+ unsigned long flags,
+ const struct path *dev_path)
__must_hold_shared(&tomoyo_ss)
{
struct tomoyo_obj_info obj = { };
- struct path path;
struct file_system_type *fstype = NULL;
const char *requested_type = NULL;
const char *requested_dir_name = NULL;
@@ -134,13 +135,23 @@ static int tomoyo_mount_acl(struct tomoyo_request_info *r,
need_dev = 1;
}
if (need_dev) {
- /* Get mount point or device file. */
- if (!dev_name || kern_path(dev_name, LOOKUP_FOLLOW, &path)) {
+ if (dev_path) {
+ /* Use pre-resolved path to avoid TOCTOU issues. */
+ obj.path1 = *dev_path;
+ path_get(&obj.path1);
+ } else if (!dev_name) {
error = -ENOENT;
goto out;
+ } else {
+ struct path path;
+
+ if (kern_path(dev_name, LOOKUP_FOLLOW, &path)) {
+ error = -ENOENT;
+ goto out;
+ }
+ obj.path1 = path;
}
- obj.path1 = path;
- requested_dev_name = tomoyo_realpath_from_path(&path);
+ requested_dev_name = tomoyo_realpath_from_path(&obj.path1);
if (!requested_dev_name) {
error = -ENOENT;
goto out;
@@ -173,7 +184,7 @@ static int tomoyo_mount_acl(struct tomoyo_request_info *r,
if (fstype)
put_filesystem(fstype);
kfree(requested_type);
- /* Drop refcount obtained by kern_path(). */
+ /* Drop refcount obtained by kern_path() or path_get(). */
if (obj.path1.dentry)
path_put(&obj.path1);
return error;
@@ -186,13 +197,13 @@ static int tomoyo_mount_acl(struct tomoyo_request_info *r,
* @path: Pointer to "struct path".
* @type: Name of filesystem type. Maybe NULL.
* @flags: Mount options.
- * @data_page: Optional data. Maybe NULL.
+ * @dev_path: Pre-resolved device/source path. Maybe NULL.
*
* Returns 0 on success, negative value otherwise.
*/
int tomoyo_mount_permission(const char *dev_name, const struct path *path,
const char *type, unsigned long flags,
- void *data_page)
+ const struct path *dev_path)
{
struct tomoyo_request_info r;
int error;
@@ -236,7 +247,7 @@ int tomoyo_mount_permission(const char *dev_name, const struct path *path,
if (!type)
type = "<NULL>";
idx = tomoyo_read_lock();
- error = tomoyo_mount_acl(&r, dev_name, path, type, flags);
+ error = tomoyo_mount_acl(&r, dev_name, path, type, flags, dev_path);
tomoyo_read_unlock(idx);
return error;
}
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index c66e02ed8ee3..eec3df35e28e 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -6,6 +6,8 @@
*/
#include <linux/lsm_hooks.h>
+#include <linux/fs_context.h>
+#include <uapi/linux/mount.h>
#include <uapi/linux/lsm.h>
#include "common.h"
@@ -399,20 +401,104 @@ static int tomoyo_path_chroot(const struct path *path)
}
/**
- * tomoyo_sb_mount - Target for security_sb_mount().
+ * tomoyo_mount_bind - Target for security_mount_bind().
*
- * @dev_name: Name of device file. Maybe NULL.
- * @path: Pointer to "struct path".
- * @type: Name of filesystem type. Maybe NULL.
- * @flags: Mount options.
- * @data: Optional data. Maybe NULL.
+ * @from: Pointer to "struct path".
+ * @to: Pointer to "struct path".
+ * @recurse: Whether recursive bind mount or not.
*
* Returns 0 on success, negative value otherwise.
*/
-static int tomoyo_sb_mount(const char *dev_name, const struct path *path,
- const char *type, unsigned long flags, void *data)
+static int tomoyo_mount_bind(const struct path *from, const struct path *to,
+ bool recurse)
{
- return tomoyo_mount_permission(dev_name, path, type, flags, data);
+ unsigned long flags = MS_BIND | (recurse ? MS_REC : 0);
+
+ return tomoyo_mount_permission(NULL, to, NULL, flags, from);
+}
+
+/**
+ * tomoyo_mount_new - Target for security_mount_new().
+ *
+ * @fc: Pointer to "struct fs_context".
+ * @mp: Pointer to "struct path".
+ * @mnt_flags: Mount options.
+ * @flags: Original mount options.
+ * @data: Optional data. Maybe NULL.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static int tomoyo_mount_new(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ /* Use original MS_* flags for policy matching */
+ return tomoyo_mount_permission(fc->source, mp, fc->fs_type->name,
+ flags, NULL);
+}
+
+/**
+ * tomoyo_mount_remount - Target for security_mount_remount().
+ *
+ * @fc: Pointer to "struct fs_context".
+ * @mp: Pointer to "struct path".
+ * @mnt_flags: Mount options.
+ * @flags: Original mount options.
+ * @data: Optional data. Maybe NULL.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static int tomoyo_mount_remount(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ /* Use original MS_* flags for policy matching */
+ return tomoyo_mount_permission(NULL, mp, NULL, flags, NULL);
+}
+
+/**
+ * tomoyo_mount_reconfigure - Target for security_mount_reconfigure().
+ *
+ * @mp: Pointer to "struct path".
+ * @mnt_flags: Mount options.
+ * @flags: Original mount options.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static int tomoyo_mount_reconfigure(const struct path *mp,
+ unsigned int mnt_flags,
+ unsigned long flags)
+{
+ /* Use original MS_* flags for policy matching */
+ return tomoyo_mount_permission(NULL, mp, NULL, flags, NULL);
+}
+
+/**
+ * tomoyo_mount_change_type - Target for security_mount_change_type().
+ *
+ * @mp: Pointer to "struct path".
+ * @ms_flags: Mount options.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static int tomoyo_mount_change_type(const struct path *mp, int ms_flags)
+{
+ return tomoyo_mount_permission(NULL, mp, NULL, ms_flags, NULL);
+}
+
+/**
+ * tomoyo_mount_move - Target for security_mount_move().
+ *
+ * @from_path: Pointer to "struct path".
+ * @to_path: Pointer to "struct path".
+ * @top_path: Pointer to "struct path" of the mount ending up on top.
+ *
+ * Returns 0 on success, negative value otherwise.
+ */
+static int tomoyo_mount_move(const struct path *from_path,
+ const struct path *to_path,
+ const struct path *top_path)
+{
+ return tomoyo_mount_permission(NULL, to_path, NULL, MS_MOVE,
+ from_path);
}
/**
@@ -576,7 +662,12 @@ static struct security_hook_list tomoyo_hooks[] __ro_after_init = {
LSM_HOOK_INIT(path_chmod, tomoyo_path_chmod),
LSM_HOOK_INIT(path_chown, tomoyo_path_chown),
LSM_HOOK_INIT(path_chroot, tomoyo_path_chroot),
- LSM_HOOK_INIT(sb_mount, tomoyo_sb_mount),
+ LSM_HOOK_INIT(mount_bind, tomoyo_mount_bind),
+ LSM_HOOK_INIT(mount_new, tomoyo_mount_new),
+ LSM_HOOK_INIT(mount_remount, tomoyo_mount_remount),
+ LSM_HOOK_INIT(mount_reconfigure, tomoyo_mount_reconfigure),
+ LSM_HOOK_INIT(mount_change_type, tomoyo_mount_change_type),
+ LSM_HOOK_INIT(mount_move, tomoyo_mount_move),
LSM_HOOK_INIT(sb_umount, tomoyo_sb_umount),
LSM_HOOK_INIT(sb_pivotroot, tomoyo_sb_pivotroot),
LSM_HOOK_INIT(socket_bind, tomoyo_socket_bind),
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v6 5/8] landlock: Convert from sb_mount to granular mount hooks
From: Song Liu @ 2026-07-06 23:50 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260706235053.4104951-1-song@kernel.org>
Replace hook_sb_mount() with granular mount hooks. Landlock denies
all mount operations for sandboxed processes regardless of flags,
so all new hooks share a common hook_mount_deny() helper. The
mount_move hook reuses hook_move_mount() (its new top_path argument
is unused).
Code generated with the assistance of Claude, reviewed by human.
Signed-off-by: Song Liu <song@kernel.org>
---
security/landlock/fs.c | 44 ++++++++++++++++++++++++++++++++++++------
1 file changed, 38 insertions(+), 6 deletions(-)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index f7e5e4ef9eac..0c56a3aa45ea 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1427,9 +1427,7 @@ static void log_fs_change_topology_dentry(
* inherit these new constraints. Anyway, for backward compatibility reasons,
* a dedicated user space option would be required (e.g. as a ruleset flag).
*/
-static int hook_sb_mount(const char *const dev_name,
- const struct path *const path, const char *const type,
- const unsigned long flags, void *const data)
+static int hook_mount_deny(const struct path *const path)
{
size_t handle_layer;
const struct landlock_cred_security *const subject =
@@ -1443,8 +1441,38 @@ static int hook_sb_mount(const char *const dev_name,
return -EPERM;
}
+static int hook_mount_bind(const struct path *const from,
+ const struct path *const to, bool recurse)
+{
+ return hook_mount_deny(to);
+}
+
+static int hook_mount_new(struct fs_context *fc, const struct path *const mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ return hook_mount_deny(mp);
+}
+
+static int hook_mount_remount(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ return hook_mount_deny(mp);
+}
+
+static int hook_mount_reconfigure(const struct path *const mp,
+ unsigned int mnt_flags, unsigned long flags)
+{
+ return hook_mount_deny(mp);
+}
+
+static int hook_mount_change_type(const struct path *const mp, int ms_flags)
+{
+ return hook_mount_deny(mp);
+}
+
static int hook_move_mount(const struct path *const from_path,
- const struct path *const to_path)
+ const struct path *const to_path,
+ const struct path *const top_path)
{
size_t handle_layer;
const struct landlock_cred_security *const subject =
@@ -1981,8 +2009,12 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
LSM_HOOK_INIT(inode_free_security_rcu, hook_inode_free_security_rcu),
LSM_HOOK_INIT(sb_delete, hook_sb_delete),
- LSM_HOOK_INIT(sb_mount, hook_sb_mount),
- LSM_HOOK_INIT(move_mount, hook_move_mount),
+ LSM_HOOK_INIT(mount_bind, hook_mount_bind),
+ LSM_HOOK_INIT(mount_new, hook_mount_new),
+ LSM_HOOK_INIT(mount_remount, hook_mount_remount),
+ LSM_HOOK_INIT(mount_reconfigure, hook_mount_reconfigure),
+ LSM_HOOK_INIT(mount_change_type, hook_mount_change_type),
+ LSM_HOOK_INIT(mount_move, hook_move_mount),
LSM_HOOK_INIT(sb_umount, hook_sb_umount),
LSM_HOOK_INIT(sb_remount, hook_sb_remount),
LSM_HOOK_INIT(sb_pivotroot, hook_sb_pivotroot),
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v6 4/8] selinux: Convert from sb_mount to granular mount hooks
From: Song Liu @ 2026-07-06 23:50 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260706235053.4104951-1-song@kernel.org>
Replace selinux_mount() with granular mount hooks, preserving the
same permission checks:
- mount_bind, mount_new, mount_change_type: FILE__MOUNTON
- mount_remount, mount_reconfigure: FILESYSTEM__REMOUNT
- mount_move: FILE__MOUNTON (reuses selinux_move_mount)
The flags, data, and top_path parameters are unused by SELinux.
Code generated with the assistance of Claude, reviewed by human.
Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: Song Liu <song@kernel.org>
---
security/selinux/hooks.c | 52 ++++++++++++++++++++++++++++------------
1 file changed, 37 insertions(+), 15 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 1a713d96206f..6cbe52375b5c 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2802,23 +2802,42 @@ static int selinux_sb_statfs(struct dentry *dentry)
return superblock_has_perm(cred, dentry->d_sb, FILESYSTEM__GETATTR, &ad);
}
-static int selinux_mount(const char *dev_name,
- const struct path *path,
- const char *type,
- unsigned long flags,
- void *data)
+static int selinux_mount_bind(const struct path *from, const struct path *to,
+ bool recurse)
{
- const struct cred *cred = current_cred();
+ return path_has_perm(current_cred(), to, FILE__MOUNTON);
+}
- if (flags & MS_REMOUNT)
- return superblock_has_perm(cred, path->dentry->d_sb,
- FILESYSTEM__REMOUNT, NULL);
- else
- return path_has_perm(cred, path, FILE__MOUNTON);
+static int selinux_mount_new(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ return path_has_perm(current_cred(), mp, FILE__MOUNTON);
+}
+
+static int selinux_mount_remount(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags,
+ void *data)
+{
+ return superblock_has_perm(current_cred(), fc->root->d_sb,
+ FILESYSTEM__REMOUNT, NULL);
+}
+
+static int selinux_mount_reconfigure(const struct path *mp,
+ unsigned int mnt_flags,
+ unsigned long flags)
+{
+ return superblock_has_perm(current_cred(), mp->dentry->d_sb,
+ FILESYSTEM__REMOUNT, NULL);
+}
+
+static int selinux_mount_change_type(const struct path *mp, int ms_flags)
+{
+ return path_has_perm(current_cred(), mp, FILE__MOUNTON);
}
static int selinux_move_mount(const struct path *from_path,
- const struct path *to_path)
+ const struct path *to_path,
+ const struct path *top_path)
{
const struct cred *cred = current_cred();
@@ -7554,13 +7573,16 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
LSM_HOOK_INIT(sb_kern_mount, selinux_sb_kern_mount),
LSM_HOOK_INIT(sb_show_options, selinux_sb_show_options),
LSM_HOOK_INIT(sb_statfs, selinux_sb_statfs),
- LSM_HOOK_INIT(sb_mount, selinux_mount),
+ LSM_HOOK_INIT(mount_bind, selinux_mount_bind),
+ LSM_HOOK_INIT(mount_new, selinux_mount_new),
+ LSM_HOOK_INIT(mount_remount, selinux_mount_remount),
+ LSM_HOOK_INIT(mount_reconfigure, selinux_mount_reconfigure),
+ LSM_HOOK_INIT(mount_change_type, selinux_mount_change_type),
+ LSM_HOOK_INIT(mount_move, selinux_move_mount),
LSM_HOOK_INIT(sb_umount, selinux_umount),
LSM_HOOK_INIT(sb_set_mnt_opts, selinux_set_mnt_opts),
LSM_HOOK_INIT(sb_clone_mnt_opts, selinux_sb_clone_mnt_opts),
- LSM_HOOK_INIT(move_mount, selinux_move_mount),
-
LSM_HOOK_INIT(dentry_init_security, selinux_dentry_init_security),
LSM_HOOK_INIT(dentry_create_files_as, selinux_dentry_create_files_as),
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v6 3/8] apparmor: Convert from sb_mount to granular mount hooks
From: Song Liu @ 2026-07-06 23:50 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260706235053.4104951-1-song@kernel.org>
Replace AppArmor's monolithic apparmor_sb_mount() with granular
mount hooks.
Key changes:
- mount_bind: uses the pre-resolved struct path from VFS instead of
re-resolving dev_name via kern_path(), eliminating a TOCTOU
vulnerability. aa_bind_mount() now takes a struct path instead of
a string for the source.
- mount_new, mount_remount: receive the original mount(2) flags and
data parameters for policy matching via match_mnt_flags() and
AA_MNT_CONT_MATCH data matching.
- mount_reconfigure: handles MS_REMOUNT|MS_BIND (mount attribute
reconfiguration) which was previously handled as a remount.
- mount_move: wired to apparmor_move_mount(); its signature gains the
new top_path argument (currently unused, reserved for
MOVE_MOUNT_BENEATH policy).
- mount_change_type: propagation type changes.
aa_move_mount_old() is removed since move mounts now go through
security_mount_move() with pre-resolved struct path pointers for
both the old mount(2) and new move_mount(2) APIs.
Code generated with the assistance of Claude, reviewed by human.
Signed-off-by: Song Liu <song@kernel.org>
---
security/apparmor/include/mount.h | 5 +-
security/apparmor/lsm.c | 103 +++++++++++++++++++++++-------
security/apparmor/mount.c | 37 ++---------
3 files changed, 85 insertions(+), 60 deletions(-)
diff --git a/security/apparmor/include/mount.h b/security/apparmor/include/mount.h
index 46834f828179..088e2f938cc1 100644
--- a/security/apparmor/include/mount.h
+++ b/security/apparmor/include/mount.h
@@ -31,16 +31,13 @@ int aa_remount(const struct cred *subj_cred,
int aa_bind_mount(const struct cred *subj_cred,
struct aa_label *label, const struct path *path,
- const char *old_name, unsigned long flags);
+ const struct path *old_path, bool recurse);
int aa_mount_change_type(const struct cred *subj_cred,
struct aa_label *label, const struct path *path,
unsigned long flags);
-int aa_move_mount_old(const struct cred *subj_cred,
- struct aa_label *label, const struct path *path,
- const char *old_name);
int aa_move_mount(const struct cred *subj_cred,
struct aa_label *label, const struct path *from_path,
const struct path *to_path);
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 71f30dd43f97..d9d5822e4022 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -13,6 +13,7 @@
#include <linux/mm.h>
#include <linux/mman.h>
#include <linux/mount.h>
+#include <linux/fs_context.h>
#include <linux/namei.h>
#include <linux/ptrace.h>
#include <linux/ctype.h>
@@ -698,41 +699,91 @@ static int apparmor_uring_sqpoll(void)
}
#endif /* CONFIG_IO_URING */
-static int apparmor_sb_mount(const char *dev_name, const struct path *path,
- const char *type, unsigned long flags, void *data)
+static int apparmor_mount_bind(const struct path *from, const struct path *to,
+ bool recurse)
{
struct aa_label *label;
int error = 0;
bool needput;
- flags &= ~AA_MS_IGNORE_MASK;
+ label = __begin_current_label_crit_section(&needput);
+ if (!unconfined(label))
+ error = aa_bind_mount(current_cred(), label, to, from,
+ recurse);
+ __end_current_label_crit_section(label, needput);
+ return error;
+}
+
+static int apparmor_mount_new(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags, void *data)
+{
+ struct aa_label *label;
+ int error = 0;
+ bool needput;
+
+ /* flags and data are from the original mount(2) call */
label = __begin_current_label_crit_section(&needput);
- if (!unconfined(label)) {
- if (flags & MS_REMOUNT)
- error = aa_remount(current_cred(), label, path, flags,
- data);
- else if (flags & MS_BIND)
- error = aa_bind_mount(current_cred(), label, path,
- dev_name, flags);
- else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE |
- MS_UNBINDABLE))
- error = aa_mount_change_type(current_cred(), label,
- path, flags);
- else if (flags & MS_MOVE)
- error = aa_move_mount_old(current_cred(), label, path,
- dev_name);
- else
- error = aa_new_mount(current_cred(), label, dev_name,
- path, type, flags, data);
- }
+ if (!unconfined(label))
+ error = aa_new_mount(current_cred(), label, fc->source,
+ mp, fc->fs_type->name, flags, data);
+ __end_current_label_crit_section(label, needput);
+
+ return error;
+}
+
+static int apparmor_mount_remount(struct fs_context *fc, const struct path *mp,
+ int mnt_flags, unsigned long flags,
+ void *data)
+{
+ struct aa_label *label;
+ int error = 0;
+ bool needput;
+
+ /* flags and data are from the original mount(2) call */
+ label = __begin_current_label_crit_section(&needput);
+ if (!unconfined(label))
+ error = aa_remount(current_cred(), label, mp, flags, data);
+ __end_current_label_crit_section(label, needput);
+
+ return error;
+}
+
+static int apparmor_mount_reconfigure(const struct path *mp,
+ unsigned int mnt_flags,
+ unsigned long flags)
+{
+ struct aa_label *label;
+ int error = 0;
+ bool needput;
+
+ /* flags are from the original mount(2) call */
+ label = __begin_current_label_crit_section(&needput);
+ if (!unconfined(label))
+ error = aa_remount(current_cred(), label, mp, flags, NULL);
+ __end_current_label_crit_section(label, needput);
+
+ return error;
+}
+
+static int apparmor_mount_change_type(const struct path *mp, int ms_flags)
+{
+ struct aa_label *label;
+ int error = 0;
+ bool needput;
+
+ label = __begin_current_label_crit_section(&needput);
+ if (!unconfined(label))
+ error = aa_mount_change_type(current_cred(), label, mp,
+ ms_flags);
__end_current_label_crit_section(label, needput);
return error;
}
static int apparmor_move_mount(const struct path *from_path,
- const struct path *to_path)
+ const struct path *to_path,
+ const struct path *top_path)
{
struct aa_label *label;
int error = 0;
@@ -1669,8 +1720,12 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = {
LSM_HOOK_INIT(capget, apparmor_capget),
LSM_HOOK_INIT(capable, apparmor_capable),
- LSM_HOOK_INIT(move_mount, apparmor_move_mount),
- LSM_HOOK_INIT(sb_mount, apparmor_sb_mount),
+ LSM_HOOK_INIT(mount_bind, apparmor_mount_bind),
+ LSM_HOOK_INIT(mount_new, apparmor_mount_new),
+ LSM_HOOK_INIT(mount_remount, apparmor_mount_remount),
+ LSM_HOOK_INIT(mount_reconfigure, apparmor_mount_reconfigure),
+ LSM_HOOK_INIT(mount_move, apparmor_move_mount),
+ LSM_HOOK_INIT(mount_change_type, apparmor_mount_change_type),
LSM_HOOK_INIT(sb_umount, apparmor_sb_umount),
LSM_HOOK_INIT(sb_pivotroot, apparmor_sb_pivotroot),
diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index 2f5d918832c1..1ea0d680437c 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -418,25 +418,17 @@ int aa_remount(const struct cred *subj_cred,
}
int aa_bind_mount(const struct cred *subj_cred,
- struct aa_label *label, const struct path *path,
- const char *dev_name, unsigned long flags)
+ struct aa_label *label, const struct path *path,
+ const struct path *old_path, bool recurse)
{
struct aa_profile *profile;
char *buffer = NULL, *old_buffer = NULL;
- struct path old_path;
+ unsigned long flags = MS_BIND | (recurse ? MS_REC : 0);
int error;
AA_BUG(!label);
AA_BUG(!path);
-
- if (!dev_name || !*dev_name)
- return -EINVAL;
-
- flags &= MS_REC | MS_BIND;
-
- error = kern_path(dev_name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &old_path);
- if (error)
- return error;
+ AA_BUG(!old_path);
buffer = aa_get_buffer(false);
old_buffer = aa_get_buffer(false);
@@ -445,12 +437,11 @@ int aa_bind_mount(const struct cred *subj_cred,
goto out;
error = fn_for_each_confined(label, profile,
- match_mnt(subj_cred, profile, path, buffer, &old_path,
+ match_mnt(subj_cred, profile, path, buffer, old_path,
old_buffer, NULL, flags, NULL, false));
out:
aa_put_buffer(buffer);
aa_put_buffer(old_buffer);
- path_put(&old_path);
return error;
}
@@ -514,24 +505,6 @@ int aa_move_mount(const struct cred *subj_cred,
return error;
}
-int aa_move_mount_old(const struct cred *subj_cred, struct aa_label *label,
- const struct path *path, const char *orig_name)
-{
- struct path old_path;
- int error;
-
- if (!orig_name || !*orig_name)
- return -EINVAL;
- error = kern_path(orig_name, LOOKUP_FOLLOW, &old_path);
- if (error)
- return error;
-
- error = aa_move_mount(subj_cred, label, &old_path, path);
- path_put(&old_path);
-
- return error;
-}
-
int aa_new_mount(const struct cred *subj_cred, struct aa_label *label,
const char *dev_name, const struct path *path,
const char *type, unsigned long flags, void *data)
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v6 2/8] apparmor: Remove redundant MS_MGC_MSK stripping in apparmor_sb_mount
From: Song Liu @ 2026-07-06 23:50 UTC (permalink / raw)
To: linux-security-module, linux-fsdevel, selinux, apparmor
Cc: paul, jmorris, serge, viro, brauner, jack, john.johansen,
stephen.smalley.work, omosnace, mic, gnoack, takedakn,
penguin-kernel, herton, kernel-team, Song Liu
In-Reply-To: <20260706235053.4104951-1-song@kernel.org>
path_mount() already strips the magic number from flags before
calling security_sb_mount(), so this check in apparmor_sb_mount()
is a no-op. Remove it.
Code generated with the assistance of Claude, reviewed by human.
Signed-off-by: Song Liu <song@kernel.org>
---
security/apparmor/lsm.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 88d12e89d115..71f30dd43f97 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -705,10 +705,6 @@ static int apparmor_sb_mount(const char *dev_name, const struct path *path,
int error = 0;
bool needput;
- /* Discard magic */
- if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
- flags &= ~MS_MGC_MSK;
-
flags &= ~AA_MS_IGNORE_MASK;
label = __begin_current_label_crit_section(&needput);
--
2.53.0-Meta
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox