* [PATCH bpf-next 0/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling @ 2026-04-27 0:15 David Windsor 2026-04-27 0:15 ` [PATCH bpf-next 1/2] " David Windsor 2026-04-27 0:15 ` [PATCH bpf-next 2/2] selftests/bpf: add tests for bpf_init_inode_xattr kfunc David Windsor 0 siblings, 2 replies; 18+ messages in thread From: David Windsor @ 2026-04-27 0:15 UTC (permalink / raw) Cc: David Windsor, bpf Many in-kernel LSMs (SELinux, Smack, IMA) store security labels in extended attributes. For these LSMs, atomic labeling during inode creation is critical: if the inode becomes accessible before its xattr is set, it is briefly unlabeled, which can disrupt LSMs making policy decisions based on file labels. Existing LSMs solve this by setting xattrs directly in the inode_init_security hook, which runs before the inode becomes accessible. BPF LSM programs currently lack this capability because the hook uses an output parameter (xattr_count) that BPF programs cannot write to, and existing kfuncs like bpf_dentry_set_xattr require a dentry that isn't available until after the inode is accessible. This series introduces the bpf_init_inode_xattr() kfunc, which takes the hook's PTR_TO_CTX to access xattrs and xattr_count, and internally writes to xattr_count via lsm_get_xattr_slot(). David Windsor (2): bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling selftests/bpf: add tests for bpf_init_inode_xattr kfunc fs/bpf_fs_kfuncs.c | 80 ++++++++++++++++++- include/linux/bpf_verifier.h | 3 + kernel/bpf/fixups.c | 20 +++++ kernel/bpf/verifier.c | 54 +++++++++++++ security/bpf/hooks.c | 3 + tools/testing/selftests/bpf/bpf_kfuncs.h | 3 + .../selftests/bpf/prog_tests/fs_kfuncs.c | 49 ++++++++++++ .../bpf/progs/test_init_inode_xattr.c | 32 ++++++++ 8 files changed, 243 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/bpf/progs/test_init_inode_xattr.c base-commit: 6c60b2dd5a7889a583389e95e79689191206f86f -- 2.53.0 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 0:15 [PATCH bpf-next 0/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling David Windsor @ 2026-04-27 0:15 ` David Windsor 2026-04-27 0:51 ` bot+bpf-ci ` (3 more replies) 2026-04-27 0:15 ` [PATCH bpf-next 2/2] selftests/bpf: add tests for bpf_init_inode_xattr kfunc David Windsor 1 sibling, 4 replies; 18+ messages in thread From: David Windsor @ 2026-04-27 0:15 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, KP Singh, Matt Bobrowski, Paul Moore, James Morris, Serge E. Hallyn Cc: David Windsor, Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf, linux-security-module Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set xattrs via inode_init_security hook using lsm_get_xattr_slot(). lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF programs cannot do: hook arguments are not directly writable from BPF. To hide this, the BPF-facing API is just bpf_init_inode_xattr(name, value), and the verifier transparently rewrites each call into bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and xattr_count are extracted from the hook context, which the verifier spills to the stack at program entry since R1 is clobbered during normal execution. A previous attempt [1] required a kmalloc string output protocol for the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to provide xattrs for inode_init_security hook") [2], the xattr name is no longer allocated; it is a static constant. We take advantage of this by passing the name directly. Because we rely on the hook-specific ctx layout, the kfunc is restricted to lsm/inode_init_security. Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1] Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2] Suggested-by: Song Liu <song@kernel.org> Signed-off-by: David Windsor <dwindsor@gmail.com> --- fs/bpf_fs_kfuncs.c | 80 +++++++++++++++++++++++++++++++++++- include/linux/bpf_verifier.h | 3 ++ kernel/bpf/fixups.c | 20 +++++++++ kernel/bpf/verifier.c | 54 ++++++++++++++++++++++++ security/bpf/hooks.c | 3 ++ 5 files changed, 159 insertions(+), 1 deletion(-) diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c index 9d27be058494..5a5951006a3f 100644 --- a/fs/bpf_fs_kfuncs.c +++ b/fs/bpf_fs_kfuncs.c @@ -10,6 +10,7 @@ #include <linux/fsnotify.h> #include <linux/file.h> #include <linux/kernfs.h> +#include <linux/lsm_hooks.h> #include <linux/mm.h> #include <linux/xattr.h> @@ -353,6 +354,68 @@ __bpf_kfunc int bpf_cgroup_read_xattr(struct cgroup *cgroup, const char *name__s } #endif /* CONFIG_CGROUPS */ +/* Called from the verifier fixup of bpf_init_inode_xattr(). */ +__bpf_kfunc int bpf_init_inode_xattr_impl(struct xattr *xattrs, int *xattr_count, + const char *name__str, + const struct bpf_dynptr *value_p) +{ + struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p; + size_t name_len; + void *xattr_value; + struct xattr *xattr; + const void *value; + u32 value_len; + + if (!xattrs || !xattr_count || !name__str) + return -EINVAL; + + name_len = strlen(name__str); + if (name_len == 0 || name_len > XATTR_NAME_MAX) + return -EINVAL; + + value_len = __bpf_dynptr_size(value_ptr); + if (value_len == 0 || value_len > XATTR_SIZE_MAX) + return -EINVAL; + + value = __bpf_dynptr_data(value_ptr, value_len); + if (!value) + return -EINVAL; + + xattr_value = kmemdup(value, value_len, GFP_ATOMIC); + if (!xattr_value) + return -ENOMEM; + + xattr = lsm_get_xattr_slot(xattrs, xattr_count); + if (!xattr) { + kfree(xattr_value); + return -ENOSPC; + } + + xattr->name = name__str; + xattr->value = xattr_value; + xattr->value_len = value_len; + + return 0; +} + +/** + * bpf_init_inode_xattr - set an xattr on a new inode from inode_init_security + * @name__str: xattr name (e.g., "bpf.file_label") + * @value_p: dynptr containing the xattr value + * + * Only callable from lsm/inode_init_security programs. The verifier rewrites + * calls to bpf_init_inode_xattr_impl() with xattrs/xattr_count extracted from + * the hook context. + * + * Return: 0 on success, negative error on failure. + */ +__bpf_kfunc int bpf_init_inode_xattr(const char *name__str, + const struct bpf_dynptr *value_p) +{ + WARN_ONCE(1, "%s called without verifier fixup\n", __func__); + return -EFAULT; +} + __bpf_kfunc_end_defs(); BTF_KFUNCS_START(bpf_fs_kfunc_set_ids) @@ -363,13 +426,28 @@ BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE) BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE) BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE) BTF_ID_FLAGS(func, bpf_remove_dentry_xattr, KF_SLEEPABLE) +BTF_ID_FLAGS(func, bpf_init_inode_xattr) +BTF_ID_FLAGS(func, bpf_init_inode_xattr_impl) BTF_KFUNCS_END(bpf_fs_kfunc_set_ids) +BTF_ID_LIST(bpf_lsm_inode_init_security_btf_ids) +BTF_ID(func, bpf_lsm_inode_init_security) + +BTF_ID_LIST(bpf_init_inode_xattr_btf_ids) +BTF_ID(func, bpf_init_inode_xattr) +BTF_ID(func, bpf_init_inode_xattr_impl) + static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id) { if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) || - prog->type == BPF_PROG_TYPE_LSM) + prog->type == BPF_PROG_TYPE_LSM) { + /* bpf_init_inode_xattr[_impl] only attach to inode_init_security. */ + if ((kfunc_id == bpf_init_inode_xattr_btf_ids[0] || + kfunc_id == bpf_init_inode_xattr_btf_ids[1]) && + prog->aux->attach_btf_id != bpf_lsm_inode_init_security_btf_ids[0]) + return -EACCES; return 0; + } return -EACCES; } diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 101ca6cc5424..e73bb2222c3d 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -682,6 +682,7 @@ struct bpf_insn_aux_data { */ u8 fastcall_spills_num:3; u8 arg_prog:4; + u8 init_inode_xattr_fixup:1; /* below fields are initialized once */ unsigned int orig_idx; /* original instruction index */ @@ -903,6 +904,8 @@ struct bpf_verifier_env { bool bypass_spec_v4; bool seen_direct_write; bool seen_exception; + bool needs_ctx_spill; + s16 ctx_stack_off; 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/fixups.c b/kernel/bpf/fixups.c index fba9e8c00878..18d612a9fe29 100644 --- a/kernel/bpf/fixups.c +++ b/kernel/bpf/fixups.c @@ -725,6 +725,26 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env) } } + if (env->needs_ctx_spill) { + if (epilogue_cnt) { + /* gen_epilogue already saved ctx to the stack */ + env->ctx_stack_off = -(s16)subprogs[0].stack_depth; + } else { + cnt = 0; + subprogs[0].stack_depth += 8; + env->ctx_stack_off = -(s16)subprogs[0].stack_depth; + insn_buf[cnt++] = BPF_STX_MEM(BPF_DW, BPF_REG_FP, + BPF_REG_1, + env->ctx_stack_off); + insn_buf[cnt++] = env->prog->insnsi[0]; + new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt); + if (!new_prog) + return -ENOMEM; + env->prog = new_prog; + delta += cnt - 1; + } + } + if (ops->gen_prologue || env->seen_direct_write) { if (!ops->gen_prologue) { verifier_bug(env, "gen_prologue is null"); diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 03f9e16c2abe..af5753ffb16b 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -10794,6 +10794,8 @@ enum special_kfunc_type { KF_bpf_arena_alloc_pages, KF_bpf_arena_free_pages, KF_bpf_arena_reserve_pages, + KF_bpf_init_inode_xattr, + KF_bpf_init_inode_xattr_impl, KF_bpf_session_is_return, KF_bpf_stream_vprintk, KF_bpf_stream_print_stack, @@ -10882,6 +10884,13 @@ BTF_ID(func, bpf_task_work_schedule_resume) BTF_ID(func, bpf_arena_alloc_pages) BTF_ID(func, bpf_arena_free_pages) BTF_ID(func, bpf_arena_reserve_pages) +#ifdef CONFIG_BPF_LSM +BTF_ID(func, bpf_init_inode_xattr) +BTF_ID(func, bpf_init_inode_xattr_impl) +#else +BTF_ID_UNUSED +BTF_ID_UNUSED +#endif BTF_ID(func, bpf_session_is_return) BTF_ID(func, bpf_stream_vprintk) BTF_ID(func, bpf_stream_print_stack) @@ -12701,6 +12710,24 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, if (err < 0) return err; + if (meta.func_id == special_kfunc_list[KF_bpf_init_inode_xattr_impl]) { + verbose(env, "bpf_init_inode_xattr_impl is not callable directly\n"); + return -EACCES; + } + + if (meta.func_id == special_kfunc_list[KF_bpf_init_inode_xattr]) { + if (env->cur_state->curframe != 0) { + verbose(env, "bpf_init_inode_xattr cannot be called from subprograms\n"); + return -EINVAL; + } + env->needs_ctx_spill = true; + insn_aux->init_inode_xattr_fixup = true; + err = bpf_add_kfunc_call(env, + special_kfunc_list[KF_bpf_init_inode_xattr_impl], 0); + if (err < 0) + return err; + } + if (is_bpf_rbtree_add_kfunc(meta.func_id)) { err = push_callback_call(env, insn, insn_idx, meta.subprogno, set_rbtree_add_callback_state); @@ -19272,6 +19299,33 @@ int bpf_fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, insn_buf[4] = BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1); insn_buf[5] = BPF_ALU64_IMM(BPF_NEG, BPF_REG_0, 0); *cnt = 6; + } else if (env->insn_aux_data[insn_idx].init_inode_xattr_fixup) { + struct bpf_kfunc_desc *impl_desc; + + impl_desc = find_kfunc_desc(env->prog, + special_kfunc_list[KF_bpf_init_inode_xattr_impl], 0); + if (!impl_desc) { + verifier_bug(env, "bpf_init_inode_xattr_impl desc not found"); + return -EFAULT; + } + + /* Rewrite bpf_init_inode_xattr(name, value) to inject xattrs and + * xattr_count loaded from the saved inode_init_security ctx. + */ + insn_buf[0] = BPF_MOV64_REG(BPF_REG_3, BPF_REG_1); + insn_buf[1] = BPF_MOV64_REG(BPF_REG_4, BPF_REG_2); + insn_buf[2] = BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_FP, + env->ctx_stack_off); + insn_buf[3] = BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_2, + 3 * sizeof(u64)); + insn_buf[4] = BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_2, + 4 * sizeof(u64)); + insn_buf[5] = *insn; + if (!bpf_jit_supports_far_kfunc_call()) + insn_buf[5].imm = BPF_CALL_IMM(impl_desc->addr); + else + insn_buf[5].imm = impl_desc->func_id; + *cnt = 6; } if (env->insn_aux_data[insn_idx].arg_prog) { diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c index 40efde233f3a..1e61baa821bd 100644 --- a/security/bpf/hooks.c +++ b/security/bpf/hooks.c @@ -28,8 +28,11 @@ static int __init bpf_lsm_init(void) return 0; } +#define BPF_LSM_INODE_INIT_XATTRS 1 + struct lsm_blob_sizes bpf_lsm_blob_sizes __ro_after_init = { .lbs_inode = sizeof(struct bpf_storage_blob), + .lbs_xattr_count = BPF_LSM_INODE_INIT_XATTRS, }; DEFINE_LSM(bpf) = { -- 2.53.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 0:15 ` [PATCH bpf-next 1/2] " David Windsor @ 2026-04-27 0:51 ` bot+bpf-ci 2026-04-27 0:56 ` sashiko-bot ` (2 subsequent siblings) 3 siblings, 0 replies; 18+ messages in thread From: bot+bpf-ci @ 2026-04-27 0:51 UTC (permalink / raw) To: dwindsor, viro, brauner, ast, daniel, andrii, eddyz87, memxor, kpsingh, mattbobrowski, paul, jmorris, serge Cc: dwindsor, song, jack, john.fastabend, martin.lau, yonghong.song, jolsa, linux-fsdevel, linux-kernel, bpf, linux-security-module, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai [-- Attachment #1: Type: text/plain, Size: 1536 bytes --] > diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c > --- a/fs/bpf_fs_kfuncs.c > +++ b/fs/bpf_fs_kfuncs.c [ ... ] > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c [ ... ] > @@ -19272,6 +19299,33 @@ struct bpf_insn addr[2] = { BPF_LD_IMM64(BPF_REG_2, (long)kptr_struct_meta) }; > insn_buf[4] = BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1); > insn_buf[5] = BPF_ALU64_IMM(BPF_NEG, BPF_REG_0, 0); > *cnt = 6; > + } else if (env->insn_aux_data[insn_idx].init_inode_xattr_fixup) { > + struct bpf_kfunc_desc *impl_desc; > + > + impl_desc = find_kfunc_desc(env->prog, > + special_kfunc_list[KF_bpf_init_inode_xattr_impl], 0); > + if (!impl_desc) { > + verifier_bug(env, "bpf_init_inode_xattr_impl desc not found"); > + return -EFAULT; > + } > + > + /* Rewrite bpf_init_inode_xattr(name, value) to inject xattrs and > + * xattr_count loaded from the saved inode_init_security ctx. > + */ This isn't a bug, but the comment style needs adjustment. In kernel/bpf/verifier.c, multi-line comments should have the opening /* on its own line: /* * Rewrite bpf_init_inode_xattr(name, value) to inject xattrs * and xattr_count loaded from the saved inode_init_security * ctx. */ [ ... ] --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24970971112 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 0:15 ` [PATCH bpf-next 1/2] " David Windsor 2026-04-27 0:51 ` bot+bpf-ci @ 2026-04-27 0:56 ` sashiko-bot 2026-04-27 2:10 ` David Windsor 2026-04-27 2:56 ` Kumar Kartikeya Dwivedi 2026-04-27 9:48 ` Matt Bobrowski 3 siblings, 1 reply; 18+ messages in thread From: sashiko-bot @ 2026-04-27 0:56 UTC (permalink / raw) To: David Windsor; +Cc: bpf Sashiko AI review found 4 potential issue(s): - [Critical] BPF programs can trigger a heap buffer overflow by calling bpf_init_inode_xattr multiple times. - [Critical] Context spoofing via backward jump leads to arbitrary kernel memory read/write. - [High] Storing name__str directly into the xattr array leads to a Use-After-Free during filesystem operations. - [High] Missing prefix validation on the xattr name allows BPF programs to spoof standard security xattrs. -- commit 40ce6a42efab6a64fca9ea5bd1aec81b4faa93f2 Author: David Windsor <dwindsor@gmail.com> bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set xattrs via inode_init_security hook using lsm_get_xattr_slot(). Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1] Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2] > diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c > --- a/fs/bpf_fs_kfuncs.c > +++ b/fs/bpf_fs_kfuncs.c [ ... ] > @@ -353,6 +354,68 @@ __bpf_kfunc int bpf_cgroup_read_xattr(struct cgroup *cgroup, const char *name__s > } > #endif /* CONFIG_CGROUPS */ > > +/* Called from the verifier fixup of bpf_init_inode_xattr(). */ > +__bpf_kfunc int bpf_init_inode_xattr_impl(struct xattr *xattrs, int *xattr_count, > + const char *name__str, > + const struct bpf_dynptr *value_p) > +{ [ ... ] > + name_len = strlen(name__str); > + if (name_len == 0 || name_len > XATTR_NAME_MAX) > + return -EINVAL; Does this code need to validate the xattr prefix? Other BPF xattr kfuncs enforce a prefix like "security.bpf." to prevent interfering with other security modules. Without this validation, could a BPF program pass a name like "selinux" and inadvertently spoof standard security xattrs? [ ... ] > + xattr = lsm_get_xattr_slot(xattrs, xattr_count); > + if (!xattr) { > + kfree(xattr_value); > + return -ENOSPC; > + } > + > + xattr->name = name__str; Can this result in a use-after-free of the name string? The name__str argument typically resides in a BPF map. Since the filesystem's initxattrs callback can perform disk I/O and sleep after the BPF hook completes, what happens if the BPF program is unloaded and its maps are freed concurrently? The name pointer might then point to freed memory when the filesystem attempts to write the xattr. > + xattr->value = xattr_value; > + xattr->value_len = value_len; > + > + return 0; > +} [ ... ] > diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c > --- a/kernel/bpf/fixups.c > +++ b/kernel/bpf/fixups.c [ ... ] > @@ -725,6 +725,26 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env) > } > } > > + if (env->needs_ctx_spill) { > + if (epilogue_cnt) { > + /* gen_epilogue already saved ctx to the stack */ > + env->ctx_stack_off = -(s16)subprogs[0].stack_depth; > + } else { > + cnt = 0; > + subprogs[0].stack_depth += 8; > + env->ctx_stack_off = -(s16)subprogs[0].stack_depth; > + insn_buf[cnt++] = BPF_STX_MEM(BPF_DW, BPF_REG_FP, > + BPF_REG_1, > + env->ctx_stack_off); > + insn_buf[cnt++] = env->prog->insnsi[0]; Is it safe to prepend the context spill instruction at the beginning of the program? Because the original BPF program does not use R1, the verifier's liveness analysis will mark R1 as dead at the start of the program. If a BPF program writes a user-controlled pointer into R1 and creates a bounded loop jumping back to the start of the program, could it trick the verifier into accepting the jump and saving a fake context to the stack slot? This could allow the kfunc fixup to extract xattrs and xattr_count from an attacker-controlled pointer, potentially leading to arbitrary kernel memory reads or writes. > + new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt); > + if (!new_prog) > + return -ENOMEM; [ ... ] > diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c > --- a/security/bpf/hooks.c > +++ b/security/bpf/hooks.c [ ... ] > @@ -28,8 +28,11 @@ static int __init bpf_lsm_init(void) > return 0; > } > > +#define BPF_LSM_INODE_INIT_XATTRS 1 > + > struct lsm_blob_sizes bpf_lsm_blob_sizes __ro_after_init = { > .lbs_inode = sizeof(struct bpf_storage_blob), > + .lbs_xattr_count = BPF_LSM_INODE_INIT_XATTRS, > }; Because lbs_xattr_count is set to 1, security_inode_init_security() will allocate exactly 1 xattr slot for BPF LSMs. Since a BPF program could call bpf_init_inode_xattr() multiple times, and lsm_get_xattr_slot() increments the slot index without bounds checking, could multiple calls write past the end of the new_xattrs heap array? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260427001602.38353-1-dwindsor@gmail.com?part=1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 0:56 ` sashiko-bot @ 2026-04-27 2:10 ` David Windsor 0 siblings, 0 replies; 18+ messages in thread From: David Windsor @ 2026-04-27 2:10 UTC (permalink / raw) To: sashiko; +Cc: bpf On Sun, Apr 26, 2026 at 8:56 PM <sashiko-bot@kernel.org> wrote: > ... > > Can this result in a use-after-free of the name string? The name__str > argument typically resides in a BPF map. > > Since the filesystem's initxattrs callback can perform disk I/O and sleep > after the BPF hook completes, what happens if the BPF program is unloaded > and its maps are freed concurrently? > > The name pointer might then point to freed memory when the filesystem > attempts to write the xattr. > Yeah, good point. We'll kmemdup the name just as we're already doing for the value. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 0:15 ` [PATCH bpf-next 1/2] " David Windsor 2026-04-27 0:51 ` bot+bpf-ci 2026-04-27 0:56 ` sashiko-bot @ 2026-04-27 2:56 ` Kumar Kartikeya Dwivedi 2026-04-27 3:23 ` David Windsor 2026-04-27 9:48 ` Matt Bobrowski 3 siblings, 1 reply; 18+ messages in thread From: Kumar Kartikeya Dwivedi @ 2026-04-27 2:56 UTC (permalink / raw) To: David Windsor Cc: Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh, Matt Bobrowski, Paul Moore, James Morris, Serge E. Hallyn, Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf, linux-security-module On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote: > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set > xattrs via inode_init_security hook using lsm_get_xattr_slot(). > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF > programs cannot do: hook arguments are not directly writable from BPF. > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name, > value), and the verifier transparently rewrites each call into > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and > xattr_count are extracted from the hook context, which the verifier > spills to the stack at program entry since R1 is clobbered during normal > execution. > > A previous attempt [1] required a kmalloc string output protocol for > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to > provide xattrs for inode_init_security hook") [2], the xattr name is no > longer allocated; it is a static constant. We take advantage of this by > passing the name directly. Because we rely on the hook-specific ctx > layout, the kfunc is restricted to lsm/inode_init_security. > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1] > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2] > Suggested-by: Song Liu <song@kernel.org> > Signed-off-by: David Windsor <dwindsor@gmail.com> > --- The explanation and code make no sense to me. Why not pass xattrs and xattr_count directly as arguments, even if you choose to restrict the kfunc to a specific hook? Why does the verifier core need the hack to spill the context and extract the two arguments? pw-bot: cr > [...] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 2:56 ` Kumar Kartikeya Dwivedi @ 2026-04-27 3:23 ` David Windsor 2026-04-27 3:32 ` Kumar Kartikeya Dwivedi 0 siblings, 1 reply; 18+ messages in thread From: David Windsor @ 2026-04-27 3:23 UTC (permalink / raw) To: Kumar Kartikeya Dwivedi Cc: Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh, Matt Bobrowski, Paul Moore, James Morris, Serge E. Hallyn, Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf, linux-security-module On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote: > > > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set > > xattrs via inode_init_security hook using lsm_get_xattr_slot(). > > > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF > > programs cannot do: hook arguments are not directly writable from BPF. > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name, > > value), and the verifier transparently rewrites each call into > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and > > xattr_count are extracted from the hook context, which the verifier > > spills to the stack at program entry since R1 is clobbered during normal > > execution. > > > > A previous attempt [1] required a kmalloc string output protocol for > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to > > provide xattrs for inode_init_security hook") [2], the xattr name is no > > longer allocated; it is a static constant. We take advantage of this by > > passing the name directly. Because we rely on the hook-specific ctx > > layout, the kfunc is restricted to lsm/inode_init_security. > > > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1] > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2] > > Suggested-by: Song Liu <song@kernel.org> > > Signed-off-by: David Windsor <dwindsor@gmail.com> > > --- > > The explanation and code make no sense to me. Why not pass xattrs and > xattr_count directly as arguments, even if you choose to restrict the > kfunc to a specific hook? Why does the verifier core need the hack to > spill the context and extract the two arguments? > xattr_count is an output parameter; we cannot currently write to it in bpf as there is no verifier support for writing to int *. xattrs and xattr_count can be fixed up by the verifier, so we only require the user to pass the name and value. > pw-bot: cr > > > [...] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 3:23 ` David Windsor @ 2026-04-27 3:32 ` Kumar Kartikeya Dwivedi 2026-04-27 3:42 ` David Windsor 2026-04-27 10:11 ` Matt Bobrowski 0 siblings, 2 replies; 18+ messages in thread From: Kumar Kartikeya Dwivedi @ 2026-04-27 3:32 UTC (permalink / raw) To: David Windsor Cc: Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh, Matt Bobrowski, Paul Moore, James Morris, Serge E. Hallyn, Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf, linux-security-module On Mon, 27 Apr 2026 at 05:24, David Windsor <dwindsor@gmail.com> wrote: > > On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi > <memxor@gmail.com> wrote: > > > > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote: > > > > > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set > > > xattrs via inode_init_security hook using lsm_get_xattr_slot(). > > > > > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF > > > programs cannot do: hook arguments are not directly writable from BPF. > > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name, > > > value), and the verifier transparently rewrites each call into > > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and > > > xattr_count are extracted from the hook context, which the verifier > > > spills to the stack at program entry since R1 is clobbered during normal > > > execution. > > > > > > A previous attempt [1] required a kmalloc string output protocol for > > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to > > > provide xattrs for inode_init_security hook") [2], the xattr name is no > > > longer allocated; it is a static constant. We take advantage of this by > > > passing the name directly. Because we rely on the hook-specific ctx > > > layout, the kfunc is restricted to lsm/inode_init_security. > > > > > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1] > > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2] > > > Suggested-by: Song Liu <song@kernel.org> > > > Signed-off-by: David Windsor <dwindsor@gmail.com> > > > --- > > > > The explanation and code make no sense to me. Why not pass xattrs and > > xattr_count directly as arguments, even if you choose to restrict the > > kfunc to a specific hook? Why does the verifier core need the hack to > > spill the context and extract the two arguments? > > > > xattr_count is an output parameter; we cannot currently write to it in > bpf as there is no verifier support for writing to int *. xattrs and > xattr_count can be fixed up by the verifier, so we only require the > user to pass the name and value. Sure, but the kfunc can. Did you try passing them in directly? If that doesn't work for some reason, we should fix it instead. > > > pw-bot: cr > > > > > [...] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 3:32 ` Kumar Kartikeya Dwivedi @ 2026-04-27 3:42 ` David Windsor 2026-04-27 10:11 ` Matt Bobrowski 1 sibling, 0 replies; 18+ messages in thread From: David Windsor @ 2026-04-27 3:42 UTC (permalink / raw) To: Kumar Kartikeya Dwivedi Cc: Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh, Matt Bobrowski, Paul Moore, James Morris, Serge E. Hallyn, Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf, linux-security-module On Sun, Apr 26, 2026 at 11:33 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > On Mon, 27 Apr 2026 at 05:24, David Windsor <dwindsor@gmail.com> wrote: > > > > On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi > > <memxor@gmail.com> wrote: > > > > > > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote: > > > > > > > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set > > > > xattrs via inode_init_security hook using lsm_get_xattr_slot(). > > > > > > > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF > > > > programs cannot do: hook arguments are not directly writable from BPF. > > > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name, > > > > value), and the verifier transparently rewrites each call into > > > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and > > > > xattr_count are extracted from the hook context, which the verifier > > > > spills to the stack at program entry since R1 is clobbered during normal > > > > execution. > > > > > > > > A previous attempt [1] required a kmalloc string output protocol for > > > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to > > > > provide xattrs for inode_init_security hook") [2], the xattr name is no > > > > longer allocated; it is a static constant. We take advantage of this by > > > > passing the name directly. Because we rely on the hook-specific ctx > > > > layout, the kfunc is restricted to lsm/inode_init_security. > > > > > > > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1] > > > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2] > > > > Suggested-by: Song Liu <song@kernel.org> > > > > Signed-off-by: David Windsor <dwindsor@gmail.com> > > > > --- > > > > > > The explanation and code make no sense to me. Why not pass xattrs and > > > xattr_count directly as arguments, even if you choose to restrict the > > > kfunc to a specific hook? Why does the verifier core need the hack to > > > spill the context and extract the two arguments? > > > > > > > xattr_count is an output parameter; we cannot currently write to it in > > bpf as there is no verifier support for writing to int *. xattrs and > > xattr_count can be fixed up by the verifier, so we only require the > > user to pass the name and value. > > Sure, but the kfunc can. Did you try passing them in directly? > If that doesn't work for some reason, we should fix it instead. > Aha yes, I did try that and I believe the verifier rejected it because btf_ctx_access converts the pointer to a scalar. Will confirm the exact error tomorrow. > > > > > pw-bot: cr > > > > > > > [...] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 3:32 ` Kumar Kartikeya Dwivedi 2026-04-27 3:42 ` David Windsor @ 2026-04-27 10:11 ` Matt Bobrowski 2026-04-27 14:20 ` Song Liu 1 sibling, 1 reply; 18+ messages in thread From: Matt Bobrowski @ 2026-04-27 10:11 UTC (permalink / raw) To: Kumar Kartikeya Dwivedi Cc: David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh, Paul Moore, James Morris, Serge E. Hallyn, Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf, linux-security-module On Mon, Apr 27, 2026 at 05:32:47AM +0200, Kumar Kartikeya Dwivedi wrote: > On Mon, 27 Apr 2026 at 05:24, David Windsor <dwindsor@gmail.com> wrote: > > > > On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi > > <memxor@gmail.com> wrote: > > > > > > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote: > > > > > > > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set > > > > xattrs via inode_init_security hook using lsm_get_xattr_slot(). > > > > > > > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF > > > > programs cannot do: hook arguments are not directly writable from BPF. > > > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name, > > > > value), and the verifier transparently rewrites each call into > > > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and > > > > xattr_count are extracted from the hook context, which the verifier > > > > spills to the stack at program entry since R1 is clobbered during normal > > > > execution. > > > > > > > > A previous attempt [1] required a kmalloc string output protocol for > > > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to > > > > provide xattrs for inode_init_security hook") [2], the xattr name is no > > > > longer allocated; it is a static constant. We take advantage of this by > > > > passing the name directly. Because we rely on the hook-specific ctx > > > > layout, the kfunc is restricted to lsm/inode_init_security. > > > > > > > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1] > > > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2] > > > > Suggested-by: Song Liu <song@kernel.org> > > > > Signed-off-by: David Windsor <dwindsor@gmail.com> > > > > --- > > > > > > The explanation and code make no sense to me. Why not pass xattrs and > > > xattr_count directly as arguments, even if you choose to restrict the > > > kfunc to a specific hook? Why does the verifier core need the hack to > > > spill the context and extract the two arguments? > > > > > > > xattr_count is an output parameter; we cannot currently write to it in > > bpf as there is no verifier support for writing to int *. xattrs and > > xattr_count can be fixed up by the verifier, so we only require the > > user to pass the name and value. > > Sure, but the kfunc can. Did you try passing them in directly? > If that doesn't work for some reason, we should fix it instead. Hm, perhaps this fixup approach might be the simplest in order to assure the needed safety? Allowing this new BPF kfunc to take xattr and xattr_count arguments directly from the BPF LSM program could possibly result in lsm_get_xattr_slot() to go off the rails, no? Unless, what you're proposing here is to also add some provenance-like tracking such that the BPF verifier assures that only the BPF LSM program context arguments end up being supplied to it (if this is something that doesn't already exist)?. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 10:11 ` Matt Bobrowski @ 2026-04-27 14:20 ` Song Liu 2026-04-27 14:33 ` Kumar Kartikeya Dwivedi 0 siblings, 1 reply; 18+ messages in thread From: Song Liu @ 2026-04-27 14:20 UTC (permalink / raw) To: Matt Bobrowski Cc: Kumar Kartikeya Dwivedi, David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh, Paul Moore, James Morris, Serge E. Hallyn, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf, linux-security-module On Mon, Apr 27, 2026 at 11:11 AM Matt Bobrowski <mattbobrowski@google.com> wrote: > > On Mon, Apr 27, 2026 at 05:32:47AM +0200, Kumar Kartikeya Dwivedi wrote: > > On Mon, 27 Apr 2026 at 05:24, David Windsor <dwindsor@gmail.com> wrote: > > > > > > On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi > > > <memxor@gmail.com> wrote: > > > > > > > > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote: > > > > > > > > > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set > > > > > xattrs via inode_init_security hook using lsm_get_xattr_slot(). > > > > > > > > > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF > > > > > programs cannot do: hook arguments are not directly writable from BPF. > > > > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name, > > > > > value), and the verifier transparently rewrites each call into > > > > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and > > > > > xattr_count are extracted from the hook context, which the verifier > > > > > spills to the stack at program entry since R1 is clobbered during normal > > > > > execution. > > > > > > > > > > A previous attempt [1] required a kmalloc string output protocol for > > > > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to > > > > > provide xattrs for inode_init_security hook") [2], the xattr name is no > > > > > longer allocated; it is a static constant. We take advantage of this by > > > > > passing the name directly. Because we rely on the hook-specific ctx > > > > > layout, the kfunc is restricted to lsm/inode_init_security. > > > > > > > > > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1] > > > > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2] > > > > > Suggested-by: Song Liu <song@kernel.org> > > > > > Signed-off-by: David Windsor <dwindsor@gmail.com> > > > > > --- > > > > > > > > The explanation and code make no sense to me. Why not pass xattrs and > > > > xattr_count directly as arguments, even if you choose to restrict the > > > > kfunc to a specific hook? Why does the verifier core need the hack to > > > > spill the context and extract the two arguments? > > > > > > > > > > xattr_count is an output parameter; we cannot currently write to it in > > > bpf as there is no verifier support for writing to int *. xattrs and > > > xattr_count can be fixed up by the verifier, so we only require the > > > user to pass the name and value. > > > > Sure, but the kfunc can. Did you try passing them in directly? > > If that doesn't work for some reason, we should fix it instead. > > Hm, perhaps this fixup approach might be the simplest in order to > assure the needed safety? +1. I think this is the best approach I can think of. Thanks, Song [...] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 14:20 ` Song Liu @ 2026-04-27 14:33 ` Kumar Kartikeya Dwivedi 2026-04-27 17:17 ` Song Liu 2026-04-28 11:03 ` Matt Bobrowski 0 siblings, 2 replies; 18+ messages in thread From: Kumar Kartikeya Dwivedi @ 2026-04-27 14:33 UTC (permalink / raw) To: Song Liu Cc: Matt Bobrowski, David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh, Paul Moore, James Morris, Serge E. Hallyn, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf, linux-security-module On Mon, 27 Apr 2026 at 16:21, Song Liu <song@kernel.org> wrote: > > On Mon, Apr 27, 2026 at 11:11 AM Matt Bobrowski > <mattbobrowski@google.com> wrote: > > > > On Mon, Apr 27, 2026 at 05:32:47AM +0200, Kumar Kartikeya Dwivedi wrote: > > > On Mon, 27 Apr 2026 at 05:24, David Windsor <dwindsor@gmail.com> wrote: > > > > > > > > On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi > > > > <memxor@gmail.com> wrote: > > > > > > > > > > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote: > > > > > > > > > > > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set > > > > > > xattrs via inode_init_security hook using lsm_get_xattr_slot(). > > > > > > > > > > > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF > > > > > > programs cannot do: hook arguments are not directly writable from BPF. > > > > > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name, > > > > > > value), and the verifier transparently rewrites each call into > > > > > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and > > > > > > xattr_count are extracted from the hook context, which the verifier > > > > > > spills to the stack at program entry since R1 is clobbered during normal > > > > > > execution. > > > > > > > > > > > > A previous attempt [1] required a kmalloc string output protocol for > > > > > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to > > > > > > provide xattrs for inode_init_security hook") [2], the xattr name is no > > > > > > longer allocated; it is a static constant. We take advantage of this by > > > > > > passing the name directly. Because we rely on the hook-specific ctx > > > > > > layout, the kfunc is restricted to lsm/inode_init_security. > > > > > > > > > > > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1] > > > > > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2] > > > > > > Suggested-by: Song Liu <song@kernel.org> > > > > > > Signed-off-by: David Windsor <dwindsor@gmail.com> > > > > > > --- > > > > > > > > > > The explanation and code make no sense to me. Why not pass xattrs and > > > > > xattr_count directly as arguments, even if you choose to restrict the > > > > > kfunc to a specific hook? Why does the verifier core need the hack to > > > > > spill the context and extract the two arguments? > > > > > > > > > > > > > xattr_count is an output parameter; we cannot currently write to it in > > > > bpf as there is no verifier support for writing to int *. xattrs and > > > > xattr_count can be fixed up by the verifier, so we only require the > > > > user to pass the name and value. > > > > > > Sure, but the kfunc can. Did you try passing them in directly? > > > If that doesn't work for some reason, we should fix it instead. > > > > Hm, perhaps this fixup approach might be the simplest in order to > > assure the needed safety? > > +1. I think this is the best approach I can think of. We're not going to add more and more special cases to the verifier. The whole approach is unscalable. If the concern is that int xattr_count passed for xattrs can be unrelated int pointer obtained from elsewhere, can we pack the xattrs and xattr_count into a struct and pass it as an argument to the LSM? Then the pair struct can be passed in directly, ensuring both originate from the arguments passed to the LSM. That should eliminate concerns about either being out of sync if obtained from different sources. Even if we wanted to ensure argument provenance was stuff loaded from context, the right solution would be some kfunc flag that constraints the argument to be derived by following the ctx pointer, not whatever is done in this patch. > > Thanks, > Song > > [...] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 14:33 ` Kumar Kartikeya Dwivedi @ 2026-04-27 17:17 ` Song Liu 2026-04-28 11:03 ` Matt Bobrowski 1 sibling, 0 replies; 18+ messages in thread From: Song Liu @ 2026-04-27 17:17 UTC (permalink / raw) To: Kumar Kartikeya Dwivedi Cc: Matt Bobrowski, David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh, Paul Moore, James Morris, Serge E. Hallyn, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf, linux-security-module On Mon, Apr 27, 2026 at 3:33 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > On Mon, 27 Apr 2026 at 16:21, Song Liu <song@kernel.org> wrote: [...] > > > Hm, perhaps this fixup approach might be the simplest in order to > > > assure the needed safety? > > > > +1. I think this is the best approach I can think of. > > We're not going to add more and more special cases to the verifier. > The whole approach is unscalable. Agreed this is not scalable. One potential solution to this scalability issue is to move the fixup logic to struct btf_kfunc_id_set, so that this fixup logic is distributed. > If the concern is that int xattr_count passed for xattrs can be > unrelated int pointer obtained from elsewhere, can we pack the xattrs > and xattr_count into a struct and pass it as an argument to the LSM? > Then the pair struct can be passed in directly, ensuring both > originate from the arguments passed to the LSM. That should eliminate > concerns about either being out of sync if obtained from different > sources. I think a trusted pointer of the pair struct will also work. But this means we need to refactor the LSM hook and other LSMs. The refactoring is not difficult though. > Even if we wanted to ensure argument provenance was stuff loaded from > context, the right solution would be some kfunc flag that constraints > the argument to be derived by following the ctx pointer, not whatever > is done in this patch. We need these two arguments to be the specific fields in the ctx. I am not sure how to do this with kfunc flags. Thanks, Song ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 14:33 ` Kumar Kartikeya Dwivedi 2026-04-27 17:17 ` Song Liu @ 2026-04-28 11:03 ` Matt Bobrowski 2026-04-28 16:33 ` Kumar Kartikeya Dwivedi 1 sibling, 1 reply; 18+ messages in thread From: Matt Bobrowski @ 2026-04-28 11:03 UTC (permalink / raw) To: Kumar Kartikeya Dwivedi Cc: Song Liu, David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh, Paul Moore, James Morris, Serge E. Hallyn, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf, linux-security-module On Mon, Apr 27, 2026 at 04:33:18PM +0200, Kumar Kartikeya Dwivedi wrote: > On Mon, 27 Apr 2026 at 16:21, Song Liu <song@kernel.org> wrote: > > > > On Mon, Apr 27, 2026 at 11:11 AM Matt Bobrowski > > <mattbobrowski@google.com> wrote: n> > > > > > On Mon, Apr 27, 2026 at 05:32:47AM +0200, Kumar Kartikeya Dwivedi wrote: > > > > On Mon, 27 Apr 2026 at 05:24, David Windsor <dwindsor@gmail.com> wrote: > > > > > > > > > > On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi > > > > > <memxor@gmail.com> wrote: > > > > > > > > > > > > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote: > > > > > > > > > > > > > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set > > > > > > > xattrs via inode_init_security hook using lsm_get_xattr_slot(). > > > > > > > > > > > > > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF > > > > > > > programs cannot do: hook arguments are not directly writable from BPF. > > > > > > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name, > > > > > > > value), and the verifier transparently rewrites each call into > > > > > > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and > > > > > > > xattr_count are extracted from the hook context, which the verifier > > > > > > > spills to the stack at program entry since R1 is clobbered during normal > > > > > > > execution. > > > > > > > > > > > > > > A previous attempt [1] required a kmalloc string output protocol for > > > > > > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to > > > > > > > provide xattrs for inode_init_security hook") [2], the xattr name is no > > > > > > > longer allocated; it is a static constant. We take advantage of this by > > > > > > > passing the name directly. Because we rely on the hook-specific ctx > > > > > > > layout, the kfunc is restricted to lsm/inode_init_security. > > > > > > > > > > > > > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1] > > > > > > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2] > > > > > > > Suggested-by: Song Liu <song@kernel.org> > > > > > > > Signed-off-by: David Windsor <dwindsor@gmail.com> > > > > > > > --- > > > > > > > > > > > > The explanation and code make no sense to me. Why not pass xattrs and > > > > > > xattr_count directly as arguments, even if you choose to restrict the > > > > > > kfunc to a specific hook? Why does the verifier core need the hack to > > > > > > spill the context and extract the two arguments? > > > > > > > > > > > > > > > > xattr_count is an output parameter; we cannot currently write to it in > > > > > bpf as there is no verifier support for writing to int *. xattrs and > > > > > xattr_count can be fixed up by the verifier, so we only require the > > > > > user to pass the name and value. > > > > > > > > Sure, but the kfunc can. Did you try passing them in directly? > > > > If that doesn't work for some reason, we should fix it instead. > > > > > > Hm, perhaps this fixup approach might be the simplest in order to > > > assure the needed safety? > > > > +1. I think this is the best approach I can think of. > > We're not going to add more and more special cases to the verifier. > The whole approach is unscalable. Totally fair of you to push back here. I'm also agreement with you on the fact that extending the BPF verifier with such special casing doesn't scale all that well. > If the concern is that int xattr_count passed for xattrs can be > unrelated int pointer obtained from elsewhere, can we pack the xattrs > and xattr_count into a struct and pass it as an argument to the LSM? > Then the pair struct can be passed in directly, ensuring both > originate from the arguments passed to the LSM. That should eliminate > concerns about either being out of sync if obtained from different > sources. This could work, but we'd also need to modify all the other pre-existing hook implementations along with the core security_inode_init_security() LSM hook itself. I don't think that'd be an issue. The biggest hurdle here I think would be convincing the LSM maintainers themselves. > Even if we wanted to ensure argument provenance was stuff loaded from > context, the right solution would be some kfunc flag that constraints > the argument to be derived by following the ctx pointer, not whatever > is done in this patch. OK, so it is provenance-like tracking which you were initially kinda alluding to here. Currently, I don't believe that PTR_TO_CTX is preserved upon any subsequent R1 (ctx) dereferences, so we'd need to think about how this type could be preserved such that we can enforce this kinda constraint (__ctx) at the time which the new BPF kfunc is called. Do you have any ideas on how to do this? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-28 11:03 ` Matt Bobrowski @ 2026-04-28 16:33 ` Kumar Kartikeya Dwivedi 0 siblings, 0 replies; 18+ messages in thread From: Kumar Kartikeya Dwivedi @ 2026-04-28 16:33 UTC (permalink / raw) To: Matt Bobrowski Cc: Song Liu, David Windsor, Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh, Paul Moore, James Morris, Serge E. Hallyn, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf, linux-security-module On Tue, 28 Apr 2026 at 13:03, Matt Bobrowski <mattbobrowski@google.com> wrote: > > On Mon, Apr 27, 2026 at 04:33:18PM +0200, Kumar Kartikeya Dwivedi wrote: > > On Mon, 27 Apr 2026 at 16:21, Song Liu <song@kernel.org> wrote: > > > > > > On Mon, Apr 27, 2026 at 11:11 AM Matt Bobrowski > > > <mattbobrowski@google.com> wrote: > n> > > > > > > On Mon, Apr 27, 2026 at 05:32:47AM +0200, Kumar Kartikeya Dwivedi wrote: > > > > > On Mon, 27 Apr 2026 at 05:24, David Windsor <dwindsor@gmail.com> wrote: > > > > > > > > > > > > On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi > > > > > > <memxor@gmail.com> wrote: > > > > > > > > > > > > > > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote: > > > > > > > > > > > > > > > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set > > > > > > > > xattrs via inode_init_security hook using lsm_get_xattr_slot(). > > > > > > > > > > > > > > > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF > > > > > > > > programs cannot do: hook arguments are not directly writable from BPF. > > > > > > > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name, > > > > > > > > value), and the verifier transparently rewrites each call into > > > > > > > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and > > > > > > > > xattr_count are extracted from the hook context, which the verifier > > > > > > > > spills to the stack at program entry since R1 is clobbered during normal > > > > > > > > execution. > > > > > > > > > > > > > > > > A previous attempt [1] required a kmalloc string output protocol for > > > > > > > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to > > > > > > > > provide xattrs for inode_init_security hook") [2], the xattr name is no > > > > > > > > longer allocated; it is a static constant. We take advantage of this by > > > > > > > > passing the name directly. Because we rely on the hook-specific ctx > > > > > > > > layout, the kfunc is restricted to lsm/inode_init_security. > > > > > > > > > > > > > > > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1] > > > > > > > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2] > > > > > > > > Suggested-by: Song Liu <song@kernel.org> > > > > > > > > Signed-off-by: David Windsor <dwindsor@gmail.com> > > > > > > > > --- > > > > > > > > > > > > > > The explanation and code make no sense to me. Why not pass xattrs and > > > > > > > xattr_count directly as arguments, even if you choose to restrict the > > > > > > > kfunc to a specific hook? Why does the verifier core need the hack to > > > > > > > spill the context and extract the two arguments? > > > > > > > > > > > > > > > > > > > xattr_count is an output parameter; we cannot currently write to it in > > > > > > bpf as there is no verifier support for writing to int *. xattrs and > > > > > > xattr_count can be fixed up by the verifier, so we only require the > > > > > > user to pass the name and value. > > > > > > > > > > Sure, but the kfunc can. Did you try passing them in directly? > > > > > If that doesn't work for some reason, we should fix it instead. > > > > > > > > Hm, perhaps this fixup approach might be the simplest in order to > > > > assure the needed safety? > > > > > > +1. I think this is the best approach I can think of. > > > > We're not going to add more and more special cases to the verifier. > > The whole approach is unscalable. > > Totally fair of you to push back here. I'm also agreement with you on > the fact that extending the BPF verifier with such special casing > doesn't scale all that well. > > > If the concern is that int xattr_count passed for xattrs can be > > unrelated int pointer obtained from elsewhere, can we pack the xattrs > > and xattr_count into a struct and pass it as an argument to the LSM? > > Then the pair struct can be passed in directly, ensuring both > > originate from the arguments passed to the LSM. That should eliminate > > concerns about either being out of sync if obtained from different > > sources. > > This could work, but we'd also need to modify all the other > pre-existing hook implementations along with the core > security_inode_init_security() LSM hook itself. I don't think that'd > be an issue. The biggest hurdle here I think would be convincing the > LSM maintainers themselves. Yeah, when these parameters were introduced, we changed all LSMs, so I don't see why we cannot adjust things again to benefit this use case. > > > Even if we wanted to ensure argument provenance was stuff loaded from > > context, the right solution would be some kfunc flag that constraints > > the argument to be derived by following the ctx pointer, not whatever > > is done in this patch. > > OK, so it is provenance-like tracking which you were initially kinda > alluding to here. Currently, I don't believe that PTR_TO_CTX is > preserved upon any subsequent R1 (ctx) dereferences, so we'd need to > think about how this type could be preserved such that we can enforce > this kinda constraint (__ctx) at the time which the new BPF kfunc is > called. Do you have any ideas on how to do this? I think we'll have to track in the register whether the PTR_TO_BTF_ID came from a PTR_TO_CTX load. That said, I still prefer changing the prototype to pack the array and its output size parameter together. It is even clearer to have a well named type than int *xattr_count in the prototype. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling 2026-04-27 0:15 ` [PATCH bpf-next 1/2] " David Windsor ` (2 preceding siblings ...) 2026-04-27 2:56 ` Kumar Kartikeya Dwivedi @ 2026-04-27 9:48 ` Matt Bobrowski 3 siblings, 0 replies; 18+ messages in thread From: Matt Bobrowski @ 2026-04-27 9:48 UTC (permalink / raw) To: David Windsor Cc: Alexander Viro, Christian Brauner, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, KP Singh, Paul Moore, James Morris, Serge E. Hallyn, Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf, linux-security-module On Sun, Apr 26, 2026 at 08:15:57PM -0400, David Windsor wrote: > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set > xattrs via inode_init_security hook using lsm_get_xattr_slot(). > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF > programs cannot do: hook arguments are not directly writable from BPF. > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name, > value), and the verifier transparently rewrites each call into > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and > xattr_count are extracted from the hook context, which the verifier > spills to the stack at program entry since R1 is clobbered during normal > execution. > > A previous attempt [1] required a kmalloc string output protocol for > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to > provide xattrs for inode_init_security hook") [2], the xattr name is no > longer allocated; it is a static constant. We take advantage of this by > passing the name directly. Because we rely on the hook-specific ctx > layout, the kfunc is restricted to lsm/inode_init_security. > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1] > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2] > Suggested-by: Song Liu <song@kernel.org> > Signed-off-by: David Windsor <dwindsor@gmail.com> > --- > fs/bpf_fs_kfuncs.c | 80 +++++++++++++++++++++++++++++++++++- > include/linux/bpf_verifier.h | 3 ++ > kernel/bpf/fixups.c | 20 +++++++++ > kernel/bpf/verifier.c | 54 ++++++++++++++++++++++++ > security/bpf/hooks.c | 3 ++ > 5 files changed, 159 insertions(+), 1 deletion(-) > > diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c > index 9d27be058494..5a5951006a3f 100644 > --- a/fs/bpf_fs_kfuncs.c > +++ b/fs/bpf_fs_kfuncs.c > @@ -10,6 +10,7 @@ > #include <linux/fsnotify.h> > #include <linux/file.h> > #include <linux/kernfs.h> > +#include <linux/lsm_hooks.h> > #include <linux/mm.h> > #include <linux/xattr.h> > > @@ -353,6 +354,68 @@ __bpf_kfunc int bpf_cgroup_read_xattr(struct cgroup *cgroup, const char *name__s > } > #endif /* CONFIG_CGROUPS */ > > +/* Called from the verifier fixup of bpf_init_inode_xattr(). */ > +__bpf_kfunc int bpf_init_inode_xattr_impl(struct xattr *xattrs, int *xattr_count, > + const char *name__str, > + const struct bpf_dynptr *value_p) > +{ > + struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p; > + size_t name_len; > + void *xattr_value; > + struct xattr *xattr; > + const void *value; > + u32 value_len; > + > + if (!xattrs || !xattr_count || !name__str) > + return -EINVAL; > + > + name_len = strlen(name__str); > + if (name_len == 0 || name_len > XATTR_NAME_MAX) > + return -EINVAL; > + > + value_len = __bpf_dynptr_size(value_ptr); > + if (value_len == 0 || value_len > XATTR_SIZE_MAX) > + return -EINVAL; > + > + value = __bpf_dynptr_data(value_ptr, value_len); > + if (!value) > + return -EINVAL; > + > + xattr_value = kmemdup(value, value_len, GFP_ATOMIC); > + if (!xattr_value) > + return -ENOMEM; > + > + xattr = lsm_get_xattr_slot(xattrs, xattr_count); > + if (!xattr) { > + kfree(xattr_value); > + return -ENOSPC; > + } I think you should also include the following check: if (!match_security_bpf_prefix(name__str)) return -EPERM; This will ensure that some namespace isolation is provided and make the behavior of this initialization based BPF kfunc consistent with the pre-existing runtime xattr-related modification BPF kfuncs (e.g., bpf_set_dentry_xattr()). > + xattr->name = name__str; > + xattr->value = xattr_value; > + xattr->value_len = value_len; > + > + return 0; > +} > + > +/** > + * bpf_init_inode_xattr - set an xattr on a new inode from inode_init_security > + * @name__str: xattr name (e.g., "bpf.file_label") > + * @value_p: dynptr containing the xattr value > + * > + * Only callable from lsm/inode_init_security programs. The verifier rewrites > + * calls to bpf_init_inode_xattr_impl() with xattrs/xattr_count extracted from > + * the hook context. > + * > + * Return: 0 on success, negative error on failure. > + */ > +__bpf_kfunc int bpf_init_inode_xattr(const char *name__str, > + const struct bpf_dynptr *value_p) > +{ > + WARN_ONCE(1, "%s called without verifier fixup\n", __func__); > + return -EFAULT; > +} > + > __bpf_kfunc_end_defs(); > > BTF_KFUNCS_START(bpf_fs_kfunc_set_ids) > @@ -363,13 +426,28 @@ BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE) > BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE) > BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE) > BTF_ID_FLAGS(func, bpf_remove_dentry_xattr, KF_SLEEPABLE) > +BTF_ID_FLAGS(func, bpf_init_inode_xattr) > +BTF_ID_FLAGS(func, bpf_init_inode_xattr_impl) > BTF_KFUNCS_END(bpf_fs_kfunc_set_ids) > > +BTF_ID_LIST(bpf_lsm_inode_init_security_btf_ids) > +BTF_ID(func, bpf_lsm_inode_init_security) > + > +BTF_ID_LIST(bpf_init_inode_xattr_btf_ids) > +BTF_ID(func, bpf_init_inode_xattr) > +BTF_ID(func, bpf_init_inode_xattr_impl) > + > static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id) > { > if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) || > - prog->type == BPF_PROG_TYPE_LSM) > + prog->type == BPF_PROG_TYPE_LSM) { > + /* bpf_init_inode_xattr[_impl] only attach to inode_init_security. */ > + if ((kfunc_id == bpf_init_inode_xattr_btf_ids[0] || > + kfunc_id == bpf_init_inode_xattr_btf_ids[1]) && > + prog->aux->attach_btf_id != bpf_lsm_inode_init_security_btf_ids[0]) > + return -EACCES; > return 0; > + } > return -EACCES; > } > > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h > index 101ca6cc5424..e73bb2222c3d 100644 > --- a/include/linux/bpf_verifier.h > +++ b/include/linux/bpf_verifier.h > @@ -682,6 +682,7 @@ struct bpf_insn_aux_data { > */ > u8 fastcall_spills_num:3; > u8 arg_prog:4; > + u8 init_inode_xattr_fixup:1; > > /* below fields are initialized once */ > unsigned int orig_idx; /* original instruction index */ > @@ -903,6 +904,8 @@ struct bpf_verifier_env { > bool bypass_spec_v4; > bool seen_direct_write; > bool seen_exception; > + bool needs_ctx_spill; > + s16 ctx_stack_off; > 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/fixups.c b/kernel/bpf/fixups.c > index fba9e8c00878..18d612a9fe29 100644 > --- a/kernel/bpf/fixups.c > +++ b/kernel/bpf/fixups.c > @@ -725,6 +725,26 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env) > } > } > > + if (env->needs_ctx_spill) { > + if (epilogue_cnt) { > + /* gen_epilogue already saved ctx to the stack */ > + env->ctx_stack_off = -(s16)subprogs[0].stack_depth; > + } else { > + cnt = 0; > + subprogs[0].stack_depth += 8; > + env->ctx_stack_off = -(s16)subprogs[0].stack_depth; > + insn_buf[cnt++] = BPF_STX_MEM(BPF_DW, BPF_REG_FP, > + BPF_REG_1, > + env->ctx_stack_off); > + insn_buf[cnt++] = env->prog->insnsi[0]; > + new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt); > + if (!new_prog) > + return -ENOMEM; > + env->prog = new_prog; > + delta += cnt - 1; > + } > + } > + > if (ops->gen_prologue || env->seen_direct_write) { > if (!ops->gen_prologue) { > verifier_bug(env, "gen_prologue is null"); > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 03f9e16c2abe..af5753ffb16b 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -10794,6 +10794,8 @@ enum special_kfunc_type { > KF_bpf_arena_alloc_pages, > KF_bpf_arena_free_pages, > KF_bpf_arena_reserve_pages, > + KF_bpf_init_inode_xattr, > + KF_bpf_init_inode_xattr_impl, > KF_bpf_session_is_return, > KF_bpf_stream_vprintk, > KF_bpf_stream_print_stack, > @@ -10882,6 +10884,13 @@ BTF_ID(func, bpf_task_work_schedule_resume) > BTF_ID(func, bpf_arena_alloc_pages) > BTF_ID(func, bpf_arena_free_pages) > BTF_ID(func, bpf_arena_reserve_pages) > +#ifdef CONFIG_BPF_LSM > +BTF_ID(func, bpf_init_inode_xattr) > +BTF_ID(func, bpf_init_inode_xattr_impl) > +#else > +BTF_ID_UNUSED > +BTF_ID_UNUSED > +#endif > BTF_ID(func, bpf_session_is_return) > BTF_ID(func, bpf_stream_vprintk) > BTF_ID(func, bpf_stream_print_stack) > @@ -12701,6 +12710,24 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, > if (err < 0) > return err; > > + if (meta.func_id == special_kfunc_list[KF_bpf_init_inode_xattr_impl]) { > + verbose(env, "bpf_init_inode_xattr_impl is not callable directly\n"); > + return -EACCES; > + } > + > + if (meta.func_id == special_kfunc_list[KF_bpf_init_inode_xattr]) { > + if (env->cur_state->curframe != 0) { > + verbose(env, "bpf_init_inode_xattr cannot be called from subprograms\n"); > + return -EINVAL; > + } > + env->needs_ctx_spill = true; > + insn_aux->init_inode_xattr_fixup = true; > + err = bpf_add_kfunc_call(env, > + special_kfunc_list[KF_bpf_init_inode_xattr_impl], 0); > + if (err < 0) > + return err; > + } > + > if (is_bpf_rbtree_add_kfunc(meta.func_id)) { > err = push_callback_call(env, insn, insn_idx, meta.subprogno, > set_rbtree_add_callback_state); > @@ -19272,6 +19299,33 @@ int bpf_fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, > insn_buf[4] = BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1); > insn_buf[5] = BPF_ALU64_IMM(BPF_NEG, BPF_REG_0, 0); > *cnt = 6; > + } else if (env->insn_aux_data[insn_idx].init_inode_xattr_fixup) { > + struct bpf_kfunc_desc *impl_desc; > + > + impl_desc = find_kfunc_desc(env->prog, > + special_kfunc_list[KF_bpf_init_inode_xattr_impl], 0); > + if (!impl_desc) { > + verifier_bug(env, "bpf_init_inode_xattr_impl desc not found"); > + return -EFAULT; > + } > + > + /* Rewrite bpf_init_inode_xattr(name, value) to inject xattrs and > + * xattr_count loaded from the saved inode_init_security ctx. > + */ > + insn_buf[0] = BPF_MOV64_REG(BPF_REG_3, BPF_REG_1); > + insn_buf[1] = BPF_MOV64_REG(BPF_REG_4, BPF_REG_2); > + insn_buf[2] = BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_FP, > + env->ctx_stack_off); > + insn_buf[3] = BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_2, > + 3 * sizeof(u64)); > + insn_buf[4] = BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_2, > + 4 * sizeof(u64)); > + insn_buf[5] = *insn; > + if (!bpf_jit_supports_far_kfunc_call()) > + insn_buf[5].imm = BPF_CALL_IMM(impl_desc->addr); > + else > + insn_buf[5].imm = impl_desc->func_id; > + *cnt = 6; > } > > if (env->insn_aux_data[insn_idx].arg_prog) { > diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c > index 40efde233f3a..1e61baa821bd 100644 > --- a/security/bpf/hooks.c > +++ b/security/bpf/hooks.c > @@ -28,8 +28,11 @@ static int __init bpf_lsm_init(void) > return 0; > } > > +#define BPF_LSM_INODE_INIT_XATTRS 1 > + > struct lsm_blob_sizes bpf_lsm_blob_sizes __ro_after_init = { > .lbs_inode = sizeof(struct bpf_storage_blob), > + .lbs_xattr_count = BPF_LSM_INODE_INIT_XATTRS, > }; > > DEFINE_LSM(bpf) = { > -- > 2.53.0 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH bpf-next 2/2] selftests/bpf: add tests for bpf_init_inode_xattr kfunc 2026-04-27 0:15 [PATCH bpf-next 0/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling David Windsor 2026-04-27 0:15 ` [PATCH bpf-next 1/2] " David Windsor @ 2026-04-27 0:15 ` David Windsor 2026-04-27 1:10 ` sashiko-bot 1 sibling, 1 reply; 18+ messages in thread From: David Windsor @ 2026-04-27 0:15 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, Shuah Khan Cc: David Windsor, Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa, linux-kernel, bpf, linux-kselftest Test bpf atomic inode xattr labeling in inode_init_security. Signed-off-by: David Windsor <dwindsor@gmail.com> --- tools/testing/selftests/bpf/bpf_kfuncs.h | 3 ++ .../selftests/bpf/prog_tests/fs_kfuncs.c | 49 +++++++++++++++++++ .../bpf/progs/test_init_inode_xattr.c | 32 ++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/test_init_inode_xattr.c diff --git a/tools/testing/selftests/bpf/bpf_kfuncs.h b/tools/testing/selftests/bpf/bpf_kfuncs.h index ae71e9b69051..15507a406266 100644 --- a/tools/testing/selftests/bpf/bpf_kfuncs.h +++ b/tools/testing/selftests/bpf/bpf_kfuncs.h @@ -92,4 +92,7 @@ extern int bpf_set_dentry_xattr(struct dentry *dentry, const char *name__str, const struct bpf_dynptr *value_p, int flags) __ksym __weak; extern int bpf_remove_dentry_xattr(struct dentry *dentry, const char *name__str) __ksym __weak; +extern int bpf_init_inode_xattr(const char *name__str, + const struct bpf_dynptr *value_p) __ksym __weak; + #endif diff --git a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c index 43a26ec69a8e..26daef116ee2 100644 --- a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c +++ b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c @@ -9,6 +9,7 @@ #include <test_progs.h> #include "test_get_xattr.skel.h" #include "test_set_remove_xattr.skel.h" +#include "test_init_inode_xattr.skel.h" #include "test_fsverity.skel.h" static const char testfile[] = "/tmp/test_progs_fs_kfuncs"; @@ -268,6 +269,51 @@ static void test_fsverity(void) remove(testfile); } +static void test_init_inode_xattr(void) +{ + struct test_init_inode_xattr *skel = NULL; + int fd = -1, err; + char value_out[32]; + const char *testfile_new = "/tmp/test_progs_fs_kfuncs_new"; + + skel = test_init_inode_xattr__open_and_load(); + if (!ASSERT_OK_PTR(skel, "test_init_inode_xattr__open_and_load")) + return; + + skel->bss->monitored_pid = getpid(); + err = test_init_inode_xattr__attach(skel); + if (!ASSERT_OK(err, "test_init_inode_xattr__attach")) + goto out; + + /* Create a new file — this triggers inode_init_security */ + fd = open(testfile_new, O_CREAT | O_RDWR, 0644); + if (!ASSERT_GE(fd, 0, "create_file")) + goto out; + + ASSERT_EQ(skel->data->init_result, 0, "init_result"); + + /* The initxattrs callback prepends "security." to the name */ + err = getxattr(testfile_new, "security.bpf.test_label", value_out, + sizeof(value_out)); + if (err < 0 && errno == ENODATA) { + printf("%s:SKIP:filesystem did not apply LSM xattrs\n", + __func__); + test__skip(); + goto out; + } + if (!ASSERT_GE(err, 0, "getxattr")) + goto out; + + ASSERT_EQ(err, (int)sizeof(skel->data->xattr_value), "xattr_size"); + ASSERT_EQ(strncmp(value_out, "test_value", + sizeof("test_value")), 0, "xattr_value"); + +out: + close(fd); + test_init_inode_xattr__destroy(skel); + remove(testfile_new); +} + void test_fs_kfuncs(void) { /* Matches xattr_names in progs/test_get_xattr.c */ @@ -286,6 +332,9 @@ void test_fs_kfuncs(void) if (test__start_subtest("set_remove_xattr")) test_set_remove_xattr(); + if (test__start_subtest("init_inode_xattr")) + test_init_inode_xattr(); + if (test__start_subtest("fsverity")) test_fsverity(); } diff --git a/tools/testing/selftests/bpf/progs/test_init_inode_xattr.c b/tools/testing/selftests/bpf/progs/test_init_inode_xattr.c new file mode 100644 index 000000000000..450798893566 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_init_inode_xattr.c @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2025 Isovalent, a Cisco company. */ + +#include "vmlinux.h" +#include <errno.h> +#include <bpf/bpf_tracing.h> +#include "bpf_kfuncs.h" + +char _license[] SEC("license") = "GPL"; + +__u32 monitored_pid; +int init_result = -1; + +const char xattr_name[] = "bpf.test_label"; +char xattr_value[] = "test_value"; + +SEC("lsm/inode_init_security") +int BPF_PROG(test_init_inode_xattr, struct inode *inode, struct inode *dir, + const struct qstr *qstr, struct xattr *xattrs, int *xattr_count) +{ + struct bpf_dynptr value_ptr; + __u32 pid; + + pid = bpf_get_current_pid_tgid() >> 32; + if (pid != monitored_pid) + return 0; + + bpf_dynptr_from_mem(xattr_value, sizeof(xattr_value), 0, &value_ptr); + init_result = bpf_init_inode_xattr(xattr_name, &value_ptr); + + return 0; +} -- 2.53.0 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: add tests for bpf_init_inode_xattr kfunc 2026-04-27 0:15 ` [PATCH bpf-next 2/2] selftests/bpf: add tests for bpf_init_inode_xattr kfunc David Windsor @ 2026-04-27 1:10 ` sashiko-bot 0 siblings, 0 replies; 18+ messages in thread From: sashiko-bot @ 2026-04-27 1:10 UTC (permalink / raw) To: David Windsor; +Cc: bpf Sashiko AI review found 5 potential issue(s): - [Medium] Test flakiness due to missing file removal before open(O_CREAT). - [Medium] Test fails instead of gracefully skipping on filesystems without xattr support. - [Low] Uninitialized stack memory read if getxattr returns an unexpected length. - [Low] Insecure temporary file creation in world-writable directory. - [Low] Closing uninitialized file descriptor on error paths. -- commit b0095935131688a0b976829cf24db472a2eab938 Author: David Windsor <dwindsor@gmail.com> selftests/bpf: add tests for bpf_init_inode_xattr kfunc This commit adds selftests for the bpf_init_inode_xattr kfunc to test atomic inode xattr labeling via the inode_init_security hook. > diff --git a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c > index 43a26ec69a8e..26daef116ee2 100644 > --- a/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c > +++ b/tools/testing/selftests/bpf/prog_tests/fs_kfuncs.c [ ... ] > @@ -268,6 +269,51 @@ static void test_fsverity(void) > remove(testfile); > } > > +static void test_init_inode_xattr(void) > +{ > + struct test_init_inode_xattr *skel = NULL; > + int fd = -1, err; > + char value_out[32]; Could value_out be read while uninitialized? If getxattr() succeeds but returns fewer bytes than expected, the size ASSERT_EQ check will fail but execution will continue. The subsequent strncmp() might then read uninitialized stack memory. Would it be better to zero-initialize this buffer, or use if (!ASSERT_EQ(...)) to abort the test early? > + const char *testfile_new = "/tmp/test_progs_fs_kfuncs_new"; > + > + skel = test_init_inode_xattr__open_and_load(); > + if (!ASSERT_OK_PTR(skel, "test_init_inode_xattr__open_and_load")) > + return; > + > + skel->bss->monitored_pid = getpid(); > + err = test_init_inode_xattr__attach(skel); > + if (!ASSERT_OK(err, "test_init_inode_xattr__attach")) > + goto out; > + > + /* Create a new file — this triggers inode_init_security */ > + fd = open(testfile_new, O_CREAT | O_RDWR, 0644); If this test file already exists from a previous failed run, will open() just open the existing file without triggering the inode_init_security hook? Since the hook only fires when a new inode is instantiated, this would cause the test to persistently fail. Also, since this runs as root in the world-writable /tmp directory, could this predictable path be vulnerable to a symlink attack? Would adding O_EXCL to the open flags ensure both that the hook is triggered and prevent symlink issues? > + if (!ASSERT_GE(fd, 0, "create_file")) > + goto out; > + > + ASSERT_EQ(skel->data->init_result, 0, "init_result"); > + > + /* The initxattrs callback prepends "security." to the name */ > + err = getxattr(testfile_new, "security.bpf.test_label", value_out, > + sizeof(value_out)); > + if (err < 0 && errno == ENODATA) { If the underlying /tmp filesystem does not support extended attributes at all, getxattr() returns -1 with errno set to EOPNOTSUPP (or ENOTSUP). Should this condition also be handled to correctly skip the test instead of failing on the next ASSERT_GE check? > + printf("%s:SKIP:filesystem did not apply LSM xattrs\n", > + __func__); > + test__skip(); > + goto out; > + } > + if (!ASSERT_GE(err, 0, "getxattr")) > + goto out; > + > + ASSERT_EQ(err, (int)sizeof(skel->data->xattr_value), "xattr_size"); > + ASSERT_EQ(strncmp(value_out, "test_value", > + sizeof("test_value")), 0, "xattr_value"); > + > +out: > + close(fd); If an error occurs before the file is opened (e.g. during skeleton load or attach), fd will be -1. Does this result in an unnecessary close(-1) which returns -EBADF? Should this be guarded with if (fd >= 0)? > + test_init_inode_xattr__destroy(skel); > + remove(testfile_new); > +} -- Sashiko AI review · https://sashiko.dev/#/patchset/20260427001602.38353-1-dwindsor@gmail.com?part=2 ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-04-28 16:33 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-27 0:15 [PATCH bpf-next 0/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling David Windsor 2026-04-27 0:15 ` [PATCH bpf-next 1/2] " David Windsor 2026-04-27 0:51 ` bot+bpf-ci 2026-04-27 0:56 ` sashiko-bot 2026-04-27 2:10 ` David Windsor 2026-04-27 2:56 ` Kumar Kartikeya Dwivedi 2026-04-27 3:23 ` David Windsor 2026-04-27 3:32 ` Kumar Kartikeya Dwivedi 2026-04-27 3:42 ` David Windsor 2026-04-27 10:11 ` Matt Bobrowski 2026-04-27 14:20 ` Song Liu 2026-04-27 14:33 ` Kumar Kartikeya Dwivedi 2026-04-27 17:17 ` Song Liu 2026-04-28 11:03 ` Matt Bobrowski 2026-04-28 16:33 ` Kumar Kartikeya Dwivedi 2026-04-27 9:48 ` Matt Bobrowski 2026-04-27 0:15 ` [PATCH bpf-next 2/2] selftests/bpf: add tests for bpf_init_inode_xattr kfunc David Windsor 2026-04-27 1:10 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox