* Re: [PATCH v3 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: David Windsor @ 2026-06-24 21:22 UTC (permalink / raw)
To: Paul Moore
Cc: viro, brauner, jack, ast, daniel, john.fastabend, andrii, eddyz87,
memxor, martin.lau, song, yonghong.song, jolsa, emil, kpsingh,
mattbobrowski, jmorris, serge, zohar, roberto.sassu,
dmitry.kasatkin, eric.snowberg, stephen.smalley.work, omosnace,
casey, shuah, linux-kernel, linux-fsdevel, bpf,
linux-security-module, linux-integrity, selinux, linux-kselftest
In-Reply-To: <75d39fd9847cca915d704235264ab474@paul-moore.com>
On Tue, Jun 23, 2026 at 8:12 PM Paul Moore <paul@paul-moore.com> wrote:
>
> I have a few specific comments below, inline with the patch, but I wanted
> to make some general comments too.
>
> The kfunc additions really don't belong in the VFS kfunc file, please
> create a LSM kfunc file (call it security/bpf_lsm_kfuncs.c) and add the
> kfunc code to this new file.
>
Makes sense. I will also do similarly for the selftests.
> While moving the kfunc additions to a LSM kfunc file does sort of convert
> the VFS changes into LSM changes, Christian's comment about splitting
> this patch two patches, one with the LSM hook changes and one with the
> BPF additions, is still reasonable and a good suggestion. I would still
> CC the VFS folks on these patches and I would encourage reviews from the
> VFS folks as there is a VFS component here, albeit a somewhat small one.
>
v4 will contain 3 patches: one to introduce struct lsm_xattrs, next to
introduce the kfunc, and finally the selftests.
> > +
> > +static int __bpf_init_inode_xattr(struct xattr_ctx *xattr_ctx,
> > + 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;
> > + struct xattr *xattrs;
> > + int *xattr_count;
> > + const void *value;
> > + u32 value_len;
> > +
> > + if (!xattr_ctx || !name__str)
> > + return -EINVAL;
> > +
> > + xattrs = xattr_ctx->xattrs;
> > + xattr_count = xattr_ctx->xattr_count;
>
> I'm not sure why the "xattrs" and "xattrs_count" local variables are
> necessary, especially since they only appear to be used in the sanity
> check below. I would suggest just using the values in the struct
> directly.
>
Leftover from the previous implementation, will fix.
> > +/**
> > + * bpf_init_inode_xattr - set an xattr on a new inode from inode_init_security
> > + * @xattr_ctx: inode_init_security xattr state from the hook context
> > + * @name__str: xattr name (e.g., "bpf.file_label")
> > + * @value_p: dynptr containing the xattr value
> > + *
> > + * Only callable from lsm/inode_init_security programs.
> > + *
> > + * Return: 0 on success, negative error on failure.
> > + */
> > +__bpf_kfunc int bpf_init_inode_xattr(struct xattr_ctx *xattr_ctx,
> > + const char *name__str,
> > + const struct bpf_dynptr *value_p)
> > +{
> > + return __bpf_init_inode_xattr(xattr_ctx, name__str, value_p);
> > +}
>
> I'm sure there is a reason why you split the code out into
> __bpf_init_inode_xattr() as opposed to just putting it directly in this
> kfunc, can you help me understand?
Not sure, perhaps prior convention, or something with the verifier
perhaps. I've removed it in -v4 and everything works.
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index 153e9043058f..1f8e84e7dd7e 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -68,6 +68,11 @@ struct watch;
> > struct watch_notification;
> > struct lsm_ctx;
> >
> > +struct xattr_ctx {
> > + struct xattr *xattrs;
> > + int *xattr_count;
> > +};
>
> I see the bots already pointed this out, and you acknowledged it, but
> since I'm looking at this I felt obliged to remind you once again about
> the rename to "struct lsm_xattrs" :)
>
Yeah, sorry about that. =)
> Also, looking at this closer, is there a reason why the "xattr_count"
> field is an integer pointer and not just an integer? We're passing
> the entire struct by reference to the individual LSMs so we shouldn't
> need this to get an updated count in the caller and having it as a
> regular integer should simplify things slightly (you could also
> make it an unsigned int while you are it).
>
Copy/paste from v2. Will change xattr_count's type to unsigned int.
> > --- a/kernel/bpf/trampoline.c
> > +++ b/kernel/bpf/trampoline.c
> > @@ -859,6 +859,9 @@ static int bpf_trampoline_add_prog(struct bpf_trampoline *tr,
> > }
> > if (cnt >= BPF_MAX_TRAMP_LINKS)
> > return -E2BIG;
> > + if (node->link->prog->aux->attach_limit &&
> > + tr->progs_cnt[kind] >= node->link->prog->aux->attach_limit)
> > + return -E2BIG;
>
> Re: Alexei's comments about this - if you look back at my previous
> comments, my concern was around BPF LSMs using too many slots in the
> xattr array and causing issues. If the BPF folks want to do that check
> in the kfunc located in the LSM framework I'm okay with that; the
> important part is that the BPF LSMs don't use more space than they
> previously requested.
>
Ack, will remove this attach-time check.
> > diff --git a/security/security.c b/security/security.c
> > index 71aea8fdf014..8f82a1352356 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -1334,6 +1334,7 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> > {
> > struct lsm_static_call *scall;
> > struct xattr *new_xattrs = NULL;
> > + struct xattr_ctx xattr_ctx;
> > int ret = -EOPNOTSUPP, xattr_count = 0;
>
> Since we have the xattr array/pointer and count in the new "lsm_xattrs"
> struct, I think we can remove the "new_xattrs" and "xattr_count" local
> variables in favor of the fields in the new struct.
>
Thanks! These will be eliminated in v4.
Best,
David
^ permalink raw reply
* Re: [PATCH] LSM: check if lsmprop_to_secctx call is supported by LSM
From: Casey Schaufler @ 2026-06-24 19:36 UTC (permalink / raw)
To: Sebastian Bockholt, linux-security-module
Cc: serge, jmorris, paul, Casey Schaufler
In-Reply-To: <DJHGSEVHV1M1.2DMGCPPU1YK4Z@mail.networkname.de>
On 6/24/2026 10:44 AM, Sebastian Bockholt wrote:
> On Fri Jun 19, 2026 at 7:44 PM CEST, Casey Schaufler wrote:
>> If you want to help with the multiple LSM support, there's still
>> plenty of work to do. Let me know.
> This is my first time trying to contribute to the kernel. If this is the wrong
> mailing list or wrong format to discuss this, please tell me directly.
You have come to the right place.
> Otherwise, multiple LSM support seems to be a little bit to ambitious for my
> first contributions.
OK.
>> If the BPF LSM (the BPF LSM infrastructure, not the eBPF programs)
>> is going to support security contexts you need to mark it
>> LSM_FLAGS_EXCLUSIVE.
> [...]
>
>> Until then your choices are:
>>
>> - Make the BPF LSM exclusive
>> - Do not use any of the security context or secid based hooks
>>
> I am not trying to load any BPF myself but I am debugging issues when using
> auditd and apparmor in parallel.
Where does BPF appear in your LSM order?
% cat /sys/kernel/security/lsm
If bpf shows up ahead of apparmor you will see this problem.
> As soon as I try to load audit rules from
> userspace our logs get spammed with "error in audit_log_subj_ctx" messages.
> According to my analysis, the function call chain leading to the bug is:
>
> 1. audit_log_subj_ctx defined in kernel/audit.c
> // the only LSM enabled is apparmor -> audit_subj_secctx_cnt == 1
> // confirmed using bpftrace
> if (audit_subj_secctx_cnt < 2) {
> error = security_lsmprop_to_secctx(prop, &ctx, LSM_ID_UNDEF);
> if (error < 0) {
> if (error != -EINVAL)
> goto error_path; // produces err msgs in logs
> return 0;
> }
> audit_log_format(ab, " subj=%s", ctx.context);
> security_release_secctx(&ctx);
> }
>
> 2. security_lsmprop_to_secctx defined in security/security.c
> // lsm_for_each_hook iterates over all registered LSMs
> // lsm_id == LSM_ID_UNDEF -> the first lsmprop_to_secctx hook is used
> // tracing the following probes using bpftrace
> // kretprobe:apparmor_lsmprop_to_secctx
> // kretprobe:selinux_lsmprop_to_secctx
> // kretprobe:smack_lsmprop_to_secctx
> // kretprobe:bpf_lsm_lsmprop_to_secctx
> // kretprobe:security_lsmprop_to_scctx
> // bpf_lsm_lsmprop_to_secctx hook is executed and returns -EOPNOTSUPP
> lsm_for_each_hook(scall, lsmprop_to_secctx) {
> if (lsmid != LSM_ID_UNDEF && lsmid != scall->hl->lsmid->id)
> continue;
> return scall->hl->hook.lsmprop_to_secctx(prop, cp);
> }
>
> 3. bpf_lsm_lsmprop_to_secctx
> is defined through #include <linux/lsm_hook_defs.h> and returns
> -EOPNOTSUPP default. The return value is propagated up the call stack
> up to security_lsmprop_to_secctx and then to audit_log_subj_ctx.
> audit_log_subj_ctx checks for error return values and prints the
> audit_panic "error in audit_log_subj_ctx"
>
> My patch could check for any errors or lsmprop_to_secctx but since some might
> be useful to check by another function in the call stack, i decided to only
> check if the hook is supported by the LSM.
^ permalink raw reply
* Re: [PATCH v17 06/10] rust: rename `AlwaysRefCounted` to `RefCounted`.
From: Andreas Hindborg @ 2026-06-24 19:17 UTC (permalink / raw)
To: Onur Özkan
Cc: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Daniel Almeida, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
Krzysztof Wilczyński, Boqun Feng, Uladzislau Rezki,
Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett, Igor Korotin,
Pavel Tikhomirov, linux-kernel, rust-for-linux, linux-block,
linux-security-module, dri-devel, linux-fsdevel, linux-mm,
linux-pm, linux-pci, driver-core, Oliver Mangold, Viresh Kumar
In-Reply-To: <20260623175814.87191-1-work@onurozkan.dev>
Onur Özkan <work@onurozkan.dev> writes:
> On Thu, 04 Jun 2026 22:11:18 +0200
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>> From: Oliver Mangold <oliver.mangold@pm.me>
>>
>> There are types where it may both be reference counted in some cases and
>> owned in others. In such cases, obtaining `ARef<T>` from `&T` would be
>> unsound as it allows creation of `ARef<T>` copy from `&Owned<T>`.
>>
>> Therefore, we split `AlwaysRefCounted` into `RefCounted` (which `ARef<T>`
>> would require) and a marker trait to indicate that the type is always
>> reference counted (and not `Ownable`) so the `&T` -> `ARef<T>` conversion
>> is possible.
>>
>> - Rename `AlwaysRefCounted` to `RefCounted`.
>> - Add a new unsafe trait `AlwaysRefCounted`.
>> - Implement the new trait `AlwaysRefCounted` for the newly renamed
>> `RefCounted` implementations. This leaves functionality of existing
>> implementers of `AlwaysRefCounted` intact.
>>
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>> [ Andreas: Updated commit message and rebase on rust-6.20-7.0 ]
>> Acked-by: Igor Korotin <igor.korotin.linux@gmail.com>
>> Acked-by: Danilo Krummrich <dakr@kernel.org>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>> Reviewed-by: Gary Guo <gary@garyguo.net>
>> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> rust/kernel/auxiliary.rs | 7 +++++-
>> rust/kernel/block/mq/request.rs | 15 ++++++++-----
>> rust/kernel/cred.rs | 13 +++++++++--
>> rust/kernel/device.rs | 12 ++++++++--
>> rust/kernel/device/property.rs | 11 +++++++--
>> rust/kernel/drm/device.rs | 9 ++++++--
>> rust/kernel/drm/gem/mod.rs | 16 ++++++++++----
>> rust/kernel/fs/file.rs | 16 ++++++++++----
>> rust/kernel/i2c.rs | 13 ++++++++---
>> rust/kernel/mm.rs | 15 +++++++++----
>> rust/kernel/mm/mmput_async.rs | 9 ++++++--
>> rust/kernel/opp.rs | 10 ++++++---
>> rust/kernel/owned.rs | 2 +-
>> rust/kernel/pci.rs | 10 ++++++++-
>> rust/kernel/pid_namespace.rs | 12 ++++++++--
>> rust/kernel/platform.rs | 7 +++++-
>> rust/kernel/sync/aref.rs | 49 ++++++++++++++++++++++++++---------------
>> rust/kernel/task.rs | 13 +++++++++--
>> rust/kernel/types.rs | 3 ++-
>> rust/kernel/usb.rs | 17 +++++++++++---
>> 20 files changed, 195 insertions(+), 64 deletions(-)
>>
>> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
>> index 93c0db1f6655..49f07740f657 100644
>> --- a/rust/kernel/auxiliary.rs
>> +++ b/rust/kernel/auxiliary.rs
>> @@ -19,6 +19,7 @@
>> to_result, //
>> },
>> prelude::*,
>> + sync::aref::{AlwaysRefCounted, RefCounted},
>
> This patch has multiple horizontal use statements around.
Thanks, I'll take another pass to fix that.
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH v17 08/10] rust: aref: update formatting of use statements
From: Andreas Hindborg @ 2026-06-24 19:16 UTC (permalink / raw)
To: Onur Özkan
Cc: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Daniel Almeida, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
Krzysztof Wilczyński, Boqun Feng, Uladzislau Rezki,
Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett, Igor Korotin,
Pavel Tikhomirov, linux-kernel, rust-for-linux, linux-block,
linux-security-module, dri-devel, linux-fsdevel, linux-mm,
linux-pm, linux-pci, driver-core
In-Reply-To: <20260623175531.85421-1-work@onurozkan.dev>
Onur Özkan <work@onurozkan.dev> writes:
> On Thu, 04 Jun 2026 22:11:20 +0200
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>> Update formatting if use statements in preparation for next commit.
>
> I guess you meant "formatting use statements"? Also, why not doing this in
> the next commit directly?
Because it is an unrelated change.
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH bpf-next v2 1/5] bpf: Verify signed loader metadata at load time
From: Paul Moore @ 2026-06-24 18:42 UTC (permalink / raw)
To: Daniel Borkmann
Cc: ast, kpsingh, James.Bottomley, bboscaccy, memxor, torvalds, bpf,
linux-security-module
In-Reply-To: <603d0f6f-bf02-48ec-af90-f16a239bad85@iogearbox.net>
On Wed, Jun 24, 2026 at 11:37 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 6/24/26 5:12 PM, Paul Moore wrote:
> > On Wed, Jun 24, 2026 at 10:03 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> [...]
> >> include/linux/bpf_verifier.h | 1 +
> >> kernel/bpf/syscall.c | 76 +---------------
> >> kernel/bpf/verifier.c | 163 ++++++++++++++++++++++++++++++++++-
> >> 3 files changed, 165 insertions(+), 75 deletions(-)
> >
> > ...
> >
> >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> >> index b44106c8ea75..026b61d78bdb 100644
> >> --- a/kernel/bpf/syscall.c
> >> +++ b/kernel/bpf/syscall.c
> >> @@ -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)
> >
> > We must preserve the existing location of the call into the
> > security_bpf_prog_load() hook as some users rely on this hook being
> > called *before* the verifier runs.
>
> Keep in mind that the verifier /at this point/ of the new location did
> _not_ verify anything. So there is no heavy-duty work happening yet at
> security_bpf_prog_load. The work that is done before security_bpf_prog_load
> is basically setting up the env, initializing the verifier log, and doing
> the process_fd_array which is resolving the map/BTF objects. But it did
> not walk any instructions etc, so semantics of the security_bpf_prog_load
> hook did not change from a user PoV.
There is still a reasonable amount of work between the existing and
new call sites, and the existing location outside of bpf_check()
offers an additional robustness benefit that future verifier changes
are less likely to impact the hook. If I'm completely honest, I also
need to consider the events of the past year and a half; I'm now much
less inclined to support LSM hook changes in the BPF subsystem because
I'm very concerned about our ability to revert/modify those changes in
the future if needed. That doesn't mean I won't support LSM hook
changes in BPF, but such changes are going to need to have a *very*
strong advantage from a LSM perspective to offset the risk associated
with the current BPF subsystem.
Based on what I see in this patchset, the security_bpf_prog_load()
call should remain in the current location. If you need an additional
hook after the bpf_prog_verify_signature() call I'm happy to work with
you on that.
I also have to bring up the same question I asked back in your v1
posting: have you discussed this signature approach with Alexei? Your
patches abandon and remove KP's signature scheme in favor of what is
effectively Blaise's signature scheme from last fall; Alexei argued
very strongly against these changes in the past. I'd hate to spend a
lot more time reviewing and discussing patches that Alexei is simply
going to NACK once again.
> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >> index 2abc79dbf281..9cd2b62da380 100644
> >> --- a/kernel/bpf/verifier.c
> >> +++ b/kernel/bpf/verifier.c
> >> @@ -19758,11 +19895,28 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
> >> ret = bpf_vlog_init(&env->log, attr_log->level, attr_log->ubuf, attr_log->size);
> >> if (ret)
> >> goto err_unlock;
> >> + if (env->check_signature) {
> >> + ret = bpf_prog_calc_tag(env->prog);
> >> + if (ret < 0)
> >> + goto skip_full_check;
> >> + }
> >>
> >> ret = process_fd_array(env, attr, uattr);
> >> if (ret)
> >> goto skip_full_check;
> >>
> >> + if (env->check_signature) {
> >> + ret = bpf_prog_verify_signature(env, attr, uattr.is_kernel);
> >> + if (ret)
> >> + goto skip_full_check;
> >> + signed_map_cnt = env->used_map_cnt;
> >> + }
> >> +
> >> + ret = security_bpf_prog_load(env->prog, attr, env->prog->aux->token,
> >> + uattr.is_kernel);
> >> + if (ret)
> >> + goto skip_full_check;
> >
> > We can always create a new LSM hook for this call site, e.g.
> > security_bpf_prog_verify_signature(...).
> >
> >> mark_verifier_state_clean(env);
> >>
> >> if (IS_ERR(btf_vmlinux)) {
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] LSM: check if lsmprop_to_secctx call is supported by LSM
From: Sebastian Bockholt @ 2026-06-24 17:44 UTC (permalink / raw)
To: Casey Schaufler, Sebastian Bockholt, linux-security-module
Cc: serge, jmorris, paul
In-Reply-To: <a28fb6b5-cdf3-4523-813a-165c00f7aef6@schaufler-ca.com>
On Fri Jun 19, 2026 at 7:44 PM CEST, Casey Schaufler wrote:
> If you want to help with the multiple LSM support, there's still
> plenty of work to do. Let me know.
This is my first time trying to contribute to the kernel. If this is the wrong
mailing list or wrong format to discuss this, please tell me directly.
Otherwise, multiple LSM support seems to be a little bit to ambitious for my
first contributions.
> If the BPF LSM (the BPF LSM infrastructure, not the eBPF programs)
> is going to support security contexts you need to mark it
> LSM_FLAGS_EXCLUSIVE.
[...]
> Until then your choices are:
>
> - Make the BPF LSM exclusive
> - Do not use any of the security context or secid based hooks
>
I am not trying to load any BPF myself but I am debugging issues when using
auditd and apparmor in parallel. As soon as I try to load audit rules from
userspace our logs get spammed with "error in audit_log_subj_ctx" messages.
According to my analysis, the function call chain leading to the bug is:
1. audit_log_subj_ctx defined in kernel/audit.c
// the only LSM enabled is apparmor -> audit_subj_secctx_cnt == 1
// confirmed using bpftrace
if (audit_subj_secctx_cnt < 2) {
error = security_lsmprop_to_secctx(prop, &ctx, LSM_ID_UNDEF);
if (error < 0) {
if (error != -EINVAL)
goto error_path; // produces err msgs in logs
return 0;
}
audit_log_format(ab, " subj=%s", ctx.context);
security_release_secctx(&ctx);
}
2. security_lsmprop_to_secctx defined in security/security.c
// lsm_for_each_hook iterates over all registered LSMs
// lsm_id == LSM_ID_UNDEF -> the first lsmprop_to_secctx hook is used
// tracing the following probes using bpftrace
// kretprobe:apparmor_lsmprop_to_secctx
// kretprobe:selinux_lsmprop_to_secctx
// kretprobe:smack_lsmprop_to_secctx
// kretprobe:bpf_lsm_lsmprop_to_secctx
// kretprobe:security_lsmprop_to_scctx
// bpf_lsm_lsmprop_to_secctx hook is executed and returns -EOPNOTSUPP
lsm_for_each_hook(scall, lsmprop_to_secctx) {
if (lsmid != LSM_ID_UNDEF && lsmid != scall->hl->lsmid->id)
continue;
return scall->hl->hook.lsmprop_to_secctx(prop, cp);
}
3. bpf_lsm_lsmprop_to_secctx
is defined through #include <linux/lsm_hook_defs.h> and returns
-EOPNOTSUPP default. The return value is propagated up the call stack
up to security_lsmprop_to_secctx and then to audit_log_subj_ctx.
audit_log_subj_ctx checks for error return values and prints the
audit_panic "error in audit_log_subj_ctx"
My patch could check for any errors or lsmprop_to_secctx but since some might
be useful to check by another function in the call stack, i decided to only
check if the hook is supported by the LSM.
^ permalink raw reply
* Re: [PATCH bpf-next v2 1/5] bpf: Verify signed loader metadata at load time
From: Daniel Borkmann @ 2026-06-24 15:37 UTC (permalink / raw)
To: Paul Moore
Cc: ast, kpsingh, James.Bottomley, bboscaccy, memxor, torvalds, bpf,
linux-security-module
In-Reply-To: <CAHC9VhRar1x7kyjhgdmJ=Mfz4Msarv=wQ1821v723CaVdk1uTA@mail.gmail.com>
Hi Paul,
On 6/24/26 5:12 PM, Paul Moore wrote:
> On Wed, Jun 24, 2026 at 10:03 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
[...]
>> include/linux/bpf_verifier.h | 1 +
>> kernel/bpf/syscall.c | 76 +---------------
>> kernel/bpf/verifier.c | 163 ++++++++++++++++++++++++++++++++++-
>> 3 files changed, 165 insertions(+), 75 deletions(-)
>
> ...
>
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index b44106c8ea75..026b61d78bdb 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -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)
>
> We must preserve the existing location of the call into the
> security_bpf_prog_load() hook as some users rely on this hook being
> called *before* the verifier runs.
Keep in mind that the verifier /at this point/ of the new location did
_not_ verify anything. So there is no heavy-duty work happening yet at
security_bpf_prog_load. The work that is done before security_bpf_prog_load
is basically setting up the env, initializing the verifier log, and doing
the process_fd_array which is resolving the map/BTF objects. But it did
not walk any instructions etc, so semantics of the security_bpf_prog_load
hook did not change from a user PoV.
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 2abc79dbf281..9cd2b62da380 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -19758,11 +19895,28 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
>> ret = bpf_vlog_init(&env->log, attr_log->level, attr_log->ubuf, attr_log->size);
>> if (ret)
>> goto err_unlock;
>> + if (env->check_signature) {
>> + ret = bpf_prog_calc_tag(env->prog);
>> + if (ret < 0)
>> + goto skip_full_check;
>> + }
>>
>> ret = process_fd_array(env, attr, uattr);
>> if (ret)
>> goto skip_full_check;
>>
>> + if (env->check_signature) {
>> + ret = bpf_prog_verify_signature(env, attr, uattr.is_kernel);
>> + if (ret)
>> + goto skip_full_check;
>> + signed_map_cnt = env->used_map_cnt;
>> + }
>> +
>> + ret = security_bpf_prog_load(env->prog, attr, env->prog->aux->token,
>> + uattr.is_kernel);
>> + if (ret)
>> + goto skip_full_check;
>
> We can always create a new LSM hook for this call site, e.g.
> security_bpf_prog_verify_signature(...).
>
>> mark_verifier_state_clean(env);
>>
>> if (IS_ERR(btf_vmlinux)) {
>
^ permalink raw reply
* Re: [PATCH bpf-next v2 1/5] bpf: Verify signed loader metadata at load time
From: bot+bpf-ci @ 2026-06-24 15:17 UTC (permalink / raw)
To: daniel, ast
Cc: kpsingh, James.Bottomley, paul, bboscaccy, memxor, torvalds, bpf,
linux-security-module, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
In-Reply-To: <20260624140301.93421-2-daniel@iogearbox.net>
[-- Attachment #1: Type: text/plain, Size: 3997 bytes --]
> 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 [0]. 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 [1][2]. 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 [3]. Let's implement the former,
> without growing the UAPI.
>
> 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. 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.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
This reworks the machinery added by 63a673e8a411 ("bpf: Expose signature
verdict via bpf_prog_aux"), which introduced the prog->aux->sig verdict and
ran verification before security_bpf_prog_load() so the hook would observe
only either UNSIGNED or VERIFIED.
Since this moves the verdict assignment and signature verification into
bpf_check() and folds the exclusive metadata maps into the signed payload
ahead of the admission hook, should it carry:
Fixes: 63a673e8a411 ("bpf: Expose signature verdict via bpf_prog_aux")
---
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/28106955037
^ permalink raw reply
* Re: [PATCH bpf-next v2 1/5] bpf: Verify signed loader metadata at load time
From: Paul Moore @ 2026-06-24 15:12 UTC (permalink / raw)
To: Daniel Borkmann
Cc: ast, kpsingh, James.Bottomley, bboscaccy, memxor, torvalds, bpf,
linux-security-module
In-Reply-To: <20260624140301.93421-2-daniel@iogearbox.net>
On Wed, Jun 24, 2026 at 10:03 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> 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 [0]. 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 [1][2]. 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 [3]. Let's implement the former,
> without growing the UAPI.
>
> 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. 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.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Link: https://lore.kernel.org/bpf/CAHC9VhSDkwGgPfrBUh7EgBKEJj_JjnY68c0YAmuuLT_i--GskQ@mail.gmail.com [0]
> Link: https://lore.kernel.org/bpf/2f71d6c03698eb17d51f7247efde777627ee578a.camel@HansenPartnership.com [1]
> Link: https://lore.kernel.org/lkml/ecf0521ed302db672672ebfbc670ecfba36a6e00.camel@HansenPartnership.com [2]
> Link: https://lore.kernel.org/bpf/88703f00d5b7a779728451008626efa45e42db3d.camel@HansenPartnership.com [3]
> ---
> include/linux/bpf_verifier.h | 1 +
> kernel/bpf/syscall.c | 76 +---------------
> kernel/bpf/verifier.c | 163 ++++++++++++++++++++++++++++++++++-
> 3 files changed, 165 insertions(+), 75 deletions(-)
...
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index b44106c8ea75..026b61d78bdb 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -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)
We must preserve the existing location of the call into the
security_bpf_prog_load() hook as some users rely on this hook being
called *before* the verifier runs.
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2abc79dbf281..9cd2b62da380 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -19758,11 +19895,28 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
> ret = bpf_vlog_init(&env->log, attr_log->level, attr_log->ubuf, attr_log->size);
> if (ret)
> goto err_unlock;
> + if (env->check_signature) {
> + ret = bpf_prog_calc_tag(env->prog);
> + if (ret < 0)
> + goto skip_full_check;
> + }
>
> ret = process_fd_array(env, attr, uattr);
> if (ret)
> goto skip_full_check;
>
> + if (env->check_signature) {
> + ret = bpf_prog_verify_signature(env, attr, uattr.is_kernel);
> + if (ret)
> + goto skip_full_check;
> + signed_map_cnt = env->used_map_cnt;
> + }
> +
> + ret = security_bpf_prog_load(env->prog, attr, env->prog->aux->token,
> + uattr.is_kernel);
> + if (ret)
> + goto skip_full_check;
We can always create a new LSM hook for this call site, e.g.
security_bpf_prog_verify_signature(...).
> mark_verifier_state_clean(env);
>
> if (IS_ERR(btf_vmlinux)) {
--
paul-moore.com
^ permalink raw reply
* [PATCH bpf-next v2 5/5] Documentation/bpf: Add BPF signing and enforcement doc
From: Daniel Borkmann @ 2026-06-24 14:03 UTC (permalink / raw)
To: ast
Cc: kpsingh, James.Bottomley, paul, bboscaccy, memxor, torvalds, bpf,
linux-security-module
In-Reply-To: <20260624140301.93421-1-daniel@iogearbox.net>
Describe the BPF signing design end to end: why a trusted loader is
needed, the signature(insns || metadata) contract, load-time
verification via fd_array (exclusive + frozen maps), the binary
BPF_SIG_{UNSIGNED,VERIFIED} verdict, and how [BPF] LSMs can enforce
policy on it.
This writes down the contract on the discussion points with the LSM /
integrity folks [0][1]: by the time security_bpf_prog_load() is
called, signature verification has fully completed and covers the
instructions plus the frozen contents of every bound exclusive map;
there is no intermediate "loader verified, payload pending" state
to reason about; and what BPF_SIG_VERIFIED means at each hook is
spelled out explicitly, including the post-verifier coverage check
that keeps the verdict binary.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/bc823ddbaf63e0e177eb46d1cc15076e4e2e689d.camel@HansenPartnership.com [0]
Link: https://lore.kernel.org/bpf/CAHC9VhSDkwGgPfrBUh7EgBKEJj_JjnY68c0YAmuuLT_i--GskQ@mail.gmail.com [1]
---
Documentation/bpf/index.rst | 1 +
Documentation/bpf/signing.rst | 490 ++++++++++++++++++++++++++++++++++
2 files changed, 491 insertions(+)
create mode 100644 Documentation/bpf/signing.rst
diff --git a/Documentation/bpf/index.rst b/Documentation/bpf/index.rst
index 0d5c6f659266..638a00d42bc2 100644
--- a/Documentation/bpf/index.rst
+++ b/Documentation/bpf/index.rst
@@ -28,6 +28,7 @@ that goes into great technical depth about the BPF Architecture.
classic_vs_extended.rst
bpf_iterators
bpf_licensing
+ signing
test_debug
clang-notes
linux-notes
diff --git a/Documentation/bpf/signing.rst b/Documentation/bpf/signing.rst
new file mode 100644
index 000000000000..71ca26f8fd2a
--- /dev/null
+++ b/Documentation/bpf/signing.rst
@@ -0,0 +1,490 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============
+BPF signing
+============
+
+This document describes how BPF programs are cryptographically signed, how the
+kernel verifies them at load time, and how Linux Security Modules (LSMs) -
+including the BPF LSM - use the resulting verdict to enforce policy. It is
+written for developers who want to produce signed BPF objects, understand what
+the signature actually guarantees, or build a policy on top of it.
+
+Motivation
+==========
+
+A signed BPF program lets the kernel establish that the bytecode being loaded
+originates from a trusted producer and was not modified in transit. On its own
+the kernel does not *require* signatures - an unsigned program loads exactly as
+before - but it records a verdict (see `The verdict`_) that an LSM can gate on.
+This is the building block for policies such as "only run BPF that was signed by
+a key in the trusted keyring", as enforced for instance by IPE.
+
+Signing is orthogonal to the existing permission model: it does not replace the
+capability checks or the verifier. A signed load still requires the usual
+privileges (``CAP_BPF`` and any program-type-specific capability, subject to
+``kernel.unprivileged_bpf_disabled``), and the loader's instructions are still
+checked by the verifier like any other program. A valid signature establishes
+*origin and integrity*, not safety - it lets a policy trust where the bytecode
+came from, it does not let a load skip any check it would otherwise face.
+
+The hard part is *what* gets signed. A naive scheme would sign a program's
+instruction buffer at build time and verify that signature at
+``BPF_PROG_LOAD``. That does not survive contact with real BPF objects, because
+the bytes the kernel finally loads are not the bytes the developer built and
+signed. Between the two, libbpf and the kernel rewrite the program:
+
+- **map file descriptors** are patched into ``ld_imm64`` instructions
+ (``BPF_PSEUDO_MAP_FD``), and a map's fd is assigned at load time, so it
+ differs on every run;
+- **CO-RE relocations** rewrite field offsets, sizes and existence flags against
+ the *running* kernel's BTF, so the result differs from one kernel to the next;
+- **kfunc and ksym references** are resolved to ids/addresses in the running
+ kernel;
+- **global data** (``.rodata``/``.data``/``.bss``) is created and seeded as maps
+ at load.
+
+So a signature over the original instructions cannot match the relocated
+instructions the verifier ends up checking, and the relocated form cannot be
+produced ahead of time because it depends on the target kernel. There is no
+fixed byte string that is both signable at build time and what the kernel
+actually loads - which is why a program cannot simply be signed and loaded
+directly.
+
+The trusted loader
+==================
+
+The solution is to move that setup work *into* a small BPF program - the
+**loader** - and sign the loader instead of the individual programs. libbpf's
+``gen_loader`` machinery (``bpftool gen skeleton -L``, the "light skeleton")
+emits a ``BPF_PROG_TYPE_SYSCALL`` program whose body performs the bpf() syscalls
+that create maps, apply relocations, and load the real programs. The payload it
+installs - the serialized programs, map descriptions, relocation data and
+initial values - lives in a separate array map, the **metadata map**
+(``__loader.map``).
+
+So the unit of trust is the loader, and the signing contract is::
+
+ Sig(I_loader || D_meta)
+
+where ``I_loader`` is the loader's instruction stream and ``D_meta`` is the
+content of the metadata map. Verifying the loader's signature establishes that
+both the loader *and* the payload it is about to install are authentic. The
+loader is reproducible: ``gen_loader`` builds it from primitives so the same
+object yields the same bytes on any build host.
+
+Why the loader is signable when the program is not
+--------------------------------------------------
+
+The loader sidesteps every rewrite listed above, because the bytes that are
+signed are *relocation-invariant*:
+
+- The loader's own instructions are a fixed sequence of bpf() syscalls emitted
+ by ``gen_loader``; they carry no CO-RE relocations and resolve no ksyms, so
+ they are identical on every kernel. The metadata map is referenced by *index*
+ into ``fd_array`` (``BPF_PSEUDO_MAP_IDX``), not by a baked-in file descriptor,
+ so even that reference does not change between build and load. The loader
+ instruction bytes the kernel verifies are exactly the bytes that were signed.
+- The metadata map is opaque, frozen data - the serialized target programs,
+ their relocation records, map descriptions and initial values. Its bytes are
+ identical at build time and at load time, so they are simply appended to the
+ instructions and covered by the same signature (there is no separate metadata
+ hash to compute or compare).
+
+All the host-specific rewriting - creating maps, patching their fds into the
+target programs, applying CO-RE, resolving ksyms, seeding global data - still
+happens, but it happens *inside the loader at runtime*, on the verified
+metadata, **after** the kernel has verified the ``insns || metadata`` signature.
+The kernel never has to verify the relocated target programs: it verifies the
+loader and its inputs once, and trust transfers to whatever that now-trusted,
+deterministic loader installs. The relocation step is moved from "before the
+signature can be checked" to "after a trusted program runs" - which is exactly
+what makes it signable.
+
+Because the metadata map is the loader's only untrusted input, two existing map
+properties are reused to keep it trustworthy across the load:
+
+Exclusive maps
+ A map created with ``excl_prog_hash`` (see ``BPF_MAP_CREATE``) may only be
+ accessed by a program whose digest matches that hash. The verifier enforces
+ ``map->excl_prog_sha == prog->digest`` for every map a program uses, so the
+ metadata map is bound to exactly the signed loader and cannot be shared with
+ or mutated by another program.
+
+Frozen maps
+ The metadata map is frozen (``BPF_MAP_FREEZE``) before the loader is loaded.
+ Freezing blocks further userspace writes, so the bytes folded into the
+ signature cannot change before the loader runs. (Freezing does not make the
+ map read-only to the loader program itself, which still writes created file
+ descriptors back into the blob's scratch area.)
+
+Load-time verification
+=======================
+
+Rather than have the loader check its own metadata from within BPF, the kernel
+verifies it directly at ``BPF_PROG_LOAD``, with no new UAPI. The mechanism
+reuses the existing ``fd_array``:
+
+#. Userspace creates the metadata map with ``excl_prog_hash`` set to the
+ loader's digest, populates it, and freezes it.
+#. The loader is loaded with ``signature``/``signature_size``/``keyring_id``
+ set, the metadata map referenced through ``fd_array``, and ``fd_array_cnt``
+ set so the kernel knows the array's length.
+#. Signature verification runs inside the verifier (``bpf_check()``), once it
+ has resolved the ``fd_array`` entries into the program's ``used_maps``. The
+ maps folded into the signature are therefore the very objects the program
+ binds - a single resolution of ``fd_array``, not a separate read, so the
+ verified bytes cannot be swapped for a different map after the check (no
+ time-of-check/time-of-use window). Each folded map must be exclusive (carry
+ ``excl_prog_sha``) and a plain array map (``BPF_MAP_TYPE_ARRAY``); only an
+ array map exposes its value buffer through ``map_direct_value_addr()`` as a
+ kernel address spanning ``value_size`` bytes. A map that is not exclusive, not
+ frozen, or not a plain array is rejected, with a verifier log message naming
+ the offending map. The kernel appends each map's frozen
+ contents to the instruction buffer and verifies the PKCS#7 signature over the
+ concatenation ``insns || metadata_0 || metadata_1 || ...`` in ``used_maps``
+ order, before it rewrites the (signed) instructions.
+
+A signed program therefore takes one of exactly two shapes, both fully
+supported:
+
+- **No bound maps** (``fd_array_cnt == 0``): there is nothing to append, so the
+ kernel verifies the signature over the instructions alone. A valid signature
+ yields ``BPF_SIG_VERIFIED`` and the program loads. This is the ordinary case
+ for a directly-loaded signed program with no separate payload; it is *not*
+ rejected for "missing" metadata, because it has none to cover.
+- **Exclusive bound maps** (``fd_array_cnt > 0``): every entry is exclusive and
+ folded, so the signature covers ``insns || metadata``.
+
+There is no third shape: a non-exclusive map in a signed program's ``fd_array``
+is rejected rather than silently left out of the signature, so a program bound
+to a signed loader never has a map the signature does not cover.
+
+The digest binding (``excl_prog_sha == prog->digest``) is enforced by the
+verifier as usual; because that check runs while ``fd_array`` is resolved -
+before the verifier would otherwise compute the tag - ``prog->digest`` is
+computed up front in the verifier, over the unmodified (signature-covered)
+instructions, for any signed load.
+
+Once the verifier has finished resolving instructions, it requires that the
+program use no map beyond those folded into the signature, and that it
+reference no BTF at all, and rejects the load otherwise. A map resolved from
+an instruction after verification - for example an exclusive map reached by a
+directly-referenced fd, or a map swapped into an ``fd_array`` slot the loader
+reads - shows up as an extra ``used_maps`` entry and is caught here; a BTF
+reference (a ksym, or a BTF fd in ``fd_array``) likewise shows up in
+``used_btfs`` and is rejected. Together with the fold rule above this
+keeps the verdict binary: a signed program cannot use a map its signature does
+not cover, and a different but equally digest-bound map cannot be substituted at
+an ``fd_array`` slot. Non-exclusive maps are never folded, so a signed program
+cannot use one at all.
+
+The verdict
+===========
+
+A program is either unsigned or fully verified - there is no intermediate
+state. The outcome is recorded in ``prog->aux->sig.verdict``:
+
+.. code-block:: c
+
+ enum bpf_sig_verdict {
+ BPF_SIG_UNSIGNED = 0,
+ BPF_SIG_VERIFIED,
+ };
+
+``BPF_SIG_VERIFIED`` means the signature is valid and covers the instructions
+*and* the frozen contents of every exclusive map the program uses:
+
+- For an ordinary, directly-loaded signed program the instructions are the whole
+ artifact and it uses no exclusive maps, so a valid instruction signature is
+ the complete verification.
+- For a signed loader the metadata map is exclusive, so its contents are folded
+ in and the signature covers ``insns || metadata``.
+
+There is deliberately no "instructions verified but metadata not" verdict: a
+signed loader that fails to cover its metadata is *rejected* (see above), not
+recorded with a weaker verdict. ``BPF_SIG_VERIFIED`` therefore always means the
+program and everything the signature is responsible for are authentic, which is
+what a policy can rely on.
+
+Alongside the verdict the kernel records which keyring validated the signature;
+see `Keyrings`_.
+
+Enforcement via LSMs
+====================
+
+Signing only *records* a verdict; an LSM turns it into policy. The verdict and
+keyring fields live in ``struct bpf_prog_aux``, so a BPF LSM program can read
+them directly (see Documentation/bpf/prog_lsm.rst for writing and attaching BPF
+LSM programs); the same fields are equally available to in-tree LSMs such as
+IPE. Two hooks are useful at different points of the load: the dedicated
+``security_bpf_prog_load()`` gates admission before the main verification work,
+and the existing ``security_bpf_prog()`` observes a program that has fully
+loaded.
+
+Admission: ``security_bpf_prog_load()``
+---------------------------------------
+
+This hook gates admission **for every load**, from a single call site inside the
+verifier (``bpf_check()``), before the main verification work. It runs after the
+optional signature verification, so the verdict and keyring fields are final - the
+hook can see whether, and how strongly, the program was signed, which keyring
+validated it, the load ``attr``, the BPF token and whether the load came from the
+kernel. For a signed load the verdict is ``BPF_SIG_VERIFIED`` here (the signature
+has just been checked); for an unsigned load it is ``BPF_SIG_UNSIGNED``.
+
+This is the place for *coarse admission* that must also see unsigned and
+not-yet-verified loads: require a signature at all, restrict the acceptable
+keyring, restrict which token/credentials may load BPF, apply per-program-type
+rules, or audit every attempt. It is the primary deny point.
+
+One subtlety: this hook runs *before* the verifier finishes its work, so
+``BPF_SIG_VERIFIED`` *here* means only "validly signed" - not "loaded". Allowing
+a load at this point lets it *proceed*; it does not guarantee the program will
+load. A validly signed program can still be rejected afterwards on two
+independent grounds: the verifier may reject it like any other program (unsafe
+memory access, bad control flow, resource limits, ...), and the kernel separately
+confirms - once the verifier resolves instructions - that the program only uses
+maps the signature covers, rejecting a violating load regardless of what this
+hook returned. Only after the program has fully loaded, at the next hook
+(``security_bpf_prog()``), does ``BPF_SIG_VERIFIED`` carry its full meaning:
+validly signed *and* fully verified.
+
+A more realistic admission policy than "is it signed at all": accept programs
+signed by a system keyring, accept a user-keyring signature only if the
+key/keyring it was verified against is on an explicit allowlist, and emit a
+tamper-evident record of every decision so that even denied attempts are
+auditable. (Illustrative - error checking elided.)
+
+.. code-block:: c
+
+ /* Serials of user keys/keyrings we additionally trust. */
+ struct {
+ __uint(type, BPF_MAP_TYPE_HASH);
+ __type(key, __s32); /* keyring_serial */
+ __type(value, __u8);
+ __uint(max_entries, 64);
+ } trusted_user_keys SEC(".maps");
+
+ /* Audit stream consumed by a userspace logger. */
+ struct {
+ __uint(type, BPF_MAP_TYPE_RINGBUF);
+ __uint(max_entries, 1 << 16);
+ } audit SEC(".maps");
+
+ struct decision { __u32 prog_type, verdict, ktype; __s32 serial, ret; };
+
+ SEC("lsm/bpf_prog_load")
+ int BPF_PROG(admit, struct bpf_prog *prog, union bpf_attr *attr,
+ struct bpf_token *token, bool kernel)
+ {
+ __u32 verdict = prog->aux->sig.verdict;
+ __u32 ktype = prog->aux->sig.keyring_type;
+ __s32 serial = prog->aux->sig.keyring_serial;
+ struct decision *d;
+ int ret = 0;
+
+ if (kernel)
+ return 0; /* trust in-kernel loads */
+
+ if (verdict != BPF_SIG_VERIFIED)
+ ret = -EPERM; /* must be validly signed */
+ else if (ktype == BPF_SIG_KEYRING_USER &&
+ !bpf_map_lookup_elem(&trusted_user_keys, &serial))
+ ret = -EPERM; /* key/keyring not allowlisted */
+
+ d = bpf_ringbuf_reserve(&audit, sizeof(*d), 0);
+ if (d) {
+ d->prog_type = attr->prog_type;
+ d->verdict = verdict;
+ d->ktype = ktype;
+ d->serial = serial;
+ d->ret = ret;
+ bpf_ringbuf_submit(d, 0); /* record allow *and* deny */
+ }
+ return ret;
+ }
+
+Observing a verified load: ``security_bpf_prog()``
+--------------------------------------------------
+
+There is deliberately no separate "metadata attested" hook. The coverage check
+above is enforced by the kernel unconditionally, so a signed loader that fails
+to cover its metadata never loads and an LSM never has to re-establish that
+fact. To *act on* a program that has successfully and fully loaded, use the
+existing ``security_bpf_prog()`` hook (``lsm/bpf_prog``), which fires from
+``bpf_prog_new_fd()`` - after the verifier, after the coverage check, and after
+``bpf_prog_alloc_id()``. Relative to the admission hook this point is strictly
+later and stronger:
+
+- the program has an id (``prog->aux->id``), so it can be recorded or correlated
+ with later events;
+- ``verdict == BPF_SIG_VERIFIED`` *here* means **fully** verified - a program
+ that used a map the signature does not cover was already rejected, so it cannot
+ reach this point;
+- it observes only programs that actually loaded; a failed load never mints an
+ fd, so it never reaches this hook.
+
+It takes only the ``prog`` and a non-zero return still aborts (the fd is not
+handed out), so it can veto as well as observe. One wrinkle: it also fires on
+other paths that mint a new program fd - notably ``bpf_prog_get_fd_by_id()`` -
+not just on a fresh load. Because the program already has its id here, an LSM
+can tell the two apart with a small hash map: the *first* time an id is seen is
+the load; a later sighting of the same id is just another fd to a program that
+already exists.
+
+To bound the map and let a reused id read as a fresh load, this can be paired
+with ``security_bpf_prog_free()`` (``lsm/bpf_prog_free``), which deletes the
+entry on teardown - keyed by the same ``prog`` pointer, since
+``bpf_prog_free_id()`` has already cleared ``prog->aux->id`` to ``0`` by the time
+that hook runs. (Illustrative - privileged LSM, error checking elided.)
+
+.. code-block:: c
+
+ struct rec { __u32 id, ktype; __s32 serial; };
+
+ struct {
+ __uint(type, BPF_MAP_TYPE_HASH);
+ __type(key, __u64); /* struct bpf_prog * -- stable id */
+ __type(value, struct rec);
+ __uint(max_entries, 4096);
+ } live SEC(".maps");
+
+ SEC("lsm/bpf_prog") /* fires after load and on every later fd */
+ int BPF_PROG(observe, struct bpf_prog *prog)
+ {
+ __u64 key = (__u64)(unsigned long)prog;
+ struct rec r;
+
+ if (prog->aux->sig.verdict != BPF_SIG_VERIFIED)
+ return 0;
+ if (bpf_map_lookup_elem(&live, &key))
+ return 0; /* seen before: a later fd, not a load */
+
+ /* First sighting == this program just loaded; id is valid here. */
+ r.id = prog->aux->id;
+ r.ktype = prog->aux->sig.keyring_type;
+ r.serial = prog->aux->sig.keyring_serial;
+ bpf_map_update_elem(&live, &key, &r, BPF_NOEXIST);
+ /* ... newly-loaded verified-program action, e.g. record r.id ... */
+ return 0;
+ }
+
+Putting them together: to *require* verified BPF, deny at the admission hook
+unless the verdict is ``BPF_SIG_VERIFIED`` (and, if desired, restrict the
+keyring). The kernel then guarantees that any program which actually loads with
+that verdict covered all of its exclusive maps, rejecting any that did not - so
+a deny-by-default admission policy needs no second enforcement point. Use
+``security_bpf_prog()`` to record or finally gate the verified programs once
+they carry an id. The ``verdict``, ``keyring_type`` and ``keyring_serial`` fields
+let a policy distinguish, for example, "verified and signed by a builtin key"
+from "verified by a user key". Policy LSMs such as IPE consume the same hooks to
+enforce system policy without writing any BPF.
+
+Keyrings
+========
+
+``keyring_id`` selects the trusted keyring the PKCS#7 signature is verified
+against. The well-known ids ``0`` (builtin), ``VERIFY_USE_SECONDARY_KEYRING``
+and ``VERIFY_USE_PLATFORM_KEYRING`` select the corresponding system keyrings;
+any other value is treated as the serial of a user/session key or keyring.
+The keyring is looked up first, before the signature bytes are examined, so a
+signature naming a non-existent keyring is rejected up front, and a failed
+verification aborts the load - so a program that loads successfully with a
+signature always has consistent keyring fields recorded.
+
+Two fields are recorded in ``prog->aux->sig`` for an LSM to inspect:
+
+``keyring_type`` (``enum bpf_sig_keyring``)
+ Classified purely from ``keyring_id`` whenever the program is signed:
+ ``BPF_SIG_KEYRING_BUILTIN``, ``_SECONDARY``, ``_PLATFORM`` for the system
+ keyrings, or ``_USER`` for a user/session keyring. It is
+ ``BPF_SIG_KEYRING_NONE`` for an unsigned program.
+
+``keyring_serial`` (``s32``)
+ Set **only** on a successful verification, to the serial of the
+ **user/session key or keyring** that ``keyring_id`` resolved to - the
+ object the signature was verified against, not the individual asymmetric
+ key inside it that matched the signer. Passing
+ ``KEY_SPEC_SESSION_KEYRING``, for example, records the session keyring's
+ serial. The system keyrings are trusted as a whole and expose no serial
+ here, so the serial is ``0`` for builtin, secondary and platform
+ signatures, and ``0`` for unsigned programs. In other words, a non-zero
+ ``keyring_serial`` is exactly "verified against the user key/keyring with
+ this serial".
+
+.. list-table::
+ :header-rows: 1
+
+ * - ``keyring_id``
+ - ``keyring_type``
+ - ``keyring_serial``
+ * - (no signature)
+ - ``BPF_SIG_KEYRING_NONE``
+ - ``0``
+ * - ``0``
+ - ``BPF_SIG_KEYRING_BUILTIN``
+ - ``0``
+ * - ``VERIFY_USE_SECONDARY_KEYRING``
+ - ``BPF_SIG_KEYRING_SECONDARY``
+ - ``0``
+ * - ``VERIFY_USE_PLATFORM_KEYRING``
+ - ``BPF_SIG_KEYRING_PLATFORM``
+ - ``0``
+ * - other (a user/session key serial)
+ - ``BPF_SIG_KEYRING_USER``
+ - serial of the resolved key/keyring
+
+Producing a signed object
+==========================
+
+``bpftool`` generates and signs a light skeleton in one step::
+
+ bpftool gen skeleton -L -S -k <private_key.pem> -i <certificate.x509> \
+ obj.bpf.o > obj.lskel.h
+
+``-L`` selects the light-skeleton (``gen_loader``) backend and ``-S`` enables
+signing; ``-k`` and ``-i`` supply the signing key and its X.509 certificate.
+``bpftool`` signs ``insns || metadata`` - the exact bytes the kernel
+reconstructs - and also computes ``excl_prog_hash`` as the digest of the loader
+instructions so the metadata map can be bound to the loader. The signature,
+certificate and hash are embedded in the generated header; loading the skeleton
+performs the create/populate/freeze/load sequence described above.
+
+At runtime the trusted public key must be present in the chosen keyring (for
+example added to the session keyring, or built into the kernel's builtin trusted
+keyring) for verification to succeed.
+
+UAPI reference
+==============
+
+``BPF_PROG_LOAD`` (``union bpf_attr``):
+
+``signature``, ``signature_size``
+ Pointer to and length of the PKCS#7 signature blob.
+
+``keyring_id``
+ Trusted keyring selector (see `Keyrings`_).
+
+``fd_array``, ``fd_array_cnt``
+ Array of map file descriptors bound to the program. ``fd_array_cnt`` must be
+ set for the kernel to scan the array. When a signature is present, every map
+ in the array must be exclusive; its frozen contents are folded into the
+ verified buffer, and a non-exclusive entry is rejected.
+
+``BPF_MAP_CREATE`` (``union bpf_attr``):
+
+``excl_prog_hash``, ``excl_prog_hash_size``
+ SHA-256 digest of the program permitted to access this (exclusive) map. This
+ binds the metadata map to the loader; it is not a hash of the map *content*.
+ The map content is not hashed separately at all - it is covered, as bytes,
+ by the program signature.
+
+Notes and limitations
+======================
+
+- The instructions plus folded metadata are verified as one ``bpf_dynptr``,
+ which bounds the combined size (currently ~16 MiB); very large objects can
+ exceed it.
+- The metadata container is a single-element array map, accessed through
+ ``map_direct_value_addr``.
--
2.43.0
^ permalink raw reply related
* [PATCH bpf-next v2 0/5] Verify BPF signed loader at load time
From: Daniel Borkmann @ 2026-06-24 14:02 UTC (permalink / raw)
To: ast
Cc: kpsingh, James.Bottomley, paul, bboscaccy, memxor, torvalds, 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/
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 (5):
bpf: Verify signed loader metadata at load time
libbpf: Drop in-loader metadata check for load-time verification
bpftool: Cover loader metadata with the program signature
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 | 490 ++++++++++++++
include/linux/bpf_verifier.h | 1 +
kernel/bpf/syscall.c | 81 +--
kernel/bpf/verifier.c | 163 ++++-
tools/bpf/bpftool/gen.c | 2 +
tools/bpf/bpftool/sign.c | 15 +-
tools/lib/bpf/bpf_gen_internal.h | 1 -
tools/lib/bpf/gen_loader.c | 76 +--
tools/lib/bpf/skel_internal.h | 27 +-
.../selftests/bpf/prog_tests/signed_loader.c | 597 +++++++++++++-----
11 files changed, 1114 insertions(+), 340 deletions(-)
create mode 100644 Documentation/bpf/signing.rst
--
2.43.0
^ permalink raw reply
* [PATCH bpf-next v2 4/5] selftests/bpf: Verify load-time signed loader metadata
From: Daniel Borkmann @ 2026-06-24 14:03 UTC (permalink / raw)
To: ast
Cc: kpsingh, James.Bottomley, paul, bboscaccy, memxor, torvalds, bpf,
linux-security-module
In-Reply-To: <20260624140301.93421-1-daniel@iogearbox.net>
The signed gen_loader no longer checks its metadata map from within
BPF; the kernel does it at BPF_PROG_LOAD by folding the loader's frozen
exclusive fd_array maps into the signature. Exercise that path end to
end. Extend with more test cases (e.g. map-less program, asserting the
LSM admission hook observes BPF_SIG_UNSIGNED and BPF_SIG_VERIFIED), and
retire the subtests that asserted the old in-loader check, which no
longer exists.
# LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh -- ./test_progs -t signed_loader
[...]
#410/1 signed_loader/loadtime_no_map:OK
#410/2 signed_loader/loadtime_with_map:OK
#410/3 signed_loader/metadata_match:OK
#410/4 signed_loader/signature_enforced:OK
#410/5 signed_loader/signed_nonexcl_fd_array_rejected:OK
#410/6 signed_loader/signed_nonarray_fd_array_rejected:OK
#410/7 signed_loader/signed_btf_fd_array_rejected:OK
#410/8 signed_loader/signature_failure_logs:OK
#410/9 signed_loader/signature_too_large:OK
#410/10 signed_loader/signature_bad_keyring:OK
#410/11 signed_loader/metadata_ctx_max_entries_ignored:OK
#410/12 signed_loader/metadata_ctx_initial_value_ignored:OK
#410/13 signed_loader/signature_authenticates_insns:OK
#410/14 signed_loader/hash_requires_frozen:OK
#410/15 signed_loader/no_update_after_freeze:OK
#410/16 signed_loader/freeze_writable_mmap:OK
#410/17 signed_loader/no_writable_mmap_frozen:OK
#410/18 signed_loader/map_hash_matches_libbpf:OK
#410/19 signed_loader/map_hash_multi_element:OK
#410/20 signed_loader/map_hash_bad_size:OK
#410/21 signed_loader/map_hash_unsupported_type:OK
#410/22 signed_loader/lsm_signature_verdict:OK
#410 signed_loader:OK
Summary: 1/22 PASSED, 0 SKIPPED, 0 FAILED
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
.../selftests/bpf/prog_tests/signed_loader.c | 597 +++++++++++++-----
1 file changed, 433 insertions(+), 164 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/signed_loader.c b/tools/testing/selftests/bpf/prog_tests/signed_loader.c
index 5fc417e31fc6..00591b67ba20 100644
--- a/tools/testing/selftests/bpf/prog_tests/signed_loader.c
+++ b/tools/testing/selftests/bpf/prog_tests/signed_loader.c
@@ -11,6 +11,8 @@
#include <linux/keyctl.h>
#include <linux/bpf.h>
+#include <bpf/btf.h>
+
#include "bpf/libbpf_internal.h" /* for libbpf_sha256() */
#include "bpf/skel_internal.h" /* for loader ctx layout (bpf_loader_ctx etc) */
@@ -19,8 +21,6 @@
#include "test_signed_loader_data.skel.h"
#include "test_signed_loader_lsm.skel.h"
-#define SIG_MATCH_INSNS 33 /* excl (5) + 4 * sha-dword (7) */
-
enum {
BPF_SIG_UNSIGNED = 0,
BPF_SIG_VERIFIED,
@@ -35,7 +35,8 @@ enum {
};
static int load_loader(const void *insns, __u32 insns_sz, int map_fd,
- const void *sig, __u32 sig_sz, __s32 keyring_id)
+ const void *sig, __u32 sig_sz, __s32 keyring_id,
+ __u32 fd_array_cnt)
{
union bpf_attr attr;
int fd;
@@ -52,6 +53,7 @@ static int load_loader(const void *insns, __u32 insns_sz, int map_fd,
attr.signature_size = sig_sz;
attr.keyring_id = keyring_id;
}
+ attr.fd_array_cnt = fd_array_cnt;
memcpy(attr.prog_name, "__loader.prog", sizeof("__loader.prog"));
fd = syscall(__NR_bpf, BPF_PROG_LOAD, &attr,
offsetofend(union bpf_attr, keyring_id));
@@ -62,14 +64,12 @@ static int run_gen_loader(const void *insns, __u32 insns_sz,
const void *data, __u32 data_sz,
const void *excl, __u32 excl_sz,
const void *sig, __u32 sig_sz,
- bool get_hash, void *ctx, __u32 ctx_sz, bool *loader_ran)
+ void *ctx, __u32 ctx_sz, bool *loader_ran)
{
LIBBPF_OPTS(bpf_map_create_opts, mopts,
.excl_prog_hash = excl,
.excl_prog_hash_size = excl_sz);
- __u8 hbuf[SHA256_DIGEST_LENGTH];
- struct bpf_map_info info;
- __u32 ilen = sizeof(info), key = 0;
+ __u32 key = 0;
union bpf_attr attr;
int map_fd, prog_fd, ret;
@@ -87,15 +87,6 @@ static int run_gen_loader(const void *insns, __u32 insns_sz,
ret = -errno;
goto out_map;
}
- if (get_hash) {
- memset(&info, 0, sizeof(info));
- info.hash = ptr_to_u64(hbuf);
- info.hash_size = sizeof(hbuf);
- if (bpf_map_get_info_by_fd(map_fd, &info, &ilen)) {
- ret = -errno;
- goto out_map;
- }
- }
memset(&attr, 0, sizeof(attr));
attr.prog_type = BPF_PROG_TYPE_SYSCALL;
@@ -108,6 +99,7 @@ static int run_gen_loader(const void *insns, __u32 insns_sz,
attr.signature = ptr_to_u64(sig);
attr.signature_size = sig_sz;
attr.keyring_id = KEY_SPEC_SESSION_KEYRING;
+ attr.fd_array_cnt = 1;
}
memcpy(attr.prog_name, "__loader.prog", sizeof("__loader.prog"));
prog_fd = syscall(__NR_bpf, BPF_PROG_LOAD, &attr,
@@ -236,79 +228,6 @@ static int sign_buf(const char *dir, const void *buf, __u32 len,
return ret;
}
-static void check_sig_match_shape(const struct bpf_insn *in, int n)
-{
- int a = -1, cleanup = -1, i, base, t, br[5], nb = 0;
-
- /* BPF_PSEUDO_MAP_IDX (the struct bpf_map * form) is used only here. */
- for (i = 0; i + 1 < n; i++) {
- if (in[i].code == (BPF_LD | BPF_IMM | BPF_DW) &&
- in[i].src_reg == BPF_PSEUDO_MAP_IDX) {
- a = i;
- break;
- }
- }
- if (!ASSERT_GE(a, 0, "emit_signature_match present"))
- return;
- if (!ASSERT_LE(a + SIG_MATCH_INSNS, n, "block fits in program"))
- return;
-
- /* excl check: r2 = *(u32 *)(map + 32); if r2 != 1 goto cleanup */
- ASSERT_EQ(in[a + 2].code, (BPF_LDX | BPF_MEM | BPF_W), "excl load width");
- ASSERT_EQ(in[a + 2].off, SHA256_DIGEST_LENGTH, "excl field offset");
- ASSERT_EQ(in[a + 4].code, (BPF_JMP | BPF_JNE | BPF_K), "excl branch op");
- ASSERT_EQ(in[a + 4].imm, 1, "excl compared to 1");
- br[nb++] = a + 4;
-
- /* 4 sha-dword checks: r2 = *(u64 *)(map + i*8); if r2 != r3 goto cleanup */
- for (i = 0; i < 4; i++) {
- base = a + 5 + i * 7;
- ASSERT_EQ(in[base + 2].code, (BPF_LDX | BPF_MEM | BPF_DW), "sha load width");
- ASSERT_EQ(in[base + 2].off, i * 8, "sha dword offset");
- ASSERT_EQ(in[base + 3].code, (BPF_LD | BPF_IMM | BPF_DW), "sha imm64 (H_meta)");
- ASSERT_EQ(in[base + 6].code, (BPF_JMP | BPF_JNE | BPF_X), "sha branch op");
- br[nb++] = base + 6;
- }
-
- /*
- * Locate the real cleanup label so we can pin the exact jump target,
- * not just "some backward label". bpf_gen__init() emits the cleanup
- * block as a prog-fd close loop whose first instruction is the label
- * every error branch jumps to.
- */
- for (i = 0; i + 2 < a; i++) {
- if (in[i].code == (BPF_LDX | BPF_MEM | BPF_W) &&
- in[i].dst_reg == BPF_REG_1 && in[i].src_reg == BPF_REG_10 &&
- in[i + 1].code == (BPF_JMP | BPF_JSLE | BPF_K) &&
- in[i + 1].dst_reg == BPF_REG_1 && in[i + 1].imm == 0 &&
- in[i + 1].off == 1 &&
- in[i + 2].code == (BPF_JMP | BPF_CALL) &&
- in[i + 2].imm == BPF_FUNC_sys_close) {
- cleanup = i;
- break;
- }
- }
- if (!ASSERT_GE(cleanup, 0, "cleanup label located"))
- return;
- for (i = 0; i < nb; i++) {
- t = br[i] + 1 + in[br[i]].off;
- ASSERT_EQ(t, cleanup, "sig-match lands on cleanup");
- }
- /*
- * Same invariant for every other cleanup-bound jump in the program:
- * emit_check_err() is the only source of "if (r7 < 0) goto cleanup",
- * so each of those must also resolve exactly to cleanup.
- */
- for (i = 0, t = 0; i < n; i++) {
- if (in[i].code != (BPF_JMP | BPF_JSLT | BPF_K) ||
- in[i].dst_reg != BPF_REG_7 || in[i].imm != 0 || in[i].off >= 0)
- continue;
- ASSERT_EQ(i + 1 + in[i].off, cleanup, "err-check lands on cleanup");
- t++;
- }
- ASSERT_GT(t, 0, "found emit_check_err jumps");
-}
-
struct gen_loader_fixture {
struct test_signed_loader *skel;
struct gen_loader_opts gopts;
@@ -372,16 +291,6 @@ static void gen_loader_fixture_fini(struct gen_loader_fixture *f)
test_signed_loader__destroy(f->skel);
}
-static void metadata_check_shape(void)
-{
- struct gen_loader_fixture f;
-
- if (gen_loader_fixture_init(&f) == 0)
- check_sig_match_shape((const struct bpf_insn *)f.gopts.insns,
- f.gopts.insns_sz / sizeof(struct bpf_insn));
- gen_loader_fixture_fini(&f);
-}
-
static void metadata_match(void)
{
struct gen_loader_fixture f;
@@ -391,94 +300,221 @@ static void metadata_match(void)
if (gen_loader_fixture_init(&f) == 0) {
r = run_gen_loader(f.gopts.insns, f.gopts.insns_sz, f.blob,
f.data_sz, f.excl, sizeof(f.excl), NULL, 0,
- true, f.ctx, f.ctx_sz, &ran);
+ f.ctx, f.ctx_sz, &ran);
ASSERT_TRUE(ran, "loader ran");
ASSERT_EQ(r, 0, "honest loader retval");
}
gen_loader_fixture_fini(&f);
}
-static void metadata_sha_mismatch(void)
+static void signature_enforced(void)
{
+ static const __u8 junk[64] = { 0x30, 0x42, 0x13, 0x37, };
struct gen_loader_fixture f;
- bool ran;
- int r;
+ int fd;
if (gen_loader_fixture_init(&f) == 0) {
/*
- * blob[0] lives in the loader's fd_array scratch (first add_data in
- * bpf_gen__init); a 0-map program never reads it, so flipping it
- * changes only map->sha. The metadata check is the only thing that
- * can notice -> isolates emit_signature_match.
+ * A present-but-invalid signature (the cert bytes are not a
+ * PKCS#7 signature) must be rejected at load: the signature
+ * path is honored, not ignored. (The valid path is covered by
+ * the signed lskels.)
*/
- f.blob[0] ^= 0xff;
- r = run_gen_loader(f.gopts.insns, f.gopts.insns_sz, f.blob,
- f.data_sz, f.excl, sizeof(f.excl), NULL, 0,
- true, f.ctx, f.ctx_sz, &ran);
- ASSERT_TRUE(ran, "loader ran");
- ASSERT_EQ(r, -EINVAL, "tampered blob rejected by emit_signature_match");
+ fd = load_loader(f.gopts.insns, f.gopts.insns_sz, -1, junk,
+ sizeof(junk), KEY_SPEC_SESSION_KEYRING, 0);
+ ASSERT_LT(fd, 0, "invalid signature rejected at load");
}
gen_loader_fixture_fini(&f);
}
-static void metadata_not_exclusive(void)
+static void signed_nonexcl_fd_array_rejected(void)
{
+ static const __u8 junk[64] = { 0x30, 0x42, 0x13, 0x37, };
struct gen_loader_fixture f;
- bool ran;
- int r;
+ int map_fd, fd;
if (gen_loader_fixture_init(&f) == 0) {
/*
- * Correct blob but a non-exclusive metadata map: the verifier does
- * not reject (excl_prog_sha unset), so the runtime map->excl == 1
- * check in the loader must.
+ * A signed program may only bind exclusive maps through fd_array
+ * (their contents are folded into the signature). Binding a
+ * non-exclusive map is rejected, before the signature is even
+ * examined.
*/
- r = run_gen_loader(f.gopts.insns, f.gopts.insns_sz, f.blob,
- f.data_sz, NULL, 0, NULL, 0, true, f.ctx,
- f.ctx_sz, &ran);
- ASSERT_TRUE(ran, "loader ran");
- ASSERT_EQ(r, -EINVAL, "non-exclusive metadata map rejected");
+ map_fd = bpf_map_create(BPF_MAP_TYPE_ARRAY, "nonexcl", 4,
+ f.data_sz, 1, NULL);
+ if (ASSERT_OK_FD(map_fd, "nonexcl_map")) {
+ fd = load_loader(f.gopts.insns, f.gopts.insns_sz, map_fd,
+ junk, sizeof(junk),
+ KEY_SPEC_SESSION_KEYRING, 1);
+ ASSERT_EQ(fd, -EPERM,
+ "non-exclusive map in signed fd_array rejected");
+ if (fd >= 0)
+ close(fd);
+ close(map_fd);
+ }
}
gen_loader_fixture_fini(&f);
}
-static void metadata_hash_not_computed(void)
+static void signed_nonarray_fd_array_rejected(void)
{
+ static const __u8 junk[64] = { 0x30, 0x42, 0x13, 0x37, };
+ LIBBPF_OPTS(bpf_map_create_opts, mopts);
struct gen_loader_fixture f;
- bool ran;
- int r;
+ int map_fd, fd;
if (gen_loader_fixture_init(&f) == 0) {
/*
- * Correct, exclusive, frozen map, but its hash was never computed
- * (no OBJ_GET_INFO_BY_FD), so map->sha stays zero. The loader must
- * fail closed rather than treat an unset hash as a match.
+ * Only a plain BPF_MAP_TYPE_ARRAY may be folded into the
+ * signature. An exclusive map of any other type is rejected
+ * (-EINVAL) rather than folded - this is the type gate that
+ * keeps arena maps (map_direct_value_addr() returns a user
+ * address) and insn-array maps (buffer smaller than value_size)
+ * out of the hashed region, where the old code would have
+ * memcpy()'d from them. A hash map stands in here: it is
+ * exclusive (bound to the loader digest) but not an array.
*/
- r = run_gen_loader(f.gopts.insns, f.gopts.insns_sz, f.blob,
- f.data_sz, f.excl, sizeof(f.excl), NULL, 0,
- false, f.ctx, f.ctx_sz, &ran);
- ASSERT_TRUE(ran, "loader ran");
- ASSERT_EQ(r, -EINVAL, "uncomputed metadata hash rejected");
+ mopts.excl_prog_hash = f.excl;
+ mopts.excl_prog_hash_size = sizeof(f.excl);
+ map_fd = bpf_map_create(BPF_MAP_TYPE_HASH, "excl_hash", 4, 4, 1,
+ &mopts);
+ if (ASSERT_OK_FD(map_fd, "excl_hash_map")) {
+ fd = load_loader(f.gopts.insns, f.gopts.insns_sz, map_fd,
+ junk, sizeof(junk),
+ KEY_SPEC_SESSION_KEYRING, 1);
+ ASSERT_EQ(fd, -EINVAL,
+ "non-array map in signed fd_array rejected");
+ if (fd >= 0)
+ close(fd);
+ close(map_fd);
+ }
}
gen_loader_fixture_fini(&f);
}
-static void signature_enforced(void)
+static int setup_meta_map(const struct gen_loader_fixture *f);
+
+static void signed_btf_fd_array_rejected(void)
+{
+ char dir_tmpl[] = "/tmp/signed_loader_btfXXXXXX", *dir = NULL;
+ __u32 sig_sz = 8192;
+ int map_fd = -1, prog_fd = -1;
+ unsigned char *buf = NULL;
+ struct gen_loader_fixture f;
+ bool have_fixture = false;
+ struct btf *btf = NULL;
+ union bpf_attr attr;
+ int fds[2];
+ __u8 sig[8192];
+
+ syscall(__NR_request_key, "keyring", "_uid.0", NULL,
+ KEY_SPEC_SESSION_KEYRING);
+ dir = mkdtemp(dir_tmpl);
+ if (!ASSERT_OK_PTR(dir, "mkdtemp"))
+ return;
+ if (!ASSERT_OK(run_setup("setup", dir), "verify_sig_setup")) {
+ rmdir(dir);
+ return;
+ }
+ if (gen_loader_fixture_init(&f) != 0)
+ goto out;
+ have_fixture = true;
+
+ /*
+ * fd_array binds maps and BTFs alike, but only exclusive array maps are
+ * folded into the signature. Build a genuinely signed load - insns ||
+ * metadata, exclusive frozen map at fd_array[0] - so it reaches
+ * BPF_SIG_VERIFIED, then smuggle an extra BTF into fd_array[1]. The BTF
+ * is not foldable and carries no excl_prog_sha, so it is not covered;
+ * the post-verifier coverage check (used_btf_cnt != 0) must reject the
+ * verified program with -EACCES.
+ */
+ buf = malloc((size_t)f.gopts.insns_sz + f.data_sz);
+ if (!ASSERT_OK_PTR(buf, "signbuf"))
+ goto out;
+ memcpy(buf, f.gopts.insns, f.gopts.insns_sz);
+ memcpy(buf + f.gopts.insns_sz, f.blob, f.data_sz);
+ if (!ASSERT_OK(sign_buf(dir, buf, f.gopts.insns_sz + f.data_sz, sig,
+ &sig_sz), "sign insns||metadata"))
+ goto out;
+
+ map_fd = setup_meta_map(&f);
+ if (!ASSERT_OK_FD(map_fd, "meta_map"))
+ goto out;
+ btf = btf__new_empty();
+ if (!ASSERT_OK_PTR(btf, "btf_new_empty"))
+ goto out;
+ btf__add_int(btf, "int", 4, BTF_INT_SIGNED);
+ if (!ASSERT_OK(btf__load_into_kernel(btf), "btf_load"))
+ goto out;
+
+ fds[0] = map_fd;
+ fds[1] = btf__fd(btf);
+ memset(&attr, 0, sizeof(attr));
+ attr.prog_type = BPF_PROG_TYPE_SYSCALL;
+ attr.insns = ptr_to_u64(f.gopts.insns);
+ attr.insn_cnt = f.gopts.insns_sz / sizeof(struct bpf_insn);
+ attr.license = ptr_to_u64("Dual BSD/GPL");
+ attr.prog_flags = BPF_F_SLEEPABLE;
+ attr.fd_array = ptr_to_u64(fds);
+ attr.fd_array_cnt = 2;
+ attr.signature = ptr_to_u64(sig);
+ attr.signature_size = sig_sz;
+ attr.keyring_id = KEY_SPEC_SESSION_KEYRING;
+ memcpy(attr.prog_name, "__loader.prog", sizeof("__loader.prog"));
+ prog_fd = syscall(__NR_bpf, BPF_PROG_LOAD, &attr,
+ offsetofend(union bpf_attr, keyring_id));
+ ASSERT_EQ(prog_fd < 0 ? -errno : prog_fd, -EACCES,
+ "BTF in signed fd_array rejected post-verifier");
+ if (prog_fd >= 0)
+ close(prog_fd);
+out:
+ if (btf)
+ btf__free(btf);
+ if (map_fd >= 0)
+ close(map_fd);
+ if (have_fixture)
+ gen_loader_fixture_fini(&f);
+ if (dir)
+ run_setup("cleanup", dir);
+ free(buf);
+}
+
+static void signature_failure_logs(void)
{
static const __u8 junk[64] = { 0x30, 0x42, 0x13, 0x37, };
+ char log_buf[1024] = {};
struct gen_loader_fixture f;
+ union bpf_attr attr;
int fd;
if (gen_loader_fixture_init(&f) == 0) {
/*
- * A present-but-invalid signature (the cert bytes are not a
- * PKCS#7 signature) must be rejected at load: the signature
- * path is honored, not ignored. (The valid path is covered by
- * the signed lskels.)
+ * Signature verification now runs inside bpf_check(), so a
+ * failure is reported through the verifier log. A present-but-
+ * invalid signature is rejected and the log says why.
*/
- fd = load_loader(f.gopts.insns, f.gopts.insns_sz, -1, junk,
- sizeof(junk), KEY_SPEC_SESSION_KEYRING);
+ memset(&attr, 0, sizeof(attr));
+ attr.prog_type = BPF_PROG_TYPE_SYSCALL;
+ attr.insns = ptr_to_u64(f.gopts.insns);
+ attr.insn_cnt = f.gopts.insns_sz / sizeof(struct bpf_insn);
+ attr.license = ptr_to_u64("Dual BSD/GPL");
+ attr.prog_flags = BPF_F_SLEEPABLE;
+ attr.signature = ptr_to_u64(junk);
+ attr.signature_size = sizeof(junk);
+ attr.keyring_id = KEY_SPEC_SESSION_KEYRING;
+ attr.log_level = 1;
+ attr.log_buf = ptr_to_u64(log_buf);
+ attr.log_size = sizeof(log_buf);
+ memcpy(attr.prog_name, "__loader.prog", sizeof("__loader.prog"));
+
+ fd = syscall(__NR_bpf, BPF_PROG_LOAD, &attr,
+ offsetofend(union bpf_attr, keyring_id));
ASSERT_LT(fd, 0, "invalid signature rejected at load");
+ if (fd >= 0)
+ close(fd);
+ ASSERT_HAS_SUBSTR(log_buf, "signature verification failed",
+ "verifier logs signature failure");
}
gen_loader_fixture_fini(&f);
}
@@ -495,7 +531,7 @@ static void signature_too_large(void)
* is rejected before the buffer is read.
*/
fd = load_loader(f.gopts.insns, f.gopts.insns_sz, -1, junk,
- 64 << 20, KEY_SPEC_SESSION_KEYRING);
+ 64 << 20, KEY_SPEC_SESSION_KEYRING, 0);
ASSERT_EQ(fd, -EINVAL, "oversized signature rejected");
}
gen_loader_fixture_fini(&f);
@@ -515,7 +551,7 @@ static void signature_bad_keyring(void)
* large positive serial takes the user-keyring path and won't exist.
*/
fd = load_loader(f.gopts.insns, f.gopts.insns_sz, -1, junk,
- sizeof(junk), INT_MAX);
+ sizeof(junk), INT_MAX, 0);
ASSERT_EQ(fd, -EINVAL, "signature with bad keyring_id rejected");
}
gen_loader_fixture_fini(&f);
@@ -575,7 +611,7 @@ static void metadata_ctx_max_entries_ignored(void)
memcpy(blob, gopts.data, data_sz);
r = run_gen_loader(gopts.insns, gopts.insns_sz, blob, data_sz,
- excl, sizeof(excl), NULL, 0, true, ctx, ctx_sz, &ran);
+ excl, sizeof(excl), NULL, 0, ctx, ctx_sz, &ran);
if (!ASSERT_TRUE(ran, "loader ran") ||
!ASSERT_EQ(r, 0, "loader retval"))
goto free_blob;
@@ -661,7 +697,7 @@ static void metadata_ctx_initial_value_ignored(void)
memcpy(blob, gopts.data, data_sz);
r = run_gen_loader(gopts.insns, gopts.insns_sz, blob, data_sz,
- excl, sizeof(excl), NULL, 0, true, ctx, ctx_sz, &ran);
+ excl, sizeof(excl), NULL, 0, ctx, ctx_sz, &ran);
if (!ASSERT_TRUE(ran, "loader ran") ||
!ASSERT_EQ(r, 0, "loader retval"))
goto free_blob;
@@ -714,6 +750,7 @@ static void signature_authenticates_insns(void)
__u8 excl[SHA256_DIGEST_LENGTH], sig[8192];
__u32 sig_sz = sizeof(sig), insns_sz, data_sz, ctx_sz;
unsigned char *insns = NULL, *tampered = NULL, *blob = NULL;
+ unsigned char *signbuf = NULL;
int nr_maps = 0, nr_progs = 0, r;
struct bpf_program *p;
struct bpf_map *m;
@@ -760,29 +797,46 @@ static void signature_authenticates_insns(void)
memcpy(blob, gopts.data, data_sz);
libbpf_sha256(insns, insns_sz, excl);
- if (!ASSERT_OK(sign_buf(dir, insns, insns_sz, sig, &sig_sz), "sign-file"))
+ signbuf = malloc((size_t)insns_sz + data_sz);
+ if (!ASSERT_OK_PTR(signbuf, "signbuf"))
+ goto cleanup;
+ memcpy(signbuf, insns, insns_sz);
+ memcpy(signbuf + insns_sz, blob, data_sz);
+ if (!ASSERT_OK(sign_buf(dir, signbuf, insns_sz + data_sz, sig, &sig_sz),
+ "sign-file"))
goto cleanup;
memset(ctx, 0, ctx_sz);
((struct bpf_loader_ctx *)ctx)->sz = ctx_sz;
r = run_gen_loader(insns, insns_sz, blob, data_sz, excl, sizeof(excl),
- sig, sig_sz, true, ctx, ctx_sz, &ran);
+ sig, sig_sz, ctx, ctx_sz, &ran);
ASSERT_TRUE(ran, "valid signature: loader loaded and ran");
ASSERT_EQ(r, 0, "valid signature accepted");
close_loader_ctx_fds(ctx, nr_maps, nr_progs);
memcpy(tampered, insns, insns_sz);
tampered[insns_sz / 2] ^= 0xff;
+ /*
+ * Bind the metadata map to the tampered loader's own digest, so the
+ * verifier's exclusive-map check (excl_prog_sha == prog->digest) passes
+ * and the signature - verified after the maps are resolved - is what
+ * rejects the load. This is the attacker's best case: even after
+ * re-binding the exclusive map to their tampered loader, the signature
+ * over the original insns || metadata still fails. (Leaving the map
+ * bound to the original digest would instead trip the excl check first.)
+ */
+ libbpf_sha256(tampered, insns_sz, excl);
memset(ctx, 0, ctx_sz);
((struct bpf_loader_ctx *)ctx)->sz = ctx_sz;
r = run_gen_loader(tampered, insns_sz, blob, data_sz, excl, sizeof(excl),
- sig, sig_sz, true, ctx, ctx_sz, &ran);
+ sig, sig_sz, ctx, ctx_sz, &ran);
ASSERT_FALSE(ran, "tampered loader rejected before run");
ASSERT_EQ(r, -EKEYREJECTED, "signature is bound to the instructions");
cleanup:
free(insns);
free(tampered);
free(blob);
+ free(signbuf);
free(ctx);
test_signed_loader__destroy(skel);
run_setup("cleanup", dir);
@@ -1007,10 +1061,11 @@ static void lsm_signature_verdict(void)
{
char dir_tmpl[] = "/tmp/signed_loader_lsmXXXXXX", *dir = NULL;
struct test_signed_loader_lsm *lsm = NULL;
+ __u32 sig_sz = 8192, msig_sz = 8192;
int map_fd = -1, prog_fd = -1;
bool have_fixture = false;
struct gen_loader_fixture f;
- __u32 sig_sz = 8192;
+ unsigned char *buf;
__s32 ses_serial;
__u8 sig[8192];
@@ -1029,7 +1084,7 @@ static void lsm_signature_verdict(void)
if (!ASSERT_OK_FD(map_fd, "meta_map_unsigned"))
goto out;
lsm->bss->seen = 0;
- prog_fd = load_loader(f.gopts.insns, f.gopts.insns_sz, map_fd, NULL, 0, 0);
+ prog_fd = load_loader(f.gopts.insns, f.gopts.insns_sz, map_fd, NULL, 0, 0, 0);
close(map_fd);
map_fd = -1;
if (!ASSERT_OK_FD(prog_fd, "unsigned loader load"))
@@ -1062,22 +1117,51 @@ static void lsm_signature_verdict(void)
goto out;
lsm->bss->seen = 0;
prog_fd = load_loader(f.gopts.insns, f.gopts.insns_sz, map_fd, sig,
- sig_sz, KEY_SPEC_SESSION_KEYRING);
+ sig_sz, KEY_SPEC_SESSION_KEYRING, 0);
close(map_fd);
map_fd = -1;
- if (!ASSERT_OK_FD(prog_fd, "signed loader load"))
- goto out;
- close(prog_fd);
+ ASSERT_EQ(prog_fd, -EACCES, "unfolded metadata rejected");
+ if (prog_fd >= 0)
+ close(prog_fd);
prog_fd = -1;
ses_serial = syscall(__NR_keyctl, KEYCTL_GET_KEYRING_ID,
KEY_SPEC_SESSION_KEYRING, 0);
ASSERT_EQ(lsm->bss->seen, 1, "signed: one observed load");
- ASSERT_EQ(lsm->bss->sig_verdict, BPF_SIG_VERIFIED, "signed verdict");
+ ASSERT_EQ(lsm->bss->sig_verdict, BPF_SIG_VERIFIED,
+ "admission saw a valid signature");
ASSERT_EQ(lsm->bss->sig_keyring_type, BPF_SIG_KEYRING_USER, "signed keyring type");
ASSERT_GT(ses_serial, 0, "session keyring serial resolved");
ASSERT_EQ(lsm->bss->sig_keyring_serial, ses_serial,
"signed: validated against session keyring");
+
+ buf = malloc((size_t)f.gopts.insns_sz + f.data_sz);
+ if (!ASSERT_OK_PTR(buf, "meta_signbuf"))
+ goto out;
+ memcpy(buf, f.gopts.insns, f.gopts.insns_sz);
+ memcpy(buf + f.gopts.insns_sz, f.blob, f.data_sz);
+ if (!ASSERT_OK(sign_buf(dir, buf, f.gopts.insns_sz + f.data_sz,
+ sig, &msig_sz), "sign insns||metadata")) {
+ free(buf);
+ goto out;
+ }
+ free(buf);
+
+ map_fd = setup_meta_map(&f);
+ if (!ASSERT_OK_FD(map_fd, "meta_map_bound"))
+ goto out;
+ lsm->bss->seen = 0;
+ prog_fd = load_loader(f.gopts.insns, f.gopts.insns_sz, map_fd, sig,
+ msig_sz, KEY_SPEC_SESSION_KEYRING, 1);
+ close(map_fd);
+ map_fd = -1;
+ if (!ASSERT_OK_FD(prog_fd, "metadata-bound loader load"))
+ goto out;
+ close(prog_fd);
+ prog_fd = -1;
+ ASSERT_EQ(lsm->bss->seen, 1, "metadata: one observed load");
+ ASSERT_EQ(lsm->bss->sig_verdict, BPF_SIG_VERIFIED,
+ "metadata-bound verdict");
out:
if (map_fd >= 0)
close(map_fd);
@@ -1090,20 +1174,205 @@ static void lsm_signature_verdict(void)
test_signed_loader_lsm__destroy(lsm);
}
+/*
+ * Load-time metadata verification: the kernel folds the frozen metadata map
+ * into the signature (insns || metadata) and checks it at BPF_PROG_LOAD via
+ * fd_array_cnt, rather than the loader checking from within BPF. Sign that
+ * concatenation, hand the kernel the map, and confirm the signed loader loads,
+ * runs, and installs its target.
+ */
+static int loadtime_drive(const char *dir, const void *insns, __u32 insns_sz,
+ const void *data, __u32 data_sz, const __u8 *excl,
+ void *ctx, __u32 ctx_sz, int *load_ret, bool *ran)
+{
+ LIBBPF_OPTS(bpf_map_create_opts, mopts,
+ .excl_prog_hash = excl,
+ .excl_prog_hash_size = SHA256_DIGEST_LENGTH);
+ __u32 sig_sz = 8192, key = 0;
+ unsigned char *buf = NULL;
+ int map_fd, prog_fd, ret = 0;
+ union bpf_attr attr;
+ __u8 sig[8192];
+
+ *ran = false;
+ *load_ret = 0;
+
+ /*
+ * Metadata map, bound to the loader digest and frozen, exactly as
+ * skel_internal.h's bpf_load_and_run() sets it up.
+ */
+ map_fd = bpf_map_create(BPF_MAP_TYPE_ARRAY, "__loader.map", 4,
+ data_sz, 1, &mopts);
+ if (map_fd < 0)
+ return -errno;
+ if (bpf_map_update_elem(map_fd, &key, data, 0) || bpf_map_freeze(map_fd)) {
+ ret = -errno;
+ goto out_map;
+ }
+
+ /* Sign insns || metadata, the same bytes the kernel reconstructs. */
+ buf = malloc((size_t)insns_sz + data_sz);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto out_map;
+ }
+ memcpy(buf, insns, insns_sz);
+ memcpy(buf + insns_sz, data, data_sz);
+ ret = sign_buf(dir, buf, insns_sz + data_sz, sig, &sig_sz);
+ if (ret)
+ goto out_map;
+
+ memset(&attr, 0, sizeof(attr));
+ attr.prog_type = BPF_PROG_TYPE_SYSCALL;
+ attr.insns = ptr_to_u64(insns);
+ attr.insn_cnt = insns_sz / sizeof(struct bpf_insn);
+ attr.license = ptr_to_u64("Dual BSD/GPL");
+ attr.prog_flags = BPF_F_SLEEPABLE;
+ attr.fd_array = ptr_to_u64(&map_fd);
+ attr.signature = ptr_to_u64(sig);
+ attr.signature_size = sig_sz;
+ attr.keyring_id = KEY_SPEC_SESSION_KEYRING;
+ attr.fd_array_cnt = 1;
+ memcpy(attr.prog_name, "__loader.prog", sizeof("__loader.prog"));
+ prog_fd = syscall(__NR_bpf, BPF_PROG_LOAD, &attr,
+ offsetofend(union bpf_attr, keyring_id));
+ if (prog_fd < 0) {
+ *load_ret = -errno;
+ ret = -errno;
+ goto out_map;
+ }
+
+ memset(&attr, 0, sizeof(attr));
+ attr.test.prog_fd = prog_fd;
+ attr.test.ctx_in = ptr_to_u64(ctx);
+ attr.test.ctx_size_in = ctx_sz;
+ if (syscall(__NR_bpf, BPF_PROG_RUN, &attr,
+ offsetofend(union bpf_attr, test)) < 0) {
+ ret = -errno;
+ goto out_prog;
+ }
+ *ran = true;
+ ret = (int)attr.test.retval;
+out_prog:
+ close(prog_fd);
+out_map:
+ free(buf);
+ close(map_fd);
+ return ret;
+}
+
+static void loadtime_verify(struct bpf_object *obj, int expect_maps)
+{
+ LIBBPF_OPTS(gen_loader_opts, gopts, .gen_hash = true);
+ char dir_tmpl[] = "/tmp/signed_loader_ltXXXXXX", *dir = NULL;
+ int nr_maps = 0, nr_progs = 0, load_ret = 0, r;
+ __u8 excl[SHA256_DIGEST_LENGTH];
+ struct bpf_prog_desc *pd;
+ struct bpf_map_desc *md;
+ unsigned char *blob = NULL;
+ struct bpf_program *p;
+ struct bpf_map *m;
+ __u32 ctx_sz, data_sz;
+ void *ctx = NULL;
+ bool ran = false;
+
+ syscall(__NR_request_key, "keyring", "_uid.0", NULL,
+ KEY_SPEC_SESSION_KEYRING);
+ dir = mkdtemp(dir_tmpl);
+ if (!ASSERT_OK_PTR(dir, "mkdtemp"))
+ return;
+ if (!ASSERT_OK(run_setup("setup", dir), "verify_sig_setup")) {
+ rmdir(dir);
+ return;
+ }
+
+ if (!ASSERT_OK(bpf_object__gen_loader(obj, &gopts), "gen_loader"))
+ goto out;
+ if (!ASSERT_OK(bpf_object__load(obj), "gen_load"))
+ goto out;
+
+ bpf_object__for_each_program(p, obj)
+ nr_progs++;
+ bpf_object__for_each_map(m, obj)
+ nr_maps++;
+ if (!ASSERT_EQ(nr_maps, expect_maps, "fixture map count"))
+ goto out;
+
+ ctx_sz = sizeof(struct bpf_loader_ctx) +
+ nr_maps * sizeof(struct bpf_map_desc) +
+ nr_progs * sizeof(struct bpf_prog_desc);
+ ctx = calloc(1, ctx_sz);
+ if (!ASSERT_OK_PTR(ctx, "ctx_alloc"))
+ goto out;
+ ((struct bpf_loader_ctx *)ctx)->sz = ctx_sz;
+
+ data_sz = gopts.data_sz;
+ blob = malloc(data_sz);
+ if (!ASSERT_OK_PTR(blob, "blob_alloc"))
+ goto out;
+ memcpy(blob, gopts.data, data_sz);
+
+ /* excl_prog_hash = SHA256(loader insns) == the loader's prog->digest. */
+ libbpf_sha256(gopts.insns, gopts.insns_sz, excl);
+
+ r = loadtime_drive(dir, gopts.insns, gopts.insns_sz, blob, data_sz,
+ excl, ctx, ctx_sz, &load_ret, &ran);
+ ASSERT_OK(load_ret, "signed loader loaded (insns || metadata)");
+ ASSERT_TRUE(ran, "loader ran");
+ ASSERT_EQ(r, 0, "loader installed its target");
+
+ md = (struct bpf_map_desc *)((char *)ctx + sizeof(struct bpf_loader_ctx));
+ pd = (struct bpf_prog_desc *)(md + nr_maps);
+ ASSERT_GT(pd[0].prog_fd, 0, "target program installed");
+ if (nr_maps)
+ ASSERT_GT(md[0].map_fd, 0, "target map installed");
+
+ close_loader_ctx_fds(ctx, nr_maps, nr_progs);
+out:
+ free(blob);
+ free(ctx);
+ if (dir)
+ run_setup("cleanup", dir);
+}
+
+static void loadtime_no_map(void)
+{
+ struct test_signed_loader *skel = test_signed_loader__open();
+
+ if (!ASSERT_OK_PTR(skel, "skel_open"))
+ return;
+ loadtime_verify(skel->obj, 0);
+ test_signed_loader__destroy(skel);
+}
+
+static void loadtime_with_map(void)
+{
+ struct test_signed_loader_map *skel = test_signed_loader_map__open();
+
+ if (!ASSERT_OK_PTR(skel, "skel_open"))
+ return;
+ loadtime_verify(skel->obj, 1);
+ test_signed_loader_map__destroy(skel);
+}
+
void test_signed_loader(void)
{
- if (test__start_subtest("metadata_check_shape"))
- metadata_check_shape();
+ if (test__start_subtest("loadtime_no_map"))
+ loadtime_no_map();
+ if (test__start_subtest("loadtime_with_map"))
+ loadtime_with_map();
if (test__start_subtest("metadata_match"))
metadata_match();
- if (test__start_subtest("metadata_sha_mismatch"))
- metadata_sha_mismatch();
- if (test__start_subtest("metadata_not_exclusive"))
- metadata_not_exclusive();
- if (test__start_subtest("metadata_hash_not_computed"))
- metadata_hash_not_computed();
if (test__start_subtest("signature_enforced"))
signature_enforced();
+ if (test__start_subtest("signed_nonexcl_fd_array_rejected"))
+ signed_nonexcl_fd_array_rejected();
+ if (test__start_subtest("signed_nonarray_fd_array_rejected"))
+ signed_nonarray_fd_array_rejected();
+ if (test__start_subtest("signed_btf_fd_array_rejected"))
+ signed_btf_fd_array_rejected();
+ if (test__start_subtest("signature_failure_logs"))
+ signature_failure_logs();
if (test__start_subtest("signature_too_large"))
signature_too_large();
if (test__start_subtest("signature_bad_keyring"))
--
2.43.0
^ permalink raw reply related
* [PATCH bpf-next v2 3/5] bpftool: Cover loader metadata with the program signature
From: Daniel Borkmann @ 2026-06-24 14:02 UTC (permalink / raw)
To: ast
Cc: kpsingh, James.Bottomley, paul, bboscaccy, memxor, torvalds, bpf,
linux-security-module
In-Reply-To: <20260624140301.93421-1-daniel@iogearbox.net>
bpftool_prog_sign() signed only the loader instructions. The metadata
blob the loader installs was left to an in-loader hash check, which
the kernel now performs at load time over insns || metadata.
Sign that same concatenation: pass the metadata blob (gen_loader_opts
data) through to bpftool_prog_sign() and feed insns || metadata to
CMS_final(). The excl_prog_hash stays a digest of the instructions
alone; it binds the metadata map to the loader and is matched against
prog->digest by the verifier, independent of what the signature covers.
The signed artifact is now plain data: both bytes the signature
covers are embedded verbatim in the generated skeleton, so signing
and verifying an lskel is an ordinary CMS operation that a signer or
auditor can perform (or reproduce) offline, without analyzing loader
bytecode to establish what the signature actually attests to [0].
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/lkml/ecf0521ed302db672672ebfbc670ecfba36a6e00.camel@HansenPartnership.com [0]
---
tools/bpf/bpftool/gen.c | 2 ++
tools/bpf/bpftool/sign.c | 15 ++++++++++++++-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 6ae7262ebe0c..a01d06d22d1a 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -793,6 +793,8 @@ static int gen_trace(struct bpf_object *obj, const char *obj_name, const char *h
if (sign_progs) {
sopts.insns = opts.insns;
sopts.insns_sz = opts.insns_sz;
+ sopts.data = opts.data;
+ sopts.data_sz = opts.data_sz;
sopts.excl_prog_hash = prog_sha;
sopts.excl_prog_hash_sz = sizeof(prog_sha);
sopts.signature = sig_buf;
diff --git a/tools/bpf/bpftool/sign.c b/tools/bpf/bpftool/sign.c
index f9b742f4bb10..4ce020a141dc 100644
--- a/tools/bpf/bpftool/sign.c
+++ b/tools/bpf/bpftool/sign.c
@@ -135,9 +135,21 @@ int bpftool_prog_sign(struct bpf_load_and_run_opts *opts)
CMS_ContentInfo *cms = NULL;
long actual_sig_len = 0;
X509 *x509 = NULL;
+ void *data = NULL;
+ size_t data_sz;
int err = 0;
- bd_in = BIO_new_mem_buf(opts->insns, opts->insns_sz);
+ data_sz = (size_t)opts->insns_sz + opts->data_sz;
+ data = malloc(data_sz);
+ if (!data) {
+ err = -ENOMEM;
+ goto cleanup;
+ }
+ memcpy(data, opts->insns, opts->insns_sz);
+ if (opts->data_sz)
+ memcpy((char *)data + opts->insns_sz, opts->data, opts->data_sz);
+
+ bd_in = BIO_new_mem_buf(data, data_sz);
if (!bd_in) {
err = -ENOMEM;
goto cleanup;
@@ -212,6 +224,7 @@ int bpftool_prog_sign(struct bpf_load_and_run_opts *opts)
X509_free(x509);
EVP_PKEY_free(private_key);
BIO_free(bd_in);
+ free(data);
DISPLAY_OSSL_ERR(err < 0);
return err;
}
--
2.43.0
^ permalink raw reply related
* [PATCH bpf-next v2 2/5] libbpf: Drop in-loader metadata check for load-time verification
From: Daniel Borkmann @ 2026-06-24 14:02 UTC (permalink / raw)
To: ast
Cc: kpsingh, James.Bottomley, paul, bboscaccy, memxor, torvalds, bpf,
linux-security-module
In-Reply-To: <20260624140301.93421-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. 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>
---
kernel/bpf/syscall.c | 5 ---
tools/lib/bpf/bpf_gen_internal.h | 1 -
tools/lib/bpf/gen_loader.c | 76 +++-----------------------------
tools/lib/bpf/skel_internal.h | 27 +-----------
4 files changed, 9 insertions(+), 100 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 026b61d78bdb..3eeeaf62c456 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1599,11 +1599,6 @@ static int map_create_alloc(union bpf_attr *attr, bpfptr_t uattr, struct bpf_ver
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");
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 d79695f01c87..baed23850997 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/skel_internal.h b/tools/lib/bpf/skel_internal.h
index 74503d358bc8..8555ab8af554 100644
--- a/tools/lib/bpf/skel_internal.h
+++ b/tools/lib/bpf/skel_internal.h
@@ -320,25 +320,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 +365,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 +375,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 v2 1/5] bpf: Verify signed loader metadata at load time
From: Daniel Borkmann @ 2026-06-24 14:02 UTC (permalink / raw)
To: ast
Cc: kpsingh, James.Bottomley, paul, bboscaccy, memxor, torvalds, bpf,
linux-security-module
In-Reply-To: <20260624140301.93421-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 [0]. 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 [1][2]. 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 [3]. Let's implement the former,
without growing the UAPI.
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. 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.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/CAHC9VhSDkwGgPfrBUh7EgBKEJj_JjnY68c0YAmuuLT_i--GskQ@mail.gmail.com [0]
Link: https://lore.kernel.org/bpf/2f71d6c03698eb17d51f7247efde777627ee578a.camel@HansenPartnership.com [1]
Link: https://lore.kernel.org/lkml/ecf0521ed302db672672ebfbc670ecfba36a6e00.camel@HansenPartnership.com [2]
Link: https://lore.kernel.org/bpf/88703f00d5b7a779728451008626efa45e42db3d.camel@HansenPartnership.com [3]
---
include/linux/bpf_verifier.h | 1 +
kernel/bpf/syscall.c | 76 +---------------
kernel/bpf/verifier.c | 163 ++++++++++++++++++++++++++++++++++-
3 files changed, 165 insertions(+), 75 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 39a851e690ec..1431cf7c620d 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -939,6 +939,7 @@ struct bpf_verifier_env {
bool bypass_spec_v4;
bool seen_direct_write;
bool seen_exception;
+ bool check_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 b44106c8ea75..026b61d78bdb 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 2abc79dbf281..9cd2b62da380 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>
@@ -19703,12 +19705,146 @@ 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 > 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 | __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)
{
u64 start_time = ktime_get_ns();
struct bpf_verifier_env *env;
int i, len, ret = -EINVAL, err;
+ u32 signed_map_cnt = 0;
bool is_priv;
BTF_TYPE_EMIT(enum bpf_features);
@@ -19745,6 +19881,7 @@ 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);
+ env->check_signature = attr->signature;
bpf_get_btf_vmlinux();
@@ -19758,11 +19895,28 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
ret = bpf_vlog_init(&env->log, attr_log->level, attr_log->ubuf, attr_log->size);
if (ret)
goto err_unlock;
+ if (env->check_signature) {
+ ret = bpf_prog_calc_tag(env->prog);
+ if (ret < 0)
+ goto skip_full_check;
+ }
ret = process_fd_array(env, attr, uattr);
if (ret)
goto skip_full_check;
+ if (env->check_signature) {
+ ret = bpf_prog_verify_signature(env, attr, uattr.is_kernel);
+ if (ret)
+ goto skip_full_check;
+ signed_map_cnt = env->used_map_cnt;
+ }
+
+ ret = security_bpf_prog_load(env->prog, attr, env->prog->aux->token,
+ uattr.is_kernel);
+ if (ret)
+ goto skip_full_check;
+
mark_verifier_state_clean(env);
if (IS_ERR(btf_vmlinux)) {
@@ -19812,7 +19966,14 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
ret = check_and_resolve_insns(env);
if (ret < 0)
goto skip_full_check;
-
+ if (env->prog->aux->sig.verdict == BPF_SIG_VERIFIED &&
+ (env->used_map_cnt != signed_map_cnt || env->used_btf_cnt)) {
+ verbose(env, "signed program uses %s not covered by the signature\n",
+ env->used_map_cnt != signed_map_cnt ?
+ (env->used_btf_cnt ? "maps and BTF" : "maps") : "BTF");
+ ret = -EACCES;
+ goto skip_full_check;
+ }
if (bpf_prog_is_offloaded(env->prog->aux)) {
ret = bpf_prog_offload_verifier_prep(env->prog);
if (ret)
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] landlock: shrink tsync works[] on partial allocation failure
From: Hao Peng @ 2026-06-24 12:47 UTC (permalink / raw)
To: Günther Noack; +Cc: mic, linux-security-module
In-Reply-To: <ajpVtFfp0ZxJUVc6@google.com>
On Tue, Jun 23, 2026 at 5:45 PM Günther Noack <gnoack@google.com> wrote:
>
> Hello Peng!
>
> Thanks for your patch!
>
> On Tue, Jun 23, 2026 at 01:01:27PM +0800, Peng Hao wrote:
> > When the per-slot kzalloc fails mid-loop in tsync_works_grow_by(), the
> > already-enlarged s->works array keeps uninitialized trailing entries.
> > Shrink the array back to its used size on the error path so no waste
> > is carried over: free it outright when nothing has been allocated yet,
> > otherwise try a shrinking krealloc_array() (keep the larger array if
> > the shrink fails, since tsync_works_release() honors s->capacity).
> >
> > Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> > ---
> > security/landlock/tsync.c | 18 ++++++++++++++++--
> > 1 file changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/landlock/tsync.c b/security/landlock/tsync.c
> > index c5730bbd..356ce94b 100644
> > --- a/security/landlock/tsync.c
> > +++ b/security/landlock/tsync.c
> > @@ -272,9 +272,23 @@ static int tsync_works_grow_by(struct tsync_works *s, size_t n, gfp_t flags)
> > work = kzalloc_obj(*work, flags);
> > if (!work) {
> > /*
> > - * Leave the object in a consistent state,
> > - * but return an error.
> > + * Leave the object in a consistent state, but return
> > + * an error. Shrink @s->works back to its used size to
> > + * avoid carrying uninitialized trailing entries. A
> > + * shrinking krealloc_array() should normally succeed,
> > + * but if it does not we simply keep the larger array;
> > + * tsync_works_release() iterates only up to capacity.
> > */
> > + if (i == 0) {
> > + kfree(s->works);
> > + s->works = NULL;
> > + } else {
> > + works = krealloc_array(s->works, i,
> > + sizeof(s->works[0]),
> > + flags | __GFP_NOWARN);
> > + if (works)
> > + s->works = works;
> > + }
> > s->capacity = i;
> > return -ENOMEM;
> > }
>
> Can you please clarify your motivation for this?
>
> To paraphrase my understanding
>
> * You are not addressing a logic bug
>
> The invariant for that data structure is that s->size <= s->capacity
> and s->capacity <= number of elements in the array <= number of
> sibling threads.
>
> If the array is slightly larger than the capacity, that does not break
> the invariant and should not result in out-of-bounds accesses.
>
> * You are addressing that the array is a bit larger than the capacity
>
> This is in the case where kzalloc_obj() failed. We set the capacity
> to i (making sure that only the objects 0 to i-1 are being looked at),
> and we return an error. Sure, the array is overallocated a little
> bit, but now that we are returning an error, the caller will
> fast-track to abort the tsync due to ENOMEM and the array does anyway
> get released very soon now.
>
>
> The state where we use slightly more memory (with number of entries <=
> number of sibling threads) is supposed to be very transient.
>
> Is the delay between raising the error and the final kfree() long enough
> that you have seen it cause problems in practice?
>
> Thanks,
> —Günther
You are right; the time between the error being triggered and the call to
kfree() is very short, so this patch is of little significance.
Thanks.
^ permalink raw reply
* [GIT PULL] AppArmor updates for 7.2-rc1
From: John Johansen @ 2026-06-24 7:52 UTC (permalink / raw)
To: Linus Torvalds; +Cc: LKLM, open list:SECURITY SUBSYSTEM, Georgia Garcia
Hi Linus,
This is another round of bug fixing and some code cleanups, there are no
new features. I ended up pulling in a few bug fixes late, and gave them
some extra time to bake.
The biggest thing to note is Georgia is being added to help co-maintain
apparmor.
Everything has been merge, build, and regression tested against your tree
for Monday June 22.
thanks
- john
The following changes since commit 254f49634ee16a731174d2ae34bc50bd5f45e731:
Linux 7.1-rc1 (2026-04-26 14:19:00 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor tags/apparmor-pr-2026-06-22
for you to fetch changes up to 2f6701a5ce6257ae7a64ddc6d89d0a08d2a034f8:
apparmor: advertise the tcp fast open fix is applied (2026-06-23 22:15:15 -0700)
----------------------------------------------------------------
* add Georgia Garcia as co-maintainer of apparmor
* Cleanups
- replace get_zeroed_page() with kzalloc()
- remove unnecessary goto and associated label
- change fn_label_build() to return err on failure instead of NULL or err
- free rawdata as soon as possible
- use explicit instead of implicit flex array in rawdata_f_data
- use __label_make_stale in __aa_proxy_redirect
- return correct error by propagate -ENOMEM correctly in unpack_table
- aa_label_alloc use aa_label_free on alloc failure
- add a conditional version of get_newest_label
* Bug Fixes
- mediate the implicit connect of TCP fast open sendmsg
- fix C23ism of label immediately before a declaration
- fix kernel-doc warnings
- fix spelling mistakes
- fix use-after-free in rawdata dedup loop
- Fix inverted comparison in cache_hold_inc()
- fix uninitialized pointer passed to audit_log_untrustedstring()
- don't audit files pointing to aa_null.dentry
- put secmark label after secid lookup
- fix aa_getprocattr free procattr leak on format failure
- release exe file resources on path failure
- fail policy unpack on accept2 allocation failure
- Fix return in ns_mkdir_op
- remove or add symlinks to rawdata according to export_binary
- fix NULL pointer dereference in unpack_pdb
- fix potential UAF in aa_replace_profiles
- grab ns lock and refresh when looking up changehat child profiles
- enable differential encoding
- check label build before no_new_privs test
- conditionally compile get_loaddata_common_ref()
- fix unix socket mediation cache update, and leak
----------------------------------------------------------------
Andrew Morton (1):
security/apparmor/apparmorfs.c: conditionally compile get_loaddata_common_ref()
Bryam Vargas (1):
apparmor: mediate the implicit connect of TCP fast open sendmsg
Eduardo Vasconcelos (1):
apparmor: Fix inverted comparison in cache_hold_inc()
Georgia Garcia (3):
apparmor: fix NULL pointer dereference in unpack_pdb
apparmor: remove or add symlinks to rawdata according to export_binary
apparmor: don't audit files pointing to aa_null.dentry
Hongling Zeng (1):
apparmor: Fix return in ns_mkdir_op
John Johansen (13):
apparmor: add Georgia Garcia as co-maintainer of apparmor
apparmor: fix shadowing of plabel that prevents cache from being updated
apparmor: fix race in unix socket mediation when peer_path is used
apparmor: fix refcount leak when updating the sk_ctx
apparmor: add a conditional version of get_newest_label
apparmor: enable differential encoding
apparmor: fix rawdata_f_data implicit flex array
apparmor: free rawdata as soon as possible
apparmor: change fn_label_build() call to not return NULL
apparmor: make fn_label_build() capable of handling not supported
apparmor: remove unnecessary goto and associated label
apparmor: fix label can not be immediately before a declaration
apparmor: advertise the tcp fast open fix is applied
Maciek Borzecki (1):
apparmor: fix uninitialised pointer passed to audit_log_untrustedstring()
Maxime Bélair (2):
apparmor: propagate -ENOMEM correctly in unpack_table
apparmor: fix potential UAF in aa_replace_profiles
Mike Rapoport (Microsoft) (1):
apparmor: replace get_zeroed_page() with kzalloc()
Qingshuang Fu (1):
security: apparmor: fix two spelling mistakes
Rodrigo Zaiden (1):
apparmor: fix kernel-doc warnings
Ruoyu Wang (1):
apparmor: check label build before no_new_privs test
Ruslan Valiyev (1):
apparmor: fix use-after-free in rawdata dedup loop
Ryan Lee (2):
apparmor: use __label_make_stale in __aa_proxy_redirect
apparmor: grab ns lock and refresh when looking up changehat child profiles
Zygmunt Krynicki (5):
apparmor: aa_label_alloc use aa_label_free on alloc failure
apparmor: fail policy unpack on accept2 allocation failure
apparmor: release exe file resources on path failure
apparmor: aa_getprocattr free procattr leak on format failure
apparmor: put secmark label after secid lookup
MAINTAINERS | 1 +
security/apparmor/af_unix.c | 73 +++++++++---------
security/apparmor/apparmorfs.c | 119 ++++++++++++++++++++++--------
security/apparmor/domain.c | 97 ++++++++++++++++--------
security/apparmor/file.c | 12 +--
security/apparmor/include/apparmorfs.h | 12 +++
security/apparmor/include/label.h | 32 ++++++++
security/apparmor/include/lib.h | 21 +++---
security/apparmor/include/policy_unpack.h | 21 +++++-
security/apparmor/label.c | 26 +++----
security/apparmor/lsm.c | 20 ++++-
security/apparmor/match.c | 22 +++---
security/apparmor/mount.c | 17 ++---
security/apparmor/net.c | 3 +
security/apparmor/policy.c | 35 ++++++++-
security/apparmor/policy_unpack.c | 6 +-
security/apparmor/procattr.c | 2 +
security/apparmor/task.c | 2 +-
18 files changed, 368 insertions(+), 153 deletions(-)
^ permalink raw reply
* Re: [PATCH v3 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: Paul Moore @ 2026-06-24 0:12 UTC (permalink / raw)
To: David Windsor, viro, brauner, jack, ast, daniel, john.fastabend,
andrii, eddyz87, memxor, martin.lau, song, yonghong.song, jolsa,
emil, kpsingh, mattbobrowski, jmorris, serge, zohar,
roberto.sassu, dmitry.kasatkin, eric.snowberg,
stephen.smalley.work, omosnace, casey, shuah
Cc: linux-kernel, linux-fsdevel, bpf, linux-security-module,
linux-integrity, selinux, linux-kselftest, David Windsor
In-Reply-To: <20260618203411.73917-2-dwindsor@gmail.com>
On Jun 18, 2026 David Windsor <dwindsor@gmail.com> wrote:
>
> Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set
> xattrs via the inode_init_security hook using lsm_get_xattr_slot().
>
> The inode_init_security hook previously took the xattr array and count
> as two separate output parameters (struct xattr *xattrs, int
> *xattr_count), which BPF programs cannot write to. Pass the xattr state
> as a single context object (struct xattr_ctx) instead, and have
> bpf_init_inode_xattr() take that context directly. Update the existing
> in-tree callers of inode_init_security to take and forward the new
> xattr_ctx.
>
> 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.
>
> Because we rely on the hook-specific ctx layout, the kfunc is
> restricted to lsm/inode_init_security. Restrict the xattr names that
> may be set via this kfunc to the bpf.* namespace.
>
> 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 | 106 +++++++++++++++++++++++++++++-
> include/linux/bpf.h | 1 +
> include/linux/bpf_lsm.h | 3 +
> include/linux/evm.h | 9 +--
> include/linux/lsm_hook_defs.h | 4 +-
> include/linux/lsm_hooks.h | 16 ++---
> include/linux/security.h | 5 ++
> kernel/bpf/bpf_lsm.c | 10 +++
> kernel/bpf/trampoline.c | 3 +
> security/bpf/hooks.c | 1 +
> security/integrity/evm/evm_main.c | 8 ++-
> security/security.c | 7 +-
> security/selinux/hooks.c | 4 +-
> security/smack/smack_lsm.c | 27 ++++----
> 14 files changed, 166 insertions(+), 38 deletions(-)
I have a few specific comments below, inline with the patch, but I wanted
to make some general comments too.
The kfunc additions really don't belong in the VFS kfunc file, please
create a LSM kfunc file (call it security/bpf_lsm_kfuncs.c) and add the
kfunc code to this new file.
While moving the kfunc additions to a LSM kfunc file does sort of convert
the VFS changes into LSM changes, Christian's comment about splitting
this patch two patches, one with the LSM hook changes and one with the
BPF additions, is still reasonable and a good suggestion. I would still
CC the VFS folks on these patches and I would encourage reviews from the
VFS folks as there is a VFS component here, albeit a somewhat small one.
> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> index 768aca2dc0f0..7abc3f3d1a67 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>
>
> @@ -374,6 +375,97 @@ __bpf_kfunc struct inode *bpf_real_inode(struct dentry *dentry)
> return d_real_inode(dentry);
> }
>
> +static int bpf_xattrs_used(const struct xattr_ctx *ctx)
> +{
> + const size_t prefix_len = sizeof(XATTR_BPF_LSM_SUFFIX) - 1;
> + int i, n = 0;
> +
> + for (i = 0; i < *ctx->xattr_count; i++) {
> + const char *name = ctx->xattrs[i].name;
> +
> + if (name && !strncmp(name, XATTR_BPF_LSM_SUFFIX, prefix_len))
> + n++;
> + }
> + return n;
> +}
> +
> +static int __bpf_init_inode_xattr(struct xattr_ctx *xattr_ctx,
> + 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;
> + struct xattr *xattrs;
> + int *xattr_count;
> + const void *value;
> + u32 value_len;
> +
> + if (!xattr_ctx || !name__str)
> + return -EINVAL;
> +
> + xattrs = xattr_ctx->xattrs;
> + xattr_count = xattr_ctx->xattr_count;
I'm not sure why the "xattrs" and "xattrs_count" local variables are
necessary, especially since they only appear to be used in the sanity
check below. I would suggest just using the values in the struct
directly.
> + if (!xattrs || !xattr_count)
> + return -EINVAL;
> + if (bpf_xattrs_used(xattr_ctx) >= BPF_LSM_INODE_INIT_XATTRS)
> + return -ENOSPC;
> +
> + name_len = strlen(name__str);
> + if (name_len == 0 || name_len > XATTR_NAME_MAX)
> + return -EINVAL;
> + if (strncmp(name__str, XATTR_BPF_LSM_SUFFIX,
> + sizeof(XATTR_BPF_LSM_SUFFIX) - 1))
> + return -EPERM;
> +
> + 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;
> +
> + /* Combine xattr value + name into one allocation. */
> + xattr_value = kmalloc(value_len + name_len + 1, GFP_KERNEL);
> + if (!xattr_value)
> + return -ENOMEM;
> +
> + memcpy(xattr_value, value, value_len);
> + memcpy(xattr_value + value_len, name__str, name_len);
> + ((char *)xattr_value)[value_len + name_len] = '\0';
> +
> + xattr = lsm_get_xattr_slot(xattr_ctx);
> + if (!xattr) {
> + kfree(xattr_value);
> + return -ENOSPC;
> + }
> +
> + xattr->value = xattr_value;
> + xattr->name = (const char *)xattr_value + value_len;
> + xattr->value_len = value_len;
> +
> + return 0;
> +}
> +
> +/**
> + * bpf_init_inode_xattr - set an xattr on a new inode from inode_init_security
> + * @xattr_ctx: inode_init_security xattr state from the hook context
> + * @name__str: xattr name (e.g., "bpf.file_label")
> + * @value_p: dynptr containing the xattr value
> + *
> + * Only callable from lsm/inode_init_security programs.
> + *
> + * Return: 0 on success, negative error on failure.
> + */
> +__bpf_kfunc int bpf_init_inode_xattr(struct xattr_ctx *xattr_ctx,
> + const char *name__str,
> + const struct bpf_dynptr *value_p)
> +{
> + return __bpf_init_inode_xattr(xattr_ctx, name__str, value_p);
> +}
I'm sure there is a reason why you split the code out into
__bpf_init_inode_xattr() as opposed to just putting it directly in this
kfunc, can you help me understand?
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 153e9043058f..1f8e84e7dd7e 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -68,6 +68,11 @@ struct watch;
> struct watch_notification;
> struct lsm_ctx;
>
> +struct xattr_ctx {
> + struct xattr *xattrs;
> + int *xattr_count;
> +};
I see the bots already pointed this out, and you acknowledged it, but
since I'm looking at this I felt obliged to remind you once again about
the rename to "struct lsm_xattrs" :)
Also, looking at this closer, is there a reason why the "xattr_count"
field is an integer pointer and not just an integer? We're passing
the entire struct by reference to the individual LSMs so we shouldn't
need this to get an updated count in the caller and having it as a
regular integer should simplify things slightly (you could also
make it an unsigned int while you are it).
> @@ -315,6 +324,7 @@ BTF_ID(func, bpf_lsm_inode_create)
> BTF_ID(func, bpf_lsm_inode_free_security)
> BTF_ID(func, bpf_lsm_inode_getattr)
> BTF_ID(func, bpf_lsm_inode_getxattr)
> +BTF_ID(func, bpf_lsm_inode_init_security)
> BTF_ID(func, bpf_lsm_inode_mknod)
> BTF_ID(func, bpf_lsm_inode_need_killpriv)
> BTF_ID(func, bpf_lsm_inode_post_setxattr)
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index 1a721fc4bef5..b41b02173e24 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -859,6 +859,9 @@ static int bpf_trampoline_add_prog(struct bpf_trampoline *tr,
> }
> if (cnt >= BPF_MAX_TRAMP_LINKS)
> return -E2BIG;
> + if (node->link->prog->aux->attach_limit &&
> + tr->progs_cnt[kind] >= node->link->prog->aux->attach_limit)
> + return -E2BIG;
Re: Alexei's comments about this - if you look back at my previous
comments, my concern was around BPF LSMs using too many slots in the
xattr array and causing issues. If the BPF folks want to do that check
in the kfunc located in the LSM framework I'm okay with that; the
important part is that the BPF LSMs don't use more space than they
previously requested.
> diff --git a/security/security.c b/security/security.c
> index 71aea8fdf014..8f82a1352356 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1334,6 +1334,7 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> {
> struct lsm_static_call *scall;
> struct xattr *new_xattrs = NULL;
> + struct xattr_ctx xattr_ctx;
> int ret = -EOPNOTSUPP, xattr_count = 0;
Since we have the xattr array/pointer and count in the new "lsm_xattrs"
struct, I think we can remove the "new_xattrs" and "xattr_count" local
variables in favor of the fields in the new struct.
> if (unlikely(IS_PRIVATE(inode)))
> @@ -1349,10 +1350,12 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
> if (!new_xattrs)
> return -ENOMEM;
> }
> + xattr_ctx.xattrs = new_xattrs;
> + xattr_ctx.xattr_count = &xattr_count;
>
> lsm_for_each_hook(scall, inode_init_security) {
> - ret = scall->hl->hook.inode_init_security(inode, dir, qstr, new_xattrs,
> - &xattr_count);
> + ret = scall->hl->hook.inode_init_security(inode, dir, qstr,
> + &xattr_ctx);
> if (ret && ret != -EOPNOTSUPP)
> goto out;
> /*
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v17 06/10] rust: rename `AlwaysRefCounted` to `RefCounted`.
From: Onur Özkan @ 2026-06-23 17:58 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Daniel Almeida, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
Krzysztof Wilczyński, Boqun Feng, Uladzislau Rezki,
Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett, Igor Korotin,
Pavel Tikhomirov, linux-kernel, rust-for-linux, linux-block,
linux-security-module, dri-devel, linux-fsdevel, linux-mm,
linux-pm, linux-pci, driver-core, Oliver Mangold, Viresh Kumar
In-Reply-To: <20260604-unique-ref-v17-6-7b4c3d2930b9@kernel.org>
On Thu, 04 Jun 2026 22:11:18 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:
> From: Oliver Mangold <oliver.mangold@pm.me>
>
> There are types where it may both be reference counted in some cases and
> owned in others. In such cases, obtaining `ARef<T>` from `&T` would be
> unsound as it allows creation of `ARef<T>` copy from `&Owned<T>`.
>
> Therefore, we split `AlwaysRefCounted` into `RefCounted` (which `ARef<T>`
> would require) and a marker trait to indicate that the type is always
> reference counted (and not `Ownable`) so the `&T` -> `ARef<T>` conversion
> is possible.
>
> - Rename `AlwaysRefCounted` to `RefCounted`.
> - Add a new unsafe trait `AlwaysRefCounted`.
> - Implement the new trait `AlwaysRefCounted` for the newly renamed
> `RefCounted` implementations. This leaves functionality of existing
> implementers of `AlwaysRefCounted` intact.
>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
> [ Andreas: Updated commit message and rebase on rust-6.20-7.0 ]
> Acked-by: Igor Korotin <igor.korotin.linux@gmail.com>
> Acked-by: Danilo Krummrich <dakr@kernel.org>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/auxiliary.rs | 7 +++++-
> rust/kernel/block/mq/request.rs | 15 ++++++++-----
> rust/kernel/cred.rs | 13 +++++++++--
> rust/kernel/device.rs | 12 ++++++++--
> rust/kernel/device/property.rs | 11 +++++++--
> rust/kernel/drm/device.rs | 9 ++++++--
> rust/kernel/drm/gem/mod.rs | 16 ++++++++++----
> rust/kernel/fs/file.rs | 16 ++++++++++----
> rust/kernel/i2c.rs | 13 ++++++++---
> rust/kernel/mm.rs | 15 +++++++++----
> rust/kernel/mm/mmput_async.rs | 9 ++++++--
> rust/kernel/opp.rs | 10 ++++++---
> rust/kernel/owned.rs | 2 +-
> rust/kernel/pci.rs | 10 ++++++++-
> rust/kernel/pid_namespace.rs | 12 ++++++++--
> rust/kernel/platform.rs | 7 +++++-
> rust/kernel/sync/aref.rs | 49 ++++++++++++++++++++++++++---------------
> rust/kernel/task.rs | 13 +++++++++--
> rust/kernel/types.rs | 3 ++-
> rust/kernel/usb.rs | 17 +++++++++++---
> 20 files changed, 195 insertions(+), 64 deletions(-)
>
> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
> index 93c0db1f6655..49f07740f657 100644
> --- a/rust/kernel/auxiliary.rs
> +++ b/rust/kernel/auxiliary.rs
> @@ -19,6 +19,7 @@
> to_result, //
> },
> prelude::*,
> + sync::aref::{AlwaysRefCounted, RefCounted},
This patch has multiple horizontal use statements around.
Onur
> types::Opaque,
> ThisModule, //
> };
> @@ -289,7 +290,7 @@ unsafe impl<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for Device<Ctx>
> kernel::impl_device_context_into_aref!(Device);
>
> // SAFETY: Instances of `Device` are always reference-counted.
> -unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
> +unsafe impl RefCounted for Device {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::get_device(self.as_ref().as_raw()) };
> @@ -308,6 +309,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
> +// `&Device`.
> +unsafe impl AlwaysRefCounted for Device {}
> +
> impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
> fn as_ref(&self) -> &device::Device<Ctx> {
> // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
> index ce3e30c81cb5..cf013b9e2cac 100644
> --- a/rust/kernel/block/mq/request.rs
> +++ b/rust/kernel/block/mq/request.rs
> @@ -9,7 +9,7 @@
> block::mq::Operations,
> error::Result,
> sync::{
> - aref::{ARef, AlwaysRefCounted},
> + aref::{ARef, AlwaysRefCounted, RefCounted},
> atomic::Relaxed,
> Refcount,
> },
> @@ -229,11 +229,10 @@ unsafe impl<T: Operations> Send for Request<T> {}
> // mutate `self` are internally synchronized`
> unsafe impl<T: Operations> Sync for Request<T> {}
>
> -// SAFETY: All instances of `Request<T>` are reference counted. This
> -// implementation of `AlwaysRefCounted` ensure that increments to the ref count
> -// keeps the object alive in memory at least until a matching reference count
> -// decrement is executed.
> -unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {
> +// SAFETY: All instances of `Request<T>` are reference counted. This implementation of `RefCounted`
> +// ensure that increments to the ref count keeps the object alive in memory at least until a
> +// matching reference count decrement is executed.
> +unsafe impl<T: Operations> RefCounted for Request<T> {
> fn inc_ref(&self) {
> self.wrapper_ref().refcount().inc();
> }
> @@ -255,3 +254,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
> }
> }
> }
> +
> +// SAFETY: We currently do not implement `Ownable`, thus it is okay to obtain an `ARef<Request>`
> +// from a `&Request` (but this will change in the future).
> +unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {}
> diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs
> index ffa156b9df37..20ef0144094b 100644
> --- a/rust/kernel/cred.rs
> +++ b/rust/kernel/cred.rs
> @@ -8,7 +8,12 @@
> //!
> //! Reference: <https://www.kernel.org/doc/html/latest/security/credentials.html>
>
> -use crate::{bindings, sync::aref::AlwaysRefCounted, task::Kuid, types::Opaque};
> +use crate::{
> + bindings,
> + sync::aref::RefCounted,
> + task::Kuid,
> + types::{AlwaysRefCounted, Opaque},
> +};
>
> /// Wraps the kernel's `struct cred`.
> ///
> @@ -76,7 +81,7 @@ pub fn euid(&self) -> Kuid {
> }
>
> // SAFETY: The type invariants guarantee that `Credential` is always ref-counted.
> -unsafe impl AlwaysRefCounted for Credential {
> +unsafe impl RefCounted for Credential {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> @@ -90,3 +95,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Credential>) {
> unsafe { bindings::put_cred(obj.cast().as_ptr()) };
> }
> }
> +
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Credential>` from a
> +// `&Credential`.
> +unsafe impl AlwaysRefCounted for Credential {}
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 6d5396a43ebe..efdf33617d12 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -8,8 +8,12 @@
> bindings,
> fmt,
> prelude::*,
> - sync::aref::ARef,
> + sync::aref::{
> + ARef,
> + RefCounted, //
> + },
> types::{
> + AlwaysRefCounted,
> ForeignOwnable,
> Opaque, //
> }, //
> @@ -508,7 +512,7 @@ pub fn name(&self) -> &CStr {
> kernel::impl_device_context_into_aref!(Device);
>
> // SAFETY: Instances of `Device` are always reference-counted.
> -unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
> +unsafe impl RefCounted for Device {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::get_device(self.as_raw()) };
> @@ -520,6 +524,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
> +// `&Device`.
> +unsafe impl AlwaysRefCounted for Device {}
> +
> // SAFETY: As by the type invariant `Device` can be sent to any thread.
> unsafe impl Send for Device {}
>
> diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs
> index 5aead835fbbc..cee7e2501368 100644
> --- a/rust/kernel/device/property.rs
> +++ b/rust/kernel/device/property.rs
> @@ -14,7 +14,10 @@
> fmt,
> prelude::*,
> str::{CStr, CString},
> - sync::aref::ARef,
> + sync::aref::{
> + ARef,
> + AlwaysRefCounted, //
> + },
> types::Opaque,
> };
>
> @@ -360,7 +363,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> }
>
> // SAFETY: Instances of `FwNode` are always reference-counted.
> -unsafe impl crate::sync::aref::AlwaysRefCounted for FwNode {
> +unsafe impl crate::sync::aref::RefCounted for FwNode {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the
> // refcount is non-zero.
> @@ -374,6 +377,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<FwNode>` from a
> +// `&FwNode`.
> +unsafe impl AlwaysRefCounted for FwNode {}
> +
> enum Node<'a> {
> Borrowed(&'a FwNode),
> Owned(ARef<FwNode>),
> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
> index adbafe8db54d..a5a040266aae 100644
> --- a/rust/kernel/drm/device.rs
> +++ b/rust/kernel/drm/device.rs
> @@ -15,7 +15,8 @@
> prelude::*,
> sync::aref::{
> ARef,
> - AlwaysRefCounted, //
> + AlwaysRefCounted,
> + RefCounted, //
> },
> types::Opaque,
> workqueue::{
> @@ -217,7 +218,7 @@ fn deref(&self) -> &Self::Target {
>
> // SAFETY: DRM device objects are always reference counted and the get/put functions
> // satisfy the requirements.
> -unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {
> +unsafe impl<T: drm::Driver> RefCounted for Device<T> {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::drm_dev_get(self.as_raw()) };
> @@ -232,6 +233,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
> +// `&Device`.
> +unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {}
> +
> impl<T: drm::Driver> AsRef<device::Device> for Device<T> {
> fn as_ref(&self) -> &device::Device {
> // SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid,
> diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
> index 75acda7ba500..f8cc2a0ff4c7 100644
> --- a/rust/kernel/drm/gem/mod.rs
> +++ b/rust/kernel/drm/gem/mod.rs
> @@ -17,7 +17,7 @@
> prelude::*,
> sync::aref::{
> ARef,
> - AlwaysRefCounted, //
> + RefCounted, //
> },
> types::Opaque,
> };
> @@ -29,7 +29,7 @@
> #[cfg(CONFIG_RUST_DRM_GEM_SHMEM_HELPER)]
> pub mod shmem;
>
> -/// A macro for implementing [`AlwaysRefCounted`] for any GEM object type.
> +/// A macro for implementing [`RefCounted`] for any GEM object type.
> ///
> /// Since all GEM objects use the same refcounting scheme.
> #[macro_export]
> @@ -42,7 +42,7 @@ impl $( <$( $tparam_id:ident ),+> )? for $type:ty
> )?
> ) => {
> // SAFETY: All GEM objects are refcounted.
> - unsafe impl $( <$( $tparam_id ),+> )? $crate::sync::aref::AlwaysRefCounted for $type
> + unsafe impl $( <$( $tparam_id ),+> )? $crate::sync::aref::RefCounted for $type
> where
> Self: IntoGEMObject,
> $( $( $bind_param : $bind_trait ),+ )?
> @@ -61,6 +61,14 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
> unsafe { bindings::drm_gem_object_put(obj) };
> }
> }
> +
> + // SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<$type>` from a
> + // `&$type`.
> + unsafe impl $( <$( $tparam_id ),+> )? $crate::sync::aref::AlwaysRefCounted for $type
> + where
> + Self: IntoGEMObject,
> + $( $( $bind_param : $bind_trait ),+ )?
> + {}
> };
> }
> #[cfg_attr(not(CONFIG_RUST_DRM_GEM_SHMEM_HELPER), allow(unused))]
> @@ -98,7 +106,7 @@ fn close(_obj: &<Self::Driver as drm::Driver>::Object, _file: &DriverFile<Self>)
> }
>
> /// Trait that represents a GEM object subtype
> -pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted {
> +pub trait IntoGEMObject: Sized + super::private::Sealed + RefCounted {
> /// Returns a reference to the raw `drm_gem_object` structure, which must be valid as long as
> /// this owning object is valid.
> fn as_raw(&self) -> *mut bindings::drm_gem_object;
> diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
> index 23ee689bd240..06e457d62a93 100644
> --- a/rust/kernel/fs/file.rs
> +++ b/rust/kernel/fs/file.rs
> @@ -12,8 +12,8 @@
> cred::Credential,
> error::{code::*, to_result, Error, Result},
> fmt,
> - sync::aref::{ARef, AlwaysRefCounted},
> - types::{NotThreadSafe, Opaque},
> + sync::aref::RefCounted,
> + types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
> };
> use core::ptr;
>
> @@ -197,7 +197,7 @@ unsafe impl Sync for File {}
>
> // SAFETY: The type invariants guarantee that `File` is always ref-counted. This implementation
> // makes `ARef<File>` own a normal refcount.
> -unsafe impl AlwaysRefCounted for File {
> +unsafe impl RefCounted for File {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> @@ -212,6 +212,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<File>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<File>` from a
> +// `&File`.
> +unsafe impl AlwaysRefCounted for File {}
> +
> /// Wraps the kernel's `struct file`. Not thread safe.
> ///
> /// This type represents a file that is not known to be safe to transfer across thread boundaries.
> @@ -233,7 +237,7 @@ pub struct LocalFile {
>
> // SAFETY: The type invariants guarantee that `LocalFile` is always ref-counted. This implementation
> // makes `ARef<LocalFile>` own a normal refcount.
> -unsafe impl AlwaysRefCounted for LocalFile {
> +unsafe impl RefCounted for LocalFile {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> @@ -249,6 +253,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<LocalFile>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<LocalFile>` from a
> +// `&LocalFile`.
> +unsafe impl AlwaysRefCounted for LocalFile {}
> +
> impl LocalFile {
> /// Constructs a new `struct file` wrapper from a file descriptor.
> ///
> diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
> index 7b908f0c5a58..56791c1d63d7 100644
> --- a/rust/kernel/i2c.rs
> +++ b/rust/kernel/i2c.rs
> @@ -18,7 +18,8 @@
> prelude::*,
> sync::aref::{
> ARef,
> - AlwaysRefCounted, //
> + AlwaysRefCounted,
> + RefCounted, //
> },
> types::Opaque, //
> };
> @@ -415,7 +416,7 @@ pub fn get(index: i32) -> Result<ARef<Self>> {
> kernel::impl_device_context_into_aref!(I2cAdapter);
>
> // SAFETY: Instances of `I2cAdapter` are always reference-counted.
> -unsafe impl AlwaysRefCounted for I2cAdapter {
> +unsafe impl RefCounted for I2cAdapter {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::i2c_get_adapter(self.index()) };
> @@ -426,6 +427,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> unsafe { bindings::i2c_put_adapter(obj.as_ref().as_raw()) }
> }
> }
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from an
> +// `&I2cAdapter`.
> +unsafe impl AlwaysRefCounted for I2cAdapter {}
>
> /// The i2c board info representation
> ///
> @@ -491,7 +495,7 @@ unsafe impl<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for I2cClient<C
> kernel::impl_device_context_into_aref!(I2cClient);
>
> // SAFETY: Instances of `I2cClient` are always reference-counted.
> -unsafe impl AlwaysRefCounted for I2cClient {
> +unsafe impl RefCounted for I2cClient {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::get_device(self.as_ref().as_raw()) };
> @@ -502,6 +506,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> unsafe { bindings::put_device(&raw mut (*obj.as_ref().as_raw()).dev) }
> }
> }
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from an
> +// `&I2cClient`.
> +unsafe impl AlwaysRefCounted for I2cClient {}
>
> impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for I2cClient<Ctx> {
> fn as_ref(&self) -> &device::Device<Ctx> {
> diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs
> index 4764d7b68f2a..dd9e3969e720 100644
> --- a/rust/kernel/mm.rs
> +++ b/rust/kernel/mm.rs
> @@ -13,8 +13,8 @@
>
> use crate::{
> bindings,
> - sync::aref::{ARef, AlwaysRefCounted},
> - types::{NotThreadSafe, Opaque},
> + sync::aref::RefCounted,
> + types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
> };
> use core::{ops::Deref, ptr::NonNull};
>
> @@ -55,7 +55,7 @@ unsafe impl Send for Mm {}
> unsafe impl Sync for Mm {}
>
> // SAFETY: By the type invariants, this type is always refcounted.
> -unsafe impl AlwaysRefCounted for Mm {
> +unsafe impl RefCounted for Mm {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The pointer is valid since self is a reference.
> @@ -69,6 +69,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Mm>` from a `&Mm`.
> +unsafe impl AlwaysRefCounted for Mm {}
> +
> /// A wrapper for the kernel's `struct mm_struct`.
> ///
> /// This type is like [`Mm`], but with non-zero `mm_users`. It can only be used when `mm_users` can
> @@ -91,7 +94,7 @@ unsafe impl Send for MmWithUser {}
> unsafe impl Sync for MmWithUser {}
>
> // SAFETY: By the type invariants, this type is always refcounted.
> -unsafe impl AlwaysRefCounted for MmWithUser {
> +unsafe impl RefCounted for MmWithUser {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The pointer is valid since self is a reference.
> @@ -105,6 +108,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<MmWithUser>` from a
> +// `&MmWithUser`.
> +unsafe impl AlwaysRefCounted for MmWithUser {}
> +
> // Make all `Mm` methods available on `MmWithUser`.
> impl Deref for MmWithUser {
> type Target = Mm;
> diff --git a/rust/kernel/mm/mmput_async.rs b/rust/kernel/mm/mmput_async.rs
> index b8d2f051225c..aba4ce675c86 100644
> --- a/rust/kernel/mm/mmput_async.rs
> +++ b/rust/kernel/mm/mmput_async.rs
> @@ -10,7 +10,8 @@
> use crate::{
> bindings,
> mm::MmWithUser,
> - sync::aref::{ARef, AlwaysRefCounted},
> + sync::aref::RefCounted,
> + types::{ARef, AlwaysRefCounted},
> };
> use core::{ops::Deref, ptr::NonNull};
>
> @@ -34,7 +35,7 @@ unsafe impl Send for MmWithUserAsync {}
> unsafe impl Sync for MmWithUserAsync {}
>
> // SAFETY: By the type invariants, this type is always refcounted.
> -unsafe impl AlwaysRefCounted for MmWithUserAsync {
> +unsafe impl RefCounted for MmWithUserAsync {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The pointer is valid since self is a reference.
> @@ -48,6 +49,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<MmWithUserAsync>`
> +// from a `&MmWithUserAsync`.
> +unsafe impl AlwaysRefCounted for MmWithUserAsync {}
> +
> // Make all `MmWithUser` methods available on `MmWithUserAsync`.
> impl Deref for MmWithUserAsync {
> type Target = MmWithUser;
> diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
> index a760fac28765..06fe2ca776a4 100644
> --- a/rust/kernel/opp.rs
> +++ b/rust/kernel/opp.rs
> @@ -16,8 +16,8 @@
> ffi::{c_char, c_ulong},
> prelude::*,
> str::CString,
> - sync::aref::{ARef, AlwaysRefCounted},
> - types::Opaque,
> + sync::aref::RefCounted,
> + types::{ARef, AlwaysRefCounted, Opaque},
> };
>
> #[cfg(CONFIG_CPU_FREQ)]
> @@ -1041,7 +1041,7 @@ unsafe impl Send for OPP {}
> unsafe impl Sync for OPP {}
>
> /// SAFETY: The type invariants guarantee that [`OPP`] is always refcounted.
> -unsafe impl AlwaysRefCounted for OPP {
> +unsafe impl RefCounted for OPP {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> unsafe { bindings::dev_pm_opp_get(self.0.get()) };
> @@ -1053,6 +1053,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<OPP>` from an
> +// `&OPP`.
> +unsafe impl AlwaysRefCounted for OPP {}
> +
> impl OPP {
> /// Creates an owned reference to a [`OPP`] from a valid pointer.
> ///
> diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
> index 5eacdf327d12..bedd4fef84fa 100644
> --- a/rust/kernel/owned.rs
> +++ b/rust/kernel/owned.rs
> @@ -27,7 +27,7 @@
> ///
> /// Note: The underlying object is not required to provide internal reference counting, because it
> /// represents a unique, owned reference. If reference counting (on the Rust side) is required,
> -/// [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented.
> +/// [`RefCounted`](crate::types::RefCounted) should be implemented.
> ///
> /// # Examples
> ///
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index af74ddff6114..acf7384fea02 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -19,6 +19,10 @@
> },
> prelude::*,
> str::CStr,
> + sync::aref::{
> + AlwaysRefCounted,
> + RefCounted, //
> + },
> types::Opaque,
> ThisModule, //
> };
> @@ -474,7 +478,7 @@ unsafe impl<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for Device<Ctx>
> impl crate::dma::Device for Device<device::Core> {}
>
> // SAFETY: Instances of `Device` are always reference-counted.
> -unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
> +unsafe impl RefCounted for Device {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::pci_dev_get(self.as_raw()) };
> @@ -486,6 +490,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
> +// `&Device`.
> +unsafe impl AlwaysRefCounted for Device {}
> +
> impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
> fn as_ref(&self) -> &device::Device<Ctx> {
> // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
> diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
> index 979a9718f153..4f6a94540e33 100644
> --- a/rust/kernel/pid_namespace.rs
> +++ b/rust/kernel/pid_namespace.rs
> @@ -7,7 +7,11 @@
> //! C header: [`include/linux/pid_namespace.h`](srctree/include/linux/pid_namespace.h) and
> //! [`include/linux/pid.h`](srctree/include/linux/pid.h)
>
> -use crate::{bindings, sync::aref::AlwaysRefCounted, types::Opaque};
> +use crate::{
> + bindings,
> + sync::aref::RefCounted,
> + types::{AlwaysRefCounted, Opaque},
> +};
> use core::ptr;
>
> /// Wraps the kernel's `struct pid_namespace`. Thread safe.
> @@ -41,7 +45,7 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::pid_namespace) -> &'a Self {
> }
>
> // SAFETY: Instances of `PidNamespace` are always reference-counted.
> -unsafe impl AlwaysRefCounted for PidNamespace {
> +unsafe impl RefCounted for PidNamespace {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> @@ -55,6 +59,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<PidNamespace>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<PidNamespace>` from
> +// a `&PidNamespace`.
> +unsafe impl AlwaysRefCounted for PidNamespace {}
> +
> // SAFETY:
> // - `PidNamespace::dec_ref` can be called from any thread.
> // - It is okay to send ownership of `PidNamespace` across thread boundaries.
> diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
> index 8917d4ee499f..3c35aa94e319 100644
> --- a/rust/kernel/platform.rs
> +++ b/rust/kernel/platform.rs
> @@ -27,6 +27,7 @@
> },
> of,
> prelude::*,
> + sync::aref::{AlwaysRefCounted, RefCounted},
> types::Opaque,
> ThisModule, //
> };
> @@ -512,7 +513,7 @@ pub fn optional_irq_by_name(&self, name: &CStr) -> Result<IrqRequest<'_>> {
> impl crate::dma::Device for Device<device::Core> {}
>
> // SAFETY: Instances of `Device` are always reference-counted.
> -unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
> +unsafe impl RefCounted for Device {
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
> unsafe { bindings::get_device(self.as_ref().as_raw()) };
> @@ -524,6 +525,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
> +// `&Device`.
> +unsafe impl AlwaysRefCounted for Device {}
> +
> impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
> fn as_ref(&self) -> &device::Device<Ctx> {
> // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
> diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
> index 4ee5fac0e0b6..2d656f672b97 100644
> --- a/rust/kernel/sync/aref.rs
> +++ b/rust/kernel/sync/aref.rs
> @@ -19,11 +19,9 @@
>
> use core::{marker::PhantomData, mem::ManuallyDrop, ops::Deref, ptr::NonNull};
>
> -/// Types that are _always_ reference counted.
> +/// Types that are internally reference counted.
> ///
> /// It allows such types to define their own custom ref increment and decrement functions.
> -/// Additionally, it allows users to convert from a shared reference `&T` to an owned reference
> -/// [`ARef<T>`].
> ///
> /// This is usually implemented by wrappers to existing structures on the C side of the code. For
> /// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted
> @@ -40,9 +38,8 @@
> /// at least until matching decrements are performed.
> ///
> /// Implementers must also ensure that all instances are reference-counted. (Otherwise they
> -/// won't be able to honour the requirement that [`AlwaysRefCounted::inc_ref`] keep the object
> -/// alive.)
> -pub unsafe trait AlwaysRefCounted {
> +/// won't be able to honour the requirement that [`RefCounted::inc_ref`] keep the object alive.)
> +pub unsafe trait RefCounted {
> /// Increments the reference count on the object.
> fn inc_ref(&self);
>
> @@ -55,11 +52,27 @@ pub unsafe trait AlwaysRefCounted {
> /// Callers must ensure that there was a previous matching increment to the reference count,
> /// and that the object is no longer used after its reference count is decremented (as it may
> /// result in the object being freed), unless the caller owns another increment on the refcount
> - /// (e.g., it calls [`AlwaysRefCounted::inc_ref`] twice, then calls
> - /// [`AlwaysRefCounted::dec_ref`] once).
> + /// (e.g., it calls [`RefCounted::inc_ref`] twice, then calls [`RefCounted::dec_ref`] once).
> unsafe fn dec_ref(obj: NonNull<Self>);
> }
>
> +/// Always reference-counted type.
> +///
> +/// It allows deriving a counted reference [`ARef<T>`] from a `&T`.
> +///
> +/// This provides some convenience, but it allows "escaping" borrow checks on `&T`. As it
> +/// complicates attempts to ensure that a reference to T is unique, it is optional to provide for
> +/// [`RefCounted`] types. See *Safety* below.
> +///
> +/// # Safety
> +///
> +/// Implementers must ensure that no safety invariants are violated by upgrading an `&T` to an
> +/// [`ARef<T>`]. In particular that implies [`AlwaysRefCounted`] and [`crate::types::Ownable`]
> +/// cannot be implemented for the same type, as this would allow violating the uniqueness guarantee
> +/// of [`crate::types::Owned<T>`] by dereferencing it into an `&T` and obtaining an [`ARef`] from
> +/// that.
> +pub unsafe trait AlwaysRefCounted: RefCounted {}
> +
> /// An owned reference to an always-reference-counted object.
> ///
> /// The object's reference count is automatically decremented when an instance of [`ARef`] is
> @@ -70,7 +83,7 @@ pub unsafe trait AlwaysRefCounted {
> ///
> /// The pointer stored in `ptr` is non-null and valid for the lifetime of the [`ARef`] instance. In
> /// particular, the [`ARef`] instance owns an increment on the underlying object's reference count.
> -pub struct ARef<T: AlwaysRefCounted> {
> +pub struct ARef<T: RefCounted> {
> ptr: NonNull<T>,
> _p: PhantomData<T>,
> }
> @@ -79,19 +92,19 @@ pub struct ARef<T: AlwaysRefCounted> {
> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
> // `T` to be `Send` because any thread that has an `ARef<T>` may ultimately access `T` using a
> // mutable reference, for example, when the reference count reaches zero and `T` is dropped.
> -unsafe impl<T: AlwaysRefCounted + Sync + Send> Send for ARef<T> {}
> +unsafe impl<T: RefCounted + Sync + Send> Send for ARef<T> {}
>
> // SAFETY: It is safe to send `&ARef<T>` to another thread when the underlying `T` is `Sync`
> // because it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally,
> // it needs `T` to be `Send` because any thread that has a `&ARef<T>` may clone it and get an
> // `ARef<T>` on that thread, so the thread may ultimately access `T` using a mutable reference, for
> // example, when the reference count reaches zero and `T` is dropped.
> -unsafe impl<T: AlwaysRefCounted + Sync + Send> Sync for ARef<T> {}
> +unsafe impl<T: RefCounted + Sync + Send> Sync for ARef<T> {}
>
> // Even if `T` is pinned, pointers to `T` can still move.
> -impl<T: AlwaysRefCounted> Unpin for ARef<T> {}
> +impl<T: RefCounted> Unpin for ARef<T> {}
>
> -impl<T: AlwaysRefCounted> ARef<T> {
> +impl<T: RefCounted> ARef<T> {
> /// Creates a new instance of [`ARef`].
> ///
> /// It takes over an increment of the reference count on the underlying object.
> @@ -120,12 +133,12 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
> ///
> /// ```
> /// use core::ptr::NonNull;
> - /// use kernel::sync::aref::{ARef, AlwaysRefCounted};
> + /// use kernel::sync::aref::{ARef, RefCounted};
> ///
> /// struct Empty {}
> ///
> /// # // SAFETY: TODO.
> - /// unsafe impl AlwaysRefCounted for Empty {
> + /// unsafe impl RefCounted for Empty {
> /// fn inc_ref(&self) {}
> /// unsafe fn dec_ref(_obj: NonNull<Self>) {}
> /// }
> @@ -143,7 +156,7 @@ pub fn into_raw(me: Self) -> NonNull<T> {
> }
> }
>
> -impl<T: AlwaysRefCounted> Clone for ARef<T> {
> +impl<T: RefCounted> Clone for ARef<T> {
> fn clone(&self) -> Self {
> self.inc_ref();
> // SAFETY: We just incremented the refcount above.
> @@ -151,7 +164,7 @@ fn clone(&self) -> Self {
> }
> }
>
> -impl<T: AlwaysRefCounted> Deref for ARef<T> {
> +impl<T: RefCounted> Deref for ARef<T> {
> type Target = T;
>
> fn deref(&self) -> &Self::Target {
> @@ -168,7 +181,7 @@ fn from(b: &T) -> Self {
> }
> }
>
> -impl<T: AlwaysRefCounted> Drop for ARef<T> {
> +impl<T: RefCounted> Drop for ARef<T> {
> fn drop(&mut self) {
> // SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to
> // decrement.
> diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
> index 38273f4eedb5..6259430b0ca3 100644
> --- a/rust/kernel/task.rs
> +++ b/rust/kernel/task.rs
> @@ -10,7 +10,12 @@
> pid_namespace::PidNamespace,
> prelude::*,
> sync::aref::ARef,
> - types::{NotThreadSafe, Opaque},
> + types::{
> + AlwaysRefCounted,
> + NotThreadSafe,
> + Opaque,
> + RefCounted, //
> + },
> };
> use core::{
> ops::Deref,
> @@ -347,7 +352,7 @@ pub fn group_leader(&self) -> &Task {
> }
>
> // SAFETY: The type invariants guarantee that `Task` is always refcounted.
> -unsafe impl crate::sync::aref::AlwaysRefCounted for Task {
> +unsafe impl RefCounted for Task {
> #[inline]
> fn inc_ref(&self) {
> // SAFETY: The existence of a shared reference means that the refcount is nonzero.
> @@ -361,6 +366,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Task>` from a
> +// `&Task`.
> +unsafe impl AlwaysRefCounted for Task {}
> +
> impl PartialEq for Task {
> #[inline]
> fn eq(&self, other: &Self) -> bool {
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 4aec7b699269..9b96aa2ebdb7 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -18,7 +18,8 @@
> },
> sync::aref::{
> ARef,
> - AlwaysRefCounted, //
> + AlwaysRefCounted,
> + RefCounted, //
> }, //
> };
>
> diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
> index 9c17a672cd27..90b13e65cc82 100644
> --- a/rust/kernel/usb.rs
> +++ b/rust/kernel/usb.rs
> @@ -18,7 +18,10 @@
> to_result, //
> },
> prelude::*,
> - sync::aref::AlwaysRefCounted,
> + sync::aref::{
> + AlwaysRefCounted,
> + RefCounted, //
> + },
> types::Opaque,
> ThisModule, //
> };
> @@ -381,7 +384,7 @@ fn as_ref(&self) -> &Device {
> }
>
> // SAFETY: Instances of `Interface` are always reference-counted.
> -unsafe impl AlwaysRefCounted for Interface {
> +unsafe impl RefCounted for Interface {
> fn inc_ref(&self) {
> // SAFETY: The invariants of `Interface` guarantee that `self.as_raw()`
> // returns a valid `struct usb_interface` pointer, for which we will
> @@ -395,6 +398,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Interface>` from a
> +// `&Interface`.
> +unsafe impl AlwaysRefCounted for Interface {}
> +
> // SAFETY: A `Interface` is always reference-counted and can be released from any thread.
> unsafe impl Send for Interface {}
>
> @@ -432,7 +439,7 @@ fn as_raw(&self) -> *mut bindings::usb_device {
> kernel::impl_device_context_into_aref!(Device);
>
> // SAFETY: Instances of `Device` are always reference-counted.
> -unsafe impl AlwaysRefCounted for Device {
> +unsafe impl RefCounted for Device {
> fn inc_ref(&self) {
> // SAFETY: The invariants of `Device` guarantee that `self.as_raw()`
> // returns a valid `struct usb_device` pointer, for which we will
> @@ -446,6 +453,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
> }
> }
>
> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
> +// `&Device`.
> +unsafe impl AlwaysRefCounted for Device {}
> +
> impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
> fn as_ref(&self) -> &device::Device<Ctx> {
> // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
>
> --
> 2.51.2
>
>
^ permalink raw reply
* Re: [PATCH v17 08/10] rust: aref: update formatting of use statements
From: Onur Özkan @ 2026-06-23 17:55 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Daniel Almeida, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
Krzysztof Wilczyński, Boqun Feng, Uladzislau Rezki,
Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett, Igor Korotin,
Pavel Tikhomirov, linux-kernel, rust-for-linux, linux-block,
linux-security-module, dri-devel, linux-fsdevel, linux-mm,
linux-pm, linux-pci, driver-core
In-Reply-To: <20260604-unique-ref-v17-8-7b4c3d2930b9@kernel.org>
On Thu, 04 Jun 2026 22:11:20 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:
> Update formatting if use statements in preparation for next commit.
I guess you meant "formatting use statements"? Also, why not doing this in
the next commit directly?
Onur
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/sync/aref.rs | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
> index 7491382bcf29..818c84fa923a 100644
> --- a/rust/kernel/sync/aref.rs
> +++ b/rust/kernel/sync/aref.rs
> @@ -17,7 +17,12 @@
> //! [`Arc`]: crate::sync::Arc
> //! [`Arc<T>`]: crate::sync::Arc
>
> -use core::{marker::PhantomData, mem::ManuallyDrop, ops::Deref, ptr::NonNull};
> +use core::{
> + marker::PhantomData,
> + mem::ManuallyDrop,
> + ops::Deref,
> + ptr::NonNull, //
> +};
>
> /// Types that are internally reference counted.
> ///
>
> --
> 2.51.2
>
>
^ permalink raw reply
* Re: [PATCH v17 10/10] rust: page: add `from_raw()`
From: Onur Özkan @ 2026-06-23 17:52 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Daniel Almeida, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
Krzysztof Wilczyński, Boqun Feng, Uladzislau Rezki,
Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett, Igor Korotin,
Pavel Tikhomirov, linux-kernel, rust-for-linux, linux-block,
linux-security-module, dri-devel, linux-fsdevel, linux-mm,
linux-pm, linux-pci, driver-core, Onur Özkan
In-Reply-To: <20260604-unique-ref-v17-10-7b4c3d2930b9@kernel.org>
On Thu, 04 Jun 2026 22:11:22 +0200
Andreas Hindborg <a.hindborg@kernel.org> wrote:
> From: Andreas Hindborg <a.hindborg@samsung.com>
>
> Add a method to `Page` that allows construction of an instance from `struct
> page` pointer.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@samsung.com>
> ---
> rust/kernel/page.rs | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
> index 844c75e54134..d56ae597f692 100644
> --- a/rust/kernel/page.rs
> +++ b/rust/kernel/page.rs
> @@ -214,6 +214,18 @@ pub fn nid(&self) -> i32 {
> unsafe { bindings::page_to_nid(self.as_ptr()) }
> }
>
> + /// Create a `&Page` from a raw `struct page` pointer.
> + ///
> + /// # Safety
> + ///
> + /// `ptr` must be convertible to a shared reference with a lifetime of `'a`.
> + #[inline]
> + pub unsafe fn from_raw<'a>(ptr: *const bindings::page) -> &'a Self {
> + // SAFETY: By function safety requirements, `ptr` is not null and is convertible to a shared
> + // reference.
> + unsafe { &*ptr.cast() }
> + }
> +
> /// Runs a piece of code with this page mapped to an address.
> ///
> /// The page is unmapped when this call returns.
>
> --
> 2.51.2
>
>
Reviewed-by: Onur Özkan <work@onurozkan.dev>
^ permalink raw reply
* Re: [PATCH v17 02/10] rust: types: Add Ownable/Owned types
From: Andreas Hindborg @ 2026-06-23 13:09 UTC (permalink / raw)
To: Alice Ryhl
Cc: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Daniel Almeida, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
Krzysztof Wilczyński, Boqun Feng, Uladzislau Rezki,
Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett, Igor Korotin,
Pavel Tikhomirov, linux-kernel, rust-for-linux, linux-block,
linux-security-module, dri-devel, linux-fsdevel, linux-mm,
linux-pm, linux-pci, driver-core, Asahi Lina, Oliver Mangold
In-Reply-To: <ajE5c-5gZtJRoadx@google.com>
Alice Ryhl <aliceryhl@google.com> writes:
> On Thu, Jun 04, 2026 at 10:11:14PM +0200, Andreas Hindborg wrote:
>> From: Asahi Lina <lina+kernel@asahilina.net>
>>
>> By analogy to `AlwaysRefCounted` and `ARef`, an `Ownable` type is a
>> (typically C FFI) type that *may* be owned by Rust, but need not be. Unlike
>> `AlwaysRefCounted`, this mechanism expects the reference to be unique
>> within Rust, and does not allow cloning.
>>
>> Conceptually, this is similar to a `KBox<T>`, except that it delegates
>> resource management to the `T` instead of using a generic allocator.
>>
>> [ om:
>> - Split code into separate file and `pub use` it from types.rs.
>> - Make from_raw() and into_raw() public.
>> - Remove OwnableMut, and make DerefMut dependent on Unpin instead.
>> - Usage example/doctest for Ownable/Owned.
>> - Fixes to documentation and commit message.
>> ]
>>
>> Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
>> Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
>> Co-developed-by: Oliver Mangold <oliver.mangold@pm.me>
>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>> [ Andreas: Updated documentation, examples, and formatting. Change safety
>> requirements, safety comments. Use a reference for `release`. ]
>> Reviewed-by: Gary Guo <gary@garyguo.net>
>> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> Overall looks good to me, but two nits below. With them fixed:
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
>> +pub trait Ownable {
>> + /// Tear down this `Ownable`.
>> + ///
>> + /// Implementers of `Ownable` can use this function to clean up the use of `Self`. This can
>> + /// include freeing the underlying object.
>> + ///
>> + /// # Safety
>> + ///
>> + /// Callers must ensure that the caller has exclusive ownership of `T`, and this ownership can
>> + /// be transferred to the `release` method.
>> + unsafe fn release(&mut self);
>
> I'd make this take a raw pointer because the pointer can be freed during
> the execution of release(), which references don't allow.
Ok.
>
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index 4329d3c2c2e5..4aec7b699269 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -11,6 +11,17 @@
>> };
>> use pin_init::{PinInit, Wrapper, Zeroable};
>>
>> +pub use crate::{
>> + owned::{
>> + Ownable,
>> + Owned, //
>> + },
>> + sync::aref::{
>> + ARef,
>> + AlwaysRefCounted, //
>> + }, //
>> +};
>
> We removed the types::ARef re-export, so you shouldn't add it back.
Looks like a rebase failure, I will remove it.
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH v17 05/10] rust: page: convert to `Ownable`
From: Andreas Hindborg @ 2026-06-23 13:11 UTC (permalink / raw)
To: Alice Ryhl
Cc: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Daniel Almeida, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
Krzysztof Wilczyński, Boqun Feng, Uladzislau Rezki,
Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett, Igor Korotin,
Pavel Tikhomirov, linux-kernel, rust-for-linux, linux-block,
linux-security-module, dri-devel, linux-fsdevel, linux-mm,
linux-pm, linux-pci, driver-core, Asahi Lina
In-Reply-To: <CAH5fLgggdf0CM0o4Oa1VbY+y8gQxrU0VU5z_yB4GWCxnbqsFuQ@mail.gmail.com>
Alice Ryhl <aliceryhl@google.com> writes:
> On Thu, Jun 4, 2026 at 10:14 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.
>>
>> Signed-off-by: Asahi Lina <lina@asahilina.net>
>> [ Andreas: Fix formatting and add a safety comment. ]
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> This will not compile unless Rust Binder is also updated.
I'll be sure to build test against binder.
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH v17 05/10] rust: page: convert to `Ownable`
From: Andreas Hindborg @ 2026-06-23 13:12 UTC (permalink / raw)
To: Alice Ryhl
Cc: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Daniel Almeida, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
Krzysztof Wilczyński, Boqun Feng, Uladzislau Rezki,
Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett, Igor Korotin,
Pavel Tikhomirov, linux-kernel, rust-for-linux, linux-block,
linux-security-module, dri-devel, linux-fsdevel, linux-mm,
linux-pm, linux-pci, driver-core, Asahi Lina
In-Reply-To: <ajKGv5ioLSassmND@google.com>
Alice Ryhl <aliceryhl@google.com> writes:
> On Thu, Jun 04, 2026 at 10:11:17PM +0200, Andreas Hindborg wrote:
>> From: Asahi Lina <lina@asahilina.net>
>>
>> This allows Page references to be returned as borrowed references,
>> without necessarily owning the struct page.
>>
>> Signed-off-by: Asahi Lina <lina@asahilina.net>
>> [ Andreas: Fix formatting and add a safety comment. ]
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> rust/kernel/page.rs | 38 +++++++++++++++++++++++++-------------
>> 1 file changed, 25 insertions(+), 13 deletions(-)
>>
>> diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
>> index 3bdcee0e16a8..844c75e54134 100644
>> --- a/rust/kernel/page.rs
>> +++ b/rust/kernel/page.rs
>> @@ -10,6 +10,11 @@
>> bindings,
>> error::code::*,
>> error::Result,
>> + types::{
>> + Opaque,
>> + Ownable,
>> + Owned, //
>> + },
>> uaccess::UserSliceReader, //
>> };
>> use core::{
>> @@ -105,7 +110,7 @@ pub const fn page_align(addr: usize) -> Option<usize> {
>> ///
>> /// [`VBox`]: kernel::alloc::VBox
>> /// [`Vmalloc`]: kernel::alloc::allocator::Vmalloc
>> -pub struct BorrowedPage<'a>(ManuallyDrop<Page>, PhantomData<&'a Page>);
>> +pub struct BorrowedPage<'a>(ManuallyDrop<NonNull<Page>>, PhantomData<&'a Page>);
>
> BorrowedPage<'a> is no longer needed because it's just &Page.
I'll add some cleaning then.
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH v17 03/10] rust: implement `ForeignOwnable` for `Owned`
From: Andreas Hindborg @ 2026-06-23 13:10 UTC (permalink / raw)
To: Gary Guo, Miguel Ojeda, Gary Guo, Björn Roy Baron,
Benno Lossin, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Dave Ertman, Ira Weiny, Leon Romanovsky,
Paul Moore, Serge Hallyn, Rafael J. Wysocki, David Airlie,
Simona Vetter, Alexander Viro, Christian Brauner, Jan Kara,
Daniel Almeida, Viresh Kumar, Nishanth Menon, Stephen Boyd,
Bjorn Helgaas, Krzysztof Wilczyński, Boqun Feng,
Uladzislau Rezki, Lorenzo Stoakes, Vlastimil Babka,
Liam R. Howlett, Igor Korotin, Pavel Tikhomirov
Cc: linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
driver-core
In-Reply-To: <DJAGDGPKZ4HX.M47NMAU53PCJ@garyguo.net>
"Gary Guo" <gary@garyguo.net> writes:
> On Thu Jun 4, 2026 at 9:11 PM BST, Andreas Hindborg 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>
>> ---
>> rust/kernel/owned.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 46 insertions(+)
>>
>> diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
>> index 456e239e906e..5eacdf327d12 100644
>> --- a/rust/kernel/owned.rs
>> +++ b/rust/kernel/owned.rs
>> @@ -15,6 +15,8 @@
>> ptr::NonNull, //
>> };
>>
>> +use kernel::types::ForeignOwnable;
>> +
>> /// Types that specify their own way of performing allocation and destruction. Typically, this trait
>> /// is implemented on types from the C side.
>> ///
>> @@ -108,6 +110,7 @@ pub trait Ownable {
>> ///
>> /// - Until `T::release` is called, this `Owned<T>` exclusively owns the underlying `T`.
>> /// - The `T` value is pinned.
>> +#[repr(transparent)]
>
> AFAIT this `#[repr(transparent)]` isn't actually needed.
I'll drop it.
>
>> pub struct Owned<T: Ownable> {
>> ptr: NonNull<T>,
>> }
>> @@ -185,3 +188,46 @@ fn drop(&mut self) {
>> unsafe { T::release(self.ptr.as_mut()) };
>> }
>> }
>> +
>> +// SAFETY: We derive the pointer to `T` from a valid `T`, so the returned
>> +// pointer satisfy alignment requirements of `T`.
>> +unsafe impl<T: Ownable + 'static> ForeignOwnable for Owned<T> {
>
> You should drop the `'static` bound and put where bound on the GAT below
> instead. See how `Box` is doing it.
I will take a look.
Best regards,
Andreas Hindborg
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox