* Re: [PATCH v2] x86/bpf: do not audit capability check in do_jit()
From: Paul Moore @ 2025-10-21 16:22 UTC (permalink / raw)
To: Ondrej Mosnacek
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, selinux,
linux-security-module, Serge E . Hallyn
In-Reply-To: <20251021122758.2659513-1-omosnace@redhat.com>
On Tue, Oct 21, 2025 at 8:28 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> The failure of this check only results in a security mitigation being
> applied, slightly affecting performance of the compiled BPF program. It
> doesn't result in a failed syscall, an thus auditing a failed LSM
> permission check for it is unwanted. For example with SELinux, it causes
> a denial to be reported for confined processes running as root, which
> tends to be flagged as a problem to be fixed in the policy. Yet
> dontauditing or allowing CAP_SYS_ADMIN to the domain may not be
> desirable, as it would allow/silence also other checks - either going
> against the principle of least privilege or making debugging potentially
> harder.
>
> Fix it by changing it from capable() to ns_capable_noaudit(), which
> instructs the LSMs to not audit the resulting denials.
>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2369326
> Fixes: d4e89d212d40 ("x86/bpf: Call branch history clearing sequence on exit")
> Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> ---
>
> v1: https://lore.kernel.org/selinux/20250806143105.915748-1-omosnace@redhat.com/
> Changes in v2:
> - just silence the audit records instead of switching to bpf_capable()
>
> arch/x86/net/bpf_jit_comp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] x86/bpf: use bpf_capable() instead of capable(CAP_SYS_ADMIN)
From: Ondrej Mosnacek @ 2025-10-21 12:32 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Serge E. Hallyn, daniel.sneddon, Pawan Gupta, Dave Hansen,
alexandre.chartre, Alexei Starovoitov, Daniel Borkmann, bpf,
selinux, LSM List
In-Reply-To: <CAADnVQL7C6hM6vwztSdq_U4b0u8p-0aVwkaKDAxd53q_+8G1bw@mail.gmail.com>
On Tue, Sep 2, 2025 at 7:37 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Aug 13, 2025 at 2:49 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > On Sat, Aug 9, 2025 at 2:46 AM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Fri, Aug 8, 2025 at 4:59 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> > > >
> > > > On Wed, Aug 06, 2025 at 08:18:55PM -0500, Serge E. Hallyn wrote:
> > > > > On Wed, Aug 06, 2025 at 04:31:05PM +0200, Ondrej Mosnacek wrote:
> > > > > > Don't check against the overloaded CAP_SYS_ADMINin do_jit(), but instead
> > > > > > use bpf_capable(), which checks against the more granular CAP_BPF first.
> > > > > > Going straight to CAP_SYS_ADMIN may cause unnecessary audit log spam
> > > > > > under SELinux, as privileged domains using BPF would usually only be
> > > > > > allowed CAP_BPF and not CAP_SYS_ADMIN.
> > > > > >
> > > > > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2369326
> > > > > > Fixes: d4e89d212d40 ("x86/bpf: Call branch history clearing sequence on exit")
> > > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > > > >
> > > > > So this seems correct, *provided* that we consider it within the purview of
> > > > > CAP_BPF to be able to avoid clearing the branch history buffer.
> > >
> > > true, but...
> > >
> > > > >
> > > > > I suspect that's the case, but it might warrant discussion.
> > > > >
> > > > > Reviewed-by: Serge Hallyn <serge@hallyn.com>
> > > >
> > > > (BTW, I'm assuming this will get pulled into a BPF tree or something, and
> > > > doesn't need to go into the capabilities tree. Let me know if that's wrong)
> > >
> > > Right.
> > > scripts/get_maintainer.pl arch/x86/net/bpf_jit_comp.c
> > > is your friend.
> > >
> > > Pls cc author-s of the commit in question in the future.
> > > Adding them now.
> > >
> > > > > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > > > > > index 15672cb926fc1..2a825e5745ca1 100644
> > > > > > --- a/arch/x86/net/bpf_jit_comp.c
> > > > > > +++ b/arch/x86/net/bpf_jit_comp.c
> > > > > > @@ -2591,8 +2591,7 @@ emit_jmp:
> > > > > > seen_exit = true;
> > > > > > /* Update cleanup_addr */
> > > > > > ctx->cleanup_addr = proglen;
> > > > > > - if (bpf_prog_was_classic(bpf_prog) &&
> > > > > > - !capable(CAP_SYS_ADMIN)) {
> > > > > > + if (bpf_prog_was_classic(bpf_prog) && !bpf_capable()) {
> > >
> > > This looks wrong for several reasons.
> > >
> > > 1.
> > > bpf_capable() and CAP_BPF in general applies to eBPF only.
> > > There is no precedent so far to do anything differently
> > > for cBPF when CAP_BPF is present.
> >
> > That's not entirely true, see below.
> >
> > > 2.
> > > commit log states that
> > > "privileged domains using BPF would usually only be allowed CAP_BPF
> > > and not CAP_SYS_ADMIN"
> > > which is true for eBPF only, since cBPF is always allowed for
> > > all unpriv users.
> > > Start chrome browser and you get cBPF loaded.
> >
> > Processes using cBPF (via SO_ATTACH_FILTER) already can trigger a
> > CAP_BPF check - when the net.core.bpf_jit_harden sysctl is set to 1,
> > then the sequence sk_attach_filter() -> __get_filter() ->
> > bpf_prog_alloc() -> bpf_prog_alloc_no_stats() ->
> > bpf_jit_blinding_enabled() -> bpf_token_capable() happens for the same
> > iio-sensor-proxy syscall as the one that hits the CAP_SYS_ADMIN check.
>
> Agree that it's a similar case and we should standardize on
> the way to check whether mitigation is necessary.
> But I think in both cases it should be "_noaudit" version.
> In other words, everywhere where jit, verifier or anything else
> is checking caps to apply a mitigation or not we should use "_noaudit".
>
> > Because of this we have already granted the BPF capability in
> > Fedora/RHEL SELinux policy to many domains that would usually run as
> > root and that use SO_ATTACH_FILTER. The logic being that they are
> > legitimately using BPF + without SELinux they would be fully
> > privileged (root) and they would pass that check + it seemed they
> > could otherwise lose some performance due to the hardening (though I'm
> > not sure now if it applies to cBPF, so this point could be moot) +
> > CAP_BPF doesn't grant any excess privileges beyond this (as opposed to
> > e.g. CAP_SYS_ADMIN). This is what I meant behind that commit log
> > statement, though I didn't remember the details, so I didn't state it
> > as clearly as I could have (my apologies).
> >
> > Now this same usage started triggering the new plain CAP_SYS_ADMIN
> > check so I naturally assumed that changing it to bpf_capable() would
> > be the most logical solution (as it would let us keep the services
> > excluded from the hardening via CAP_BPF without granting the broad
> > CAP_SYS_ADMIN).
>
> I see. Sounds like you identified bpf_jit_blinding_enabled() case
> and special cased it selinux. So doing bpf_capable() in JIT
> would fit the existing selinux handling.
>
> >
> > Is the fact that CAP_BPF check is reachable via cBPF use unexpected
> > behavior? If both cBPF and eBPF can be JIT'd and CAP_BPF is already
> > being used for the "exempt from JIT hardening" semantics in one place,
> > why should cBPF and eBPF be treated differently? In fact, shouldn't
> > the decision to apply the Spectre mitigation also take into account
> > the net.core.bpf_jit_harden sysctl even when the program is not cBPF?
> >
> > > 3.
> > > glancing over bugzilla it seems that the issue is
> > > excessive audit spam and not related to CAP_BPF and privileges.
> > > If so then the fix is to use
> > > ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN)
> > >
> > > 4.
> > > I don't understand how the patch is supposed to fix the issue.
> > > iio-sensor-proxy is probably unpriv. Why would it use CAP_BPF?
> > > It's using cBPF, so there is no reason for it to have CAP_BPF.
> > > So capable(CAP_BPF) will fail just like capable(CAP_SYS_ADMIN),
> > > but since CAP_BPF check was done first, the audit won't
> > > be printed, because it's some undocumented internal selinux behavior ?
> > > None of it is in the commit log :(
> >
> > It is not unprivileged. It runs as root and without SELinux it would
> > have all capabilities allowed. If it were running without any
> > capabilities, then indeed there would be no SELinux checks.
> >
> > > 5.
> > > And finally all that looks like a selinux bug.
> > > Just because something in the kernel is asking capable(CAP_SYS_ADMIN)
> > > there is no need to spam users with the wrong message:
> > > "SELinux is preventing iio-sensor-prox from using the 'sys_admin' capabilities."
> > > iio-sensor-prox is not trying to use 'sys_admin' capabilities.
> > > cBPF prog will be loaded anyway, with or without BHB clearing.
> >
> > Well, it depends... In this case the AVC denial informs us that the
> > kernel is making some decision depending on the capability and that a
> > decision should be made in the policy to allow or silence the access
> > vector. Even when the consequence is not a failure of the syscall, it
> > still may be useful to have the denial reported, since there is a
> > potential performance impact. OTOH, with CAP_SYS_ADMIN if the decision
> > is to not allow it, then silencing it via a dontaudit rule would
> > potentially hide other more critical CAP_SYS_ADMIN denials, so it's
> > hard to decide what is better - to silence this specific case in the
> > kernel vs. to let the user allow/silence the specific AV in the
> > policy...
>
> imo we should apply a general rule for using "_noaudit"
> for all checks that don't end up as a denial.
> There are various bpf_allow_ptr_leaks(), bpf_bypass_spec_v[14]()
> checks in the verifier that should be converted to "_noaudit".
> It's true that the check affects performance and users could be
> interested in the end result, but if they enable mitigations they
> expect the performance hit across the board, so skipping a mitigation
> for a privileged process is a bonus. A prog will be tiny bit faster.
> So not worth flagging anywhere.
Ok, so I have sent a v2 that switches to ns_capable_noaudit() instead:
https://lore.kernel.org/selinux/20251021122758.2659513-1-omosnace@redhat.com/
--
Ondrej Mosnacek
Senior Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
^ permalink raw reply
* [PATCH v2] x86/bpf: do not audit capability check in do_jit()
From: Ondrej Mosnacek @ 2025-10-21 12:27 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: bpf, selinux, linux-security-module, Serge E . Hallyn
The failure of this check only results in a security mitigation being
applied, slightly affecting performance of the compiled BPF program. It
doesn't result in a failed syscall, an thus auditing a failed LSM
permission check for it is unwanted. For example with SELinux, it causes
a denial to be reported for confined processes running as root, which
tends to be flagged as a problem to be fixed in the policy. Yet
dontauditing or allowing CAP_SYS_ADMIN to the domain may not be
desirable, as it would allow/silence also other checks - either going
against the principle of least privilege or making debugging potentially
harder.
Fix it by changing it from capable() to ns_capable_noaudit(), which
instructs the LSMs to not audit the resulting denials.
Link: https://bugzilla.redhat.com/show_bug.cgi?id=2369326
Fixes: d4e89d212d40 ("x86/bpf: Call branch history clearing sequence on exit")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
v1: https://lore.kernel.org/selinux/20250806143105.915748-1-omosnace@redhat.com/
Changes in v2:
- just silence the audit records instead of switching to bpf_capable()
arch/x86/net/bpf_jit_comp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index d4c93d9e73e40..de5083cb1d374 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -2701,7 +2701,7 @@ emit_jmp:
/* Update cleanup_addr */
ctx->cleanup_addr = proglen;
if (bpf_prog_was_classic(bpf_prog) &&
- !capable(CAP_SYS_ADMIN)) {
+ !ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN)) {
u8 *ip = image + addrs[i - 1];
if (emit_spectre_bhb_barrier(&prog, ip, bpf_prog))
--
2.51.0
^ permalink raw reply related
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: Alexei Starovoitov @ 2025-10-21 1:25 UTC (permalink / raw)
To: James Bottomley
Cc: Paul Moore, Linus Torvalds, Alexei Starovoitov, KP Singh,
Blaise Boscaccy, bpf, LSM List, K. Y. Srinivasan, Daniel Borkmann,
Andrii Nakryiko, wufan, Quentin Monnet
In-Reply-To: <bc823ddbaf63e0e177eb46d1cc15076e4e2e689d.camel@HansenPartnership.com>
On Mon, Oct 20, 2025 at 4:13 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> On Fri, 2025-10-17 at 11:03 -0700, Alexei Starovoitov wrote:
> > On Thu, Oct 16, 2025 at 6:36 PM Paul Moore <paul@paul-moore.com>
> > wrote:
> > >
> > > On Thu, Oct 16, 2025 at 6:01 PM Alexei Starovoitov
> > > <alexei.starovoitov@gmail.com> wrote:
> > > > On Thu, Oct 16, 2025 at 1:51 PM Paul Moore <paul@paul-moore.com>
> > > > wrote:
> > > > > On Sun, Oct 12, 2025 at 10:12 PM Paul Moore
> > > > > <paul@paul-moore.com> wrote:
> > > > > > On Sat, Oct 11, 2025 at 1:09 PM James Bottomley
> > > > > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > > > > On Sat, 2025-10-11 at 09:31 -0700, Alexei Starovoitov
> > > > > > > wrote:
> > > > > > > > On Sat, Oct 11, 2025 at 7:52 AM James Bottomley
> > > > > > > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > > > > > >
> > > > > > > > > It doesn't need to, once we check both the loader and
> > > > > > > > > the map, the integrity is verified and the loader can
> > > > > > > > > be trusted to run and relocate the map into the bpf
> > > > > > > > > program
> > > > > > > >
> > > > > > > > You should read KP's cover letter again and then research
> > > > > > > > trusted hash chains. Here is a quote from the first
> > > > > > > > googled link:
> > > > > > > >
> > > > > > > > "A trusted hash chain is a cryptographic process used to
> > > > > > > > verify the integrity and authenticity of data by creating
> > > > > > > > a sequence of hash values, where each hash is linked to
> > > > > > > > the next".
> > > > > > > >
> > > > > > > > In addition KP's algorithm was vetted by various security
> > > > > > > > teams. There is nothing novel here. It's a classic
> > > > > > > > algorithm used to verify integrity and that's what was
> > > > > > > > implemented.
> > > > > > >
> > > > > > > Both KP and Blaise's patch sets are implementations of
> > > > > > > trusted hash chains. The security argument isn't about
> > > > > > > whether the hash chain algorithm works, it's about where,
> > > > > > > in relation to the LSM hook, the hash chain verification
> > > > > > > completes.
> > > >
> > > > Not true. Blaise's patch is a trusted hash chain denial.
> > >
> > > It would be helpful if you could clarify what you mean by "trusted
> > > hash chain denial" and how that differs from a "trusted hash
> > > chain".
> >
> > Paul,
> > This is getting ridiculous. You're arguing about the code that you
> > don't understand. Stop this broken phone and let Blaise defend his
> > code.
>
> That might be my fault: I told Blaise only to respond to technical
> issues and arguing about what you want to name an algorithm isn't
> really a technical issue with the patch.
>
> The point, for me, is when doing integrity tests both patch sets
> produce identical results and correctly detect when integrity of a
> light skeleton is compromised (in mathematical terms that means they're
> functionally equivalent). The only difference is that with Blaise's
> patch set verification completes before the LSM load hook is called and
> with KP's it completes after ... and the security problem with the
> latter case is that there's no LSM hook to collect the verification
> result.
the security problem with KP's approach? wtf.
I'm going to add "depends on !microsoft" to kconfig bpf_syscall
and be done with it.
Don't use it since it's so insecure.
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/3] BPF signature hash chains
From: James Bottomley @ 2025-10-20 23:13 UTC (permalink / raw)
To: Alexei Starovoitov, Paul Moore
Cc: Linus Torvalds, Alexei Starovoitov, KP Singh, Blaise Boscaccy,
bpf, LSM List, K. Y. Srinivasan, Daniel Borkmann, Andrii Nakryiko,
wufan, Quentin Monnet
In-Reply-To: <CAADnVQLRtfPrH6sffaPVyFP4Aib+e7uVVWLi7bb79d9TrHjHpQ@mail.gmail.com>
On Fri, 2025-10-17 at 11:03 -0700, Alexei Starovoitov wrote:
> On Thu, Oct 16, 2025 at 6:36 PM Paul Moore <paul@paul-moore.com>
> wrote:
> >
> > On Thu, Oct 16, 2025 at 6:01 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > > On Thu, Oct 16, 2025 at 1:51 PM Paul Moore <paul@paul-moore.com>
> > > wrote:
> > > > On Sun, Oct 12, 2025 at 10:12 PM Paul Moore
> > > > <paul@paul-moore.com> wrote:
> > > > > On Sat, Oct 11, 2025 at 1:09 PM James Bottomley
> > > > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > > > On Sat, 2025-10-11 at 09:31 -0700, Alexei Starovoitov
> > > > > > wrote:
> > > > > > > On Sat, Oct 11, 2025 at 7:52 AM James Bottomley
> > > > > > > <James.Bottomley@hansenpartnership.com> wrote:
> > > > > > > >
> > > > > > > > It doesn't need to, once we check both the loader and
> > > > > > > > the map, the integrity is verified and the loader can
> > > > > > > > be trusted to run and relocate the map into the bpf
> > > > > > > > program
> > > > > > >
> > > > > > > You should read KP's cover letter again and then research
> > > > > > > trusted hash chains. Here is a quote from the first
> > > > > > > googled link:
> > > > > > >
> > > > > > > "A trusted hash chain is a cryptographic process used to
> > > > > > > verify the integrity and authenticity of data by creating
> > > > > > > a sequence of hash values, where each hash is linked to
> > > > > > > the next".
> > > > > > >
> > > > > > > In addition KP's algorithm was vetted by various security
> > > > > > > teams. There is nothing novel here. It's a classic
> > > > > > > algorithm used to verify integrity and that's what was
> > > > > > > implemented.
> > > > > >
> > > > > > Both KP and Blaise's patch sets are implementations of
> > > > > > trusted hash chains. The security argument isn't about
> > > > > > whether the hash chain algorithm works, it's about where,
> > > > > > in relation to the LSM hook, the hash chain verification
> > > > > > completes.
> > >
> > > Not true. Blaise's patch is a trusted hash chain denial.
> >
> > It would be helpful if you could clarify what you mean by "trusted
> > hash chain denial" and how that differs from a "trusted hash
> > chain".
>
> Paul,
> This is getting ridiculous. You're arguing about the code that you
> don't understand. Stop this broken phone and let Blaise defend his
> code.
That might be my fault: I told Blaise only to respond to technical
issues and arguing about what you want to name an algorithm isn't
really a technical issue with the patch.
The point, for me, is when doing integrity tests both patch sets
produce identical results and correctly detect when integrity of a
light skeleton is compromised (in mathematical terms that means they're
functionally equivalent). The only difference is that with Blaise's
patch set verification completes before the LSM load hook is called and
with KP's it completes after ... and the security problem with the
latter case is that there's no LSM hook to collect the verification
result.
Regards,
James
^ permalink raw reply
* Re: [PATCH v2 6/6] Implement quiet for optional accesses
From: Mickaël Salaün @ 2025-10-20 20:11 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
In-Reply-To: <326e24ff-8887-46ff-bad9-f30a6daca351@maowtm.org>
On Sun, Oct 19, 2025 at 06:45:55PM +0100, Tingmao Wang wrote:
> On 10/15/25 20:09, Mickaël Salaün wrote:
> > [...]
> > On Sun, Oct 05, 2025 at 06:55:29PM +0100, Tingmao Wang wrote:
> >> [..]
> >> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> >> index a647b68e8d06..0f611ad516be 100644
> >> --- a/security/landlock/domain.c
> >> +++ b/security/landlock/domain.c
> >> @@ -212,6 +212,29 @@ landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
> >> return deny_masks;
> >> }
> >>
> >
> > Just using u8 is confusing. Please document what is the "type" of the
> > returned value, and use a dedicated typedef instead of u8 (see my other
> > comment about static_assert). This typedef should probably be named
> > optional_access_t and have a size less or equal to access_t's one.
> >
> >> +u8 landlock_get_quiet_optional_accesses(
> >> + const access_mask_t all_existing_optional_access,
> >> + const deny_masks_t deny_masks,
> >> + const struct collected_rule_flags rule_flags)
> >> +{
> >> + const unsigned long access_opt = all_existing_optional_access;
> >> + size_t access_index = 0;
> >> + unsigned long access_bit;
> >> + u8 quiet_optional_accesses = 0;
> >
> > As for deny_masks_t, we should define an "optional_access_t" type with
> > appropriate safeguard to guarantee that it can always hold all optional
> > access rights (see static_assert for deny_masks_t in access.h).
> >
> > We should also copy the WARN_ON_ONCE() check from
> > get_layer_from_deny_masks().
>
> I don't see how that WARN_ON_ONCE is applicable here since we're no longer
> dealing with the `optional_access`... Can you clarify?
WARN_ON_ONCE(access_opt != _LANDLOCK_ACCESS_FS_OPTIONAL);
>
> >
> >> [...]
> >> @@ -80,13 +88,24 @@ struct landlock_file_security {
> >> struct landlock_cred_security fown_subject;
> >> };
> >>
> >> -#ifdef CONFIG_AUDIT
> >> +static void build_check_file_security(void)
> >
> > You can move this function to fs.c and call it in
> > hook_file_alloc_security() instead.
> >
> >> +{
> >> + const struct landlock_file_security file_sec = {
> >> + .quiet_optional_accesses = ~0,
> >> + .fown_layer = ~0,
> >> + };
> >> +
> >> + /*
> >> + * Make sure quiet_optional_accesses has enough bits to cover all
> >> + * optional accesses
> >> + */
> >> + BUILD_BUG_ON(__const_hweight8(file_sec.quiet_optional_accesses) <
> >
> > We should be able to use HWEIGHT() instead.
>
> I tried it and unfortunately it doesn't seem to work :(
>
> security/landlock/fs.c: In function ‘build_check_file_security’:
> ./include/linux/compiler.h:201:82: error: expression in static assertion is not constant
> 201 | #define __BUILD_BUG_ON_ZERO_MSG(e, msg, ...) ((int)sizeof(struct {_Static_assert(!(e), msg);}))
> | ^~~~
> ././include/linux/compiler_types.h:577:23: note: in definition of macro ‘__compiletime_assert’
> 577 | if (!(condition)) \
> | ^~~~~~~~~
> ././include/linux/compiler_types.h:597:9: note: in expansion of macro ‘_compiletime_assert’
> 597 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> | ^~~~~~~~~~~~~~~~~~~
> ./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
> 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> | ^~~~~~~~~~~~~~~~~~
> ./include/linux/build_bug.h:50:9: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
> 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
> | ^~~~~~~~~~~~~~~~
> security/landlock/fs.c:1769:9: note: in expansion of macro ‘BUILD_BUG_ON’
> 1769 | BUILD_BUG_ON(HWEIGHT(file_sec.quiet_optional_accesses) <
> | ^~~~~~~~~~~~
> ./include/linux/build_bug.h:17:9: note: in expansion of macro ‘__BUILD_BUG_ON_ZERO_MSG’
> 17 | __BUILD_BUG_ON_ZERO_MSG(e, ##__VA_ARGS__, #e " is true")
> | ^~~~~~~~~~~~~~~~~~~~~~~
> ./include/asm-generic/bitops/const_hweight.h:37:23: note: in expansion of macro ‘BUILD_BUG_ON_ZERO’
> 37 | #define HWEIGHT64(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_hweight64(w))
> | ^~~~~~~~~~~~~~~~~
> ./include/asm-generic/bitops/const_hweight.h:42:22: note: in expansion of macro ‘HWEIGHT64’
> 42 | #define HWEIGHT(w) HWEIGHT64((u64)w)
> | ^~~~~~~~~
> security/landlock/fs.c:1769:22: note: in expansion of macro ‘HWEIGHT’
> 1769 | BUILD_BUG_ON(HWEIGHT(file_sec.quiet_optional_accesses) <
> | ^~~~~~~
>
Indeed, it works with Clang but not GCC.
Consistently always using __const_hweight64() might help avoid future
issue if quiet_optional_accesses grows more than 8 bits though.
> >
> >> + __const_hweight64(_LANDLOCK_ACCESS_FS_OPTIONAL));
> >> + /* Makes sure all layers can be identified. */
> >> + BUILD_BUG_ON(file_sec.fown_layer < LANDLOCK_MAX_NUM_LAYERS - 1);
> >> +}
> >>
> >> -/* Makes sure all layers can be identified. */
> >> -/* clang-format off */
> >> -static_assert((typeof_member(struct landlock_file_security, fown_layer))~0 >=
> >> - LANDLOCK_MAX_NUM_LAYERS);
> >> -/* clang-format off */
> >> +#ifdef CONFIG_AUDIT
> >>
> >> #endif /* CONFIG_AUDIT */
> >>
> >> @@ -107,6 +126,7 @@ struct landlock_superblock_security {
> >> static inline struct landlock_file_security *
> >> landlock_file(const struct file *const file)
> >> {
> >> + build_check_file_security();
> >> return file->f_security + landlock_blob_sizes.lbs_file;
> >> }
> >>
> >> --
> >> 2.51.0
>
^ permalink raw reply
* Re: [PATCH v2 3/6] landlock/audit: Check for quiet flag in landlock_log_denial
From: Mickaël Salaün @ 2025-10-20 20:12 UTC (permalink / raw)
To: Tingmao Wang; +Cc: Günther Noack, Jan Kara, linux-security-module
In-Reply-To: <b44db371-59b5-4e52-bdb2-f6359580a0df@maowtm.org>
On Sun, Oct 19, 2025 at 06:39:55PM +0100, Tingmao Wang wrote:
> On 10/15/25 20:09, Mickaël Salaün wrote:
> > Just use "landlock: " as subject prefix.
>
> What about samples/landlock or selftests/landlock (in the next version)?
> Should they also just be landlock: ?
No, those are good. The idea is that these three prefixes identify
Landlock-related code (and potential different/complementary
maintainers) in different places. It's not useful to identify part of
Landlock inside one file hierarchy. In doubt, just follow the same
pattern as in previous commits:
- samples/landlock: Files in samples/landlock/
- selftests/landlock: Files in tools/testing/selftests/landlock/
- landlock: Files in security/landlock/, Documentation/*/landlock.rst,
and Landlock catchall.
^ permalink raw reply
* Re: [PATCH v2 1/2] landlock: Multithreading support for landlock_restrict_self()
From: Mickaël Salaün @ 2025-10-20 20:12 UTC (permalink / raw)
To: Günther Noack
Cc: Konstantin Meskhidze, Tingmao Wang, Paul Moore,
linux-security-module, Jann Horn
In-Reply-To: <20251001111807.18902-2-gnoack@google.com>
On Wed, Oct 01, 2025 at 01:18:06PM +0200, Günther Noack wrote:
> Introduce the LANDLOCK_RESTRICT_SELF_TSYNC flag. With this flag, a
> given Landlock ruleset is applied to all threads of the calling
> process, instead of only the current one.
>
> Without this flag, multithreaded userspace programs currently resort
> to using the nptl(7)/libpsx hack for multithreaded policy enforcement,
> which is also used by libcap and for setuid(2). Using this scheme,
> the threads of a process enforce the same Landlock ruleset, but the
> resulting Landlock domains are still separate, which makes a
> difference for Landlock's "scoped" access rights, where the domain
> identity and nesting is used. As a result, when using
> LANLDOCK_SCOPE_SIGNAL, signaling between sibling threads stops
> working. This is a problem for programming languages and frameworks
> which are inherently multithreaded (e.g. Go).
>
> Cc: Mickaël Salaün <mic@digikod.net>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: linux-security-module@vger.kernel.org
> Suggested-by: Jann Horn <jannh@google.com>
> Signed-off-by: Günther Noack <gnoack@google.com>
> ---
> include/uapi/linux/landlock.h | 4 +
> security/landlock/cred.h | 12 +
> security/landlock/limits.h | 2 +-
> security/landlock/syscalls.c | 433 +++++++++++++++++++++++++++++++++-
> 4 files changed, 448 insertions(+), 3 deletions(-)
> +/*
> + * restrict_sibling_threads - enables a Landlock policy for all sibling threads
> + */
> +static int restrict_sibling_threads(const struct cred *old_cred,
> + const struct cred *new_cred)
> +{
> + int res;
> + struct task_struct *thread, *caller;
> + struct tsync_shared_context shared_ctx;
> + struct tsync_works works = {};
> + size_t newly_discovered_threads;
> + bool found_more_threads;
> + struct tsync_work *ctx;
> +
> + atomic_set(&shared_ctx.preparation_error, 0);
> + init_completion(&shared_ctx.all_prepared);
> + init_completion(&shared_ctx.ready_to_commit);
> + atomic_set(&shared_ctx.num_unfinished, 0);
> + init_completion(&shared_ctx.all_finished);
> + shared_ctx.old_cred = old_cred;
> + shared_ctx.new_cred = new_cred;
> +
> + caller = current;
> +
> + /*
> + * We schedule a pseudo-signal task_work for each of the calling task's
> + * sibling threads. In the task work, each thread:
> + *
> + * 1) runs prepare_creds() and writes back the error to
> + * shared_ctx.preparation_error, if needed.
> + *
> + * 2) signals that it's done with prepare_creds() to the calling task.
> + * (completion "all_prepared").
> + *
> + * 3) waits for the completion "ready_to_commit". This is sent by the
> + * calling task after ensuring that all sibling threads have done
> + * with the "preparation" stage.
> + *
> + * After this barrier is reached, it's safe to read
> + * shared_ctx.preparation_error.
> + *
> + * 4) reads shared_ctx.preparation_error and then either does
> + * commit_creds() or abort_creds().
> + *
> + * 5) signals that it's done altogether (barrier synchronization
> + * "all_finished")
> + */
> + do {
> + found_more_threads = false;
> +
> + /*
> + * The "all_prepared" barrier is used locally to the inner loop,
> + * this use of for_each_thread(). We can reset it on each loop
> + * iteration because all previous loop iterations are done with
> + * it already.
> + *
> + * num_preparing is initialized to 1 so that the counter can not
> + * go to 0 and mark the completion as done before all task works
> + * are registered. (We decrement it at the end of this loop.)
> + */
> + atomic_set(&shared_ctx.num_preparing, 1);
> + reinit_completion(&shared_ctx.all_prepared);
> +
> + /* In RCU read-lock, count the threads we need. */
> + newly_discovered_threads = 0;
> + rcu_read_lock();
> + for_each_thread(caller, thread) {
> + /* Skip current, since it is initiating the sync. */
> + if (thread == caller)
> + continue;
> +
> + /* Skip exited threads. */
> + if (thread->flags & PF_EXITING)
> + continue;
> +
> + /* Skip threads that we have already seen. */
> + if (tsync_works_contains_task(&works, thread))
> + continue;
> +
> + newly_discovered_threads++;
> + }
> + rcu_read_unlock();
This RCU block could be moved in a dedicated helper that will return the
number of newly discovered threads. In this helper, we could use
guard()(rcu).
> +
> + if (newly_discovered_threads == 0)
> + break; /* done */
> +
> + res = tsync_works_grow_by(&works, newly_discovered_threads,
> + GFP_KERNEL_ACCOUNT);
> + if (res) {
> + atomic_set(&shared_ctx.preparation_error, res);
> + break;
> + }
> +
> + rcu_read_lock();
> + for_each_thread(caller, thread) {
> + /* Skip current, since it is initiating the sync. */
> + if (thread == caller)
> + continue;
> +
> + /* Skip exited threads. */
> + if (thread->flags & PF_EXITING)
> + continue;
> +
> + /* Skip threads that we already looked at. */
> + if (tsync_works_contains_task(&works, thread))
> + continue;
> +
> + /*
> + * We found a sibling thread that is not doing its
> + * task_work yet, and which might spawn new threads
> + * before our task work runs, so we need at least one
> + * more round in the outer loop.
> + */
> + found_more_threads = true;
> +
> + ctx = tsync_works_provide(&works, thread);
> + if (!ctx) {
> + /*
> + * We ran out of preallocated contexts -- we
> + * need to try again with this thread at a later
> + * time! found_more_threads is already true
> + * at this point.
> + */
> + break;
> + }
> +
> + ctx->shared_ctx = &shared_ctx;
> +
> + atomic_inc(&shared_ctx.num_preparing);
> + atomic_inc(&shared_ctx.num_unfinished);
> +
> + init_task_work(&ctx->work,
> + restrict_one_thread_callback);
> + res = task_work_add(thread, &ctx->work, TWA_SIGNAL);
> + if (res) {
> + /*
> + * Remove the task from ctx so that we will
> + * revisit the task at a later stage, if it
> + * still exists.
> + */
> + put_task_struct_rcu_user(ctx->task);
> + ctx->task = NULL;
> +
> + atomic_set(&shared_ctx.preparation_error, res);
> + atomic_dec(&shared_ctx.num_preparing);
> + atomic_dec(&shared_ctx.num_unfinished);
> + }
> + }
> + rcu_read_unlock();
As for the other RCU block, it might help to move this RCU block into a
dedicated helper. It seems that it would look easier to read and
maintain.
> +
> + /*
> + * Decrement num_preparing for current, to undo that we
> + * initialized it to 1 at the beginning of the inner loop.
> + */
> + if (atomic_dec_return(&shared_ctx.num_preparing) > 0)
> + wait_for_completion(&shared_ctx.all_prepared);
> + } while (found_more_threads &&
> + !atomic_read(&shared_ctx.preparation_error));
Is it safe to prevent inconsistencies wrt execve? seccomp uses
cred_guard_mutex (new code should probably use exec_update_lock), why
should Landlock not do the same?
Why shouldn't we lock sighand as well?
Answers to these questions should be explained in comments.
> +
> + /*
> + * We now have all sibling threads blocking and in "prepared" state in
> + * the task work. Ask all threads to commit.
> + */
> + complete_all(&shared_ctx.ready_to_commit);
> +
> + if (works.size)
> + wait_for_completion(&shared_ctx.all_finished);
> +
> + tsync_works_free(&works);
> +
> + return atomic_read(&shared_ctx.preparation_error);
> +}
> +
> /**
> * sys_landlock_restrict_self - Enforce a ruleset on the calling thread
> *
> @@ -454,12 +866,20 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
> * - %LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF
> * - %LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON
> * - %LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF
> + * - %LANDLOCK_RESTRICT_SELF_TSYNC
> *
> - * This system call enables to enforce a Landlock ruleset on the current
> - * thread. Enforcing a ruleset requires that the task has %CAP_SYS_ADMIN in its
> + * This system call enforces a Landlock ruleset on the current thread.
> + * Enforcing a ruleset requires that the task has %CAP_SYS_ADMIN in its
> * namespace or is running with no_new_privs. This avoids scenarios where
> * unprivileged tasks can affect the behavior of privileged children.
> *
> + * If %LANDLOCK_RESTRICT_SELF_TSYNC is specified in @flags, all other threads of
> + * the process will be brought into the exact same Landlock configuration as the
> + * calling thread. This includes both the enforced ruleset and logging
> + * configuration, and happens irrespective of previously established rulesets
> + * and logging configurations on these threads. If required, this operation
> + * also enables the no_new_privs flag for these threads.
> + *
> * Possible returned errors are:
> *
> * - %EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
> @@ -484,6 +904,7 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
> struct landlock_cred_security *new_llcred;
> bool __maybe_unused log_same_exec, log_new_exec, log_subdomains,
> prev_log_subdomains;
> + int res;
>
> if (!is_initialized())
> return -EOPNOTSUPP;
> @@ -566,5 +987,13 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
> new_llcred->domain_exec |= BIT(new_dom->num_layers - 1);
> #endif /* CONFIG_AUDIT */
>
> + if (flags & LANDLOCK_RESTRICT_SELF_TSYNC) {
> + res = restrict_sibling_threads(current_cred(), new_cred);
> + if (res != 0) {
> + abort_creds(new_cred);
> + return res;
> + }
> + }
> +
> return commit_creds(new_cred);
> }
> --
> 2.51.0.618.g983fd99d29-goog
>
>
^ permalink raw reply
* Re: [PATCH v2] nbd: override creds to kernel when calling sock_{send,recv}msg()
From: Jens Axboe @ 2025-10-20 16:38 UTC (permalink / raw)
To: Josef Bacik, Ondrej Mosnacek
Cc: linux-block, nbd, linux-security-module, selinux, Ming Lei
In-Reply-To: <20251010080900.1680512-1-omosnace@redhat.com>
On Fri, 10 Oct 2025 10:09:00 +0200, Ondrej Mosnacek wrote:
> sock_{send,recv}msg() internally calls security_socket_{send,recv}msg(),
> which does security checks (e.g. SELinux) for socket access against the
> current task. However, _sock_xmit() in drivers/block/nbd.c may be called
> indirectly from a userspace syscall, where the NBD socket access would
> be incorrectly checked against the calling userspace task (which simply
> tries to read/write a file that happens to reside on an NBD device).
>
> [...]
Applied, thanks!
[1/1] nbd: override creds to kernel when calling sock_{send,recv}msg()
commit: 81ccca31214e11ea2b537fd35d4f66d7cf46268e
Best regards,
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH kvm-next V11 0/7] Add NUMA mempolicy support for KVM guest-memfd
From: Sean Christopherson @ 2025-10-20 15:52 UTC (permalink / raw)
To: willy, akpm, david, pbonzini, shuah, vbabka, Shivank Garg
Cc: brauner, viro, dsterba, xiang, chao, jaegeuk, clm, josef,
kent.overstreet, zbestahu, jefflexu, dhavale, lihongbo22,
lorenzo.stoakes, Liam.Howlett, rppt, surenb, mhocko, ziy,
matthew.brost, joshua.hahnjy, rakie.kim, byungchul, gourry,
ying.huang, apopple, tabba, ackerleytng, paul, jmorris, serge,
pvorel, bfoster, vannapurve, chao.gao, bharata, nikunj,
michael.day, shdhiman, yan.y.zhao, Neeraj.Upadhyay,
thomas.lendacky, michael.roth, aik, kalyazin, peterx, jack, hch,
cgzones, ira.weiny, rientjes, roypat, chao.p.peng, amit, ddutile,
dan.j.williams, ashish.kalra, gshan, jgowans, pankaj.gupta,
papaluri, yuzhao, suzuki.poulose, quic_eberman, linux-bcachefs,
linux-btrfs, linux-erofs, linux-f2fs-devel, linux-fsdevel,
linux-mm, linux-kernel, linux-security-module, kvm,
linux-kselftest, linux-coco, Jason Gunthorpe
In-Reply-To: <176055105546.1527431.3611256810380818215.b4-ty@google.com>
On Wed, Oct 15, 2025, Sean Christopherson wrote:
> On Wed, 27 Aug 2025 17:52:41 +0000, Shivank Garg wrote:
> > This series introduces NUMA-aware memory placement support for KVM guests
> > with guest_memfd memory backends. It builds upon Fuad Tabba's work (V17)
> > that enabled host-mapping for guest_memfd memory [1] and can be applied
> > directly applied on KVM tree [2] (branch kvm-next, base commit: a6ad5413,
> > Merge branch 'guest-memfd-mmap' into HEAD)
> >
> > == Background ==
> > KVM's guest-memfd memory backend currently lacks support for NUMA policy
> > enforcement, causing guest memory allocations to be distributed across host
> > nodes according to kernel's default behavior, irrespective of any policy
> > specified by the VMM. This limitation arises because conventional userspace
> > NUMA control mechanisms like mbind(2) don't work since the memory isn't
> > directly mapped to userspace when allocations occur.
> > Fuad's work [1] provides the necessary mmap capability, and this series
> > leverages it to enable mbind(2).
> >
> > [...]
>
> Applied the non-KVM change to kvm-x86 gmem. We're still tweaking and iterating
> on the KVM changes, but I fully expect them to land in 6.19.
>
> Holler if you object to taking these through the kvm tree.
>
> [1/7] mm/filemap: Add NUMA mempolicy support to filemap_alloc_folio()
> https://github.com/kvm-x86/linux/commit/601aa29f762f
> [2/7] mm/filemap: Extend __filemap_get_folio() to support NUMA memory policies
> https://github.com/kvm-x86/linux/commit/2bb25703e5bd
> [3/7] mm/mempolicy: Export memory policy symbols
> https://github.com/kvm-x86/linux/commit/e1b4cf7d6be3
FYI, I rebased these onto 6.18-rc2 to avoid a silly merge. New hashes:
[1/3] mm/filemap: Add NUMA mempolicy support to filemap_alloc_folio()
https://github.com/kvm-x86/linux/commit/7f3779a3ac3e
[2/3] mm/filemap: Extend __filemap_get_folio() to support NUMA memory policies
https://github.com/kvm-x86/linux/commit/16a542e22339
[3/3] mm/mempolicy: Export memory policy symbols
https://github.com/kvm-x86/linux/commit/f634f10809ec
^ permalink raw reply
* Re: [PATCH] ima: Fall back to default kernel module signature verification
From: Mimi Zohar @ 2025-10-20 13:57 UTC (permalink / raw)
To: Roberto Sassu, Coiby Xu
Cc: linux-integrity, Dmitry Torokhov, Karel Srot, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris,
Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <1987088bb29971883d2b5c06a31c8114c729422c.camel@huaweicloud.com>
On Mon, 2025-10-20 at 14:45 +0200, Roberto Sassu wrote:
> On Mon, 2025-10-20 at 08:21 -0400, Mimi Zohar wrote:
> > On Sat, 2025-10-18 at 07:19 +0800, Coiby Xu wrote:
> > > > > > 2. Instead of defining an additional process_measurement() argument to identify
> > > > > > compressed kernel modules, to simplify the code it might be possible to define a
> > > > > > new "func" named COMPRESSED_MODULE_CHECK.
> > > > > >
> > > > > > + [READING_COMPRESSED_MODULE] = MODULE_CHECK, -> COMPRESSED_MODULE_CHECK
> > > > >
> > > > > I also thought about this approach. But IMA rule maps kernel module
> > > > > loading to MODULE_CHECK. If we define a new rule and ask users to use
> > > > > this new rule, ima_policy=secure_boot still won't work.
> > > >
> > > > I don't have a problem with extending the "secure-boot" policy to support
> > > > uncompressed kernel modules appended signatures, based on whether
> > > > CONFIG_MODULE_SIG is enabled. The new rule would be in addition to the existing
> > > > MODULE_CHECK rule.
> > >
> > > I assume once the new rule get added, we can't remove it for userspace
> > > backward compatibility, right? And with CPIO xattr supported, it seems
> > > there is no need to keep this rule. So if this concern is valid, do you
> > > think we shall switch to another approach i.e. to make IMA support
> > > verifying decompressed module and then make "secure-boot" to allow
> > > appended module signature?
> >
> > Yes, once the rule is added, it wouldn't be removed. As for "to make IMA
> > support verifying decompressed module", yes that might be a better solution,
> > than relying on "sig_enforce" being enabled. IMA already supports verifying the
> > appended signatures. A new IMA specific or LSM hook would need to be defined
> > after module_decompress().
> >
> > Remember based on policy, IMA supports:
> > 1. verifying the signature stored in security.ima xattr
> > 2. verifying the appended signature (not for compressed kernel modules)
> > 3. verifying both the xattr and appended signatures
> > 4. none
> >
> > To prevent 3 - verifying both types of signatures, the IMA arch specific policy
> > rule only adds the "appraise func=MODULE_CHECK ..." rule if CONFIG_MODULE_SIG is
> > NOT enabled. Calling set_module_sig_enforced() from ima_appraise_measurement()
> > to set sig_enforce could inadvertently result in requiring both the xattr and
> > the appended signature kernel module verification. To prevent this from
> > happening, "sig_enforce" should not be set, only verified in
> > ima_appraise_measurement().
> >
> > >
> > > Another thought is to make CPIO support xattr. Today I realize that
> > > ima_policy=secure_boot can also cause failure of loading kdump kernel.
> > > So the issue this patch tries to resolves has much less impact than I
> > > thought. Maybe we can wait until CPIO xattr support is ready? I'll help
> > > review and test Roberto's patches if this is the best way forward.
> >
> > I'm not sure of the status of the CPIO patch set. Roberto?
>
> I haven't had time to look at it recently. I can take the openEuler
> version, address the remaining comments and repost.
Thank you!
^ permalink raw reply
* Re: [PATCH] ima: Fall back to default kernel module signature verification
From: Roberto Sassu @ 2025-10-20 12:45 UTC (permalink / raw)
To: Mimi Zohar, Coiby Xu
Cc: linux-integrity, Dmitry Torokhov, Karel Srot, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris,
Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <559f6ebf4a19da321fffc2a3ca180dc3d6216a22.camel@linux.ibm.com>
On Mon, 2025-10-20 at 08:21 -0400, Mimi Zohar wrote:
> On Sat, 2025-10-18 at 07:19 +0800, Coiby Xu wrote:
> > > > > 2. Instead of defining an additional process_measurement() argument to identify
> > > > > compressed kernel modules, to simplify the code it might be possible to define a
> > > > > new "func" named COMPRESSED_MODULE_CHECK.
> > > > >
> > > > > + [READING_COMPRESSED_MODULE] = MODULE_CHECK, -> COMPRESSED_MODULE_CHECK
> > > >
> > > > I also thought about this approach. But IMA rule maps kernel module
> > > > loading to MODULE_CHECK. If we define a new rule and ask users to use
> > > > this new rule, ima_policy=secure_boot still won't work.
> > >
> > > I don't have a problem with extending the "secure-boot" policy to support
> > > uncompressed kernel modules appended signatures, based on whether
> > > CONFIG_MODULE_SIG is enabled. The new rule would be in addition to the existing
> > > MODULE_CHECK rule.
> >
> > I assume once the new rule get added, we can't remove it for userspace
> > backward compatibility, right? And with CPIO xattr supported, it seems
> > there is no need to keep this rule. So if this concern is valid, do you
> > think we shall switch to another approach i.e. to make IMA support
> > verifying decompressed module and then make "secure-boot" to allow
> > appended module signature?
>
> Yes, once the rule is added, it wouldn't be removed. As for "to make IMA
> support verifying decompressed module", yes that might be a better solution,
> than relying on "sig_enforce" being enabled. IMA already supports verifying the
> appended signatures. A new IMA specific or LSM hook would need to be defined
> after module_decompress().
>
> Remember based on policy, IMA supports:
> 1. verifying the signature stored in security.ima xattr
> 2. verifying the appended signature (not for compressed kernel modules)
> 3. verifying both the xattr and appended signatures
> 4. none
>
> To prevent 3 - verifying both types of signatures, the IMA arch specific policy
> rule only adds the "appraise func=MODULE_CHECK ..." rule if CONFIG_MODULE_SIG is
> NOT enabled. Calling set_module_sig_enforced() from ima_appraise_measurement()
> to set sig_enforce could inadvertently result in requiring both the xattr and
> the appended signature kernel module verification. To prevent this from
> happening, "sig_enforce" should not be set, only verified in
> ima_appraise_measurement().
>
> >
> > Another thought is to make CPIO support xattr. Today I realize that
> > ima_policy=secure_boot can also cause failure of loading kdump kernel.
> > So the issue this patch tries to resolves has much less impact than I
> > thought. Maybe we can wait until CPIO xattr support is ready? I'll help
> > review and test Roberto's patches if this is the best way forward.
>
> I'm not sure of the status of the CPIO patch set. Roberto?
I haven't had time to look at it recently. I can take the openEuler
version, address the remaining comments and repost.
Roberto
^ permalink raw reply
* Re: [PATCH] ima: Fall back to default kernel module signature verification
From: Mimi Zohar @ 2025-10-20 12:21 UTC (permalink / raw)
To: Coiby Xu
Cc: linux-integrity, Dmitry Torokhov, Karel Srot, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris,
Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <z6f4getlayaxaxvlxfxn2yvn5dvhrct64wke4uu2s3dfll3bqq@754bklrku55n>
On Sat, 2025-10-18 at 07:19 +0800, Coiby Xu wrote:
> > > > 2. Instead of defining an additional process_measurement() argument to identify
> > > > compressed kernel modules, to simplify the code it might be possible to define a
> > > > new "func" named COMPRESSED_MODULE_CHECK.
> > > >
> > > > + [READING_COMPRESSED_MODULE] = MODULE_CHECK, -> COMPRESSED_MODULE_CHECK
> > >
> > > I also thought about this approach. But IMA rule maps kernel module
> > > loading to MODULE_CHECK. If we define a new rule and ask users to use
> > > this new rule, ima_policy=secure_boot still won't work.
> >
> > I don't have a problem with extending the "secure-boot" policy to support
> > uncompressed kernel modules appended signatures, based on whether
> > CONFIG_MODULE_SIG is enabled. The new rule would be in addition to the existing
> > MODULE_CHECK rule.
>
> I assume once the new rule get added, we can't remove it for userspace
> backward compatibility, right? And with CPIO xattr supported, it seems
> there is no need to keep this rule. So if this concern is valid, do you
> think we shall switch to another approach i.e. to make IMA support
> verifying decompressed module and then make "secure-boot" to allow
> appended module signature?
Yes, once the rule is added, it wouldn't be removed. As for "to make IMA
support verifying decompressed module", yes that might be a better solution,
than relying on "sig_enforce" being enabled. IMA already supports verifying the
appended signatures. A new IMA specific or LSM hook would need to be defined
after module_decompress().
Remember based on policy, IMA supports:
1. verifying the signature stored in security.ima xattr
2. verifying the appended signature (not for compressed kernel modules)
3. verifying both the xattr and appended signatures
4. none
To prevent 3 - verifying both types of signatures, the IMA arch specific policy
rule only adds the "appraise func=MODULE_CHECK ..." rule if CONFIG_MODULE_SIG is
NOT enabled. Calling set_module_sig_enforced() from ima_appraise_measurement()
to set sig_enforce could inadvertently result in requiring both the xattr and
the appended signature kernel module verification. To prevent this from
happening, "sig_enforce" should not be set, only verified in
ima_appraise_measurement().
>
> Another thought is to make CPIO support xattr. Today I realize that
> ima_policy=secure_boot can also cause failure of loading kdump kernel.
> So the issue this patch tries to resolves has much less impact than I
> thought. Maybe we can wait until CPIO xattr support is ready? I'll help
> review and test Roberto's patches if this is the best way forward.
I'm not sure of the status of the CPIO patch set. Roberto?
Mimi
^ permalink raw reply
* Re: [PATCH v6 10/10] tpm-buf: Enable managed and stack allocations.
From: Stefano Garzarella @ 2025-10-20 9:04 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, keyring, dpsmith, ross.philipson,
Jonathan McDowell, Roberto Sassu, Jarkko Sakkinen, Stefan Berger,
Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
linux-kernel, keyrings, linux-security-module
In-Reply-To: <20251018111725.3116386-11-jarkko@kernel.org>
On Sat, Oct 18, 2025 at 02:17:25PM +0300, Jarkko Sakkinen wrote:
>From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>
>Decouple kzalloc from buffer creation, so that a managed allocation can be
>done trivially:
>
> struct tpm_buf *buf __free(kfree) buf = kzalloc(TPM_BUFSIZE,
> GFP_KERNEL);
> if (!buf)
> return -ENOMEM;
> tpm_buf_init(buf, TPM_BUFSIZE);
>
>Alternatively, stack allocations are also possible:
>
> u8 buf_data[512];
> struct tpm_buf *buf = (struct tpm_buf *)buf_data;
> tpm_buf_init(buf, sizeof(buf_data));
>
>Given that every single tpm_transmit_cmd() call site needs to be changed,
>place command names from TCG 1.2 and 2.0 specifications to the @dest
>parameter, which will e.g., help tracing bugs.
Perhaps my previous message fell through the cracks, but I still have a
couple of comments (perhaps trivial, sorry in that case) that have not
been answered about this patch:
>
>Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>---
>v6
>- Update commit message.
>v5:
>- There was a spurious change in tpm2_seal_trusted() error
> code handling introduce by this patch.
>v4:
>- Since every single tpm_transmit_cmd() call site needs to be
> changed anyhow, use 'dest' parameter more structured and
> actually useful way, and pick the string TCG 1.2 and 2.0
> specifications.
>- tpm1-cmd: Remove useless rc declarations and repliace them
> with trivial "return tpm_transmit_cmd" statement.
>- Reverted spurious changes in include/linux/tpm.h.
>- Use concisely TPM_BUFSIZE instead of PAGE_SIZE.
>v3:
>- A new patch from the earlier series with more scoped changes and
> less abstract commit message.
>---
> drivers/char/tpm/tpm-buf.c | 122 +++++----
> drivers/char/tpm/tpm-sysfs.c | 21 +-
> drivers/char/tpm/tpm.h | 1 -
> drivers/char/tpm/tpm1-cmd.c | 162 +++++-------
> drivers/char/tpm/tpm2-cmd.c | 299 ++++++++++------------
> drivers/char/tpm/tpm2-sessions.c | 122 +++++----
> drivers/char/tpm/tpm2-space.c | 44 ++--
> drivers/char/tpm/tpm_vtpm_proxy.c | 30 +--
> include/linux/tpm.h | 18 +-
> security/keys/trusted-keys/trusted_tpm1.c | 34 ++-
> security/keys/trusted-keys/trusted_tpm2.c | 175 ++++++-------
> 11 files changed, 484 insertions(+), 544 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
>index 1b9dee0d0681..a3bf3c3d0c48 100644
>--- a/drivers/char/tpm/tpm-buf.c
>+++ b/drivers/char/tpm/tpm-buf.c
[...]
>@@ -92,6 +119,9 @@ EXPORT_SYMBOL_GPL(tpm_buf_destroy);
> */
> u32 tpm_buf_length(struct tpm_buf *buf)
Should we update the return value to u16?
> {
>+ if (buf->flags & TPM_BUF_INVALID)
>+ return 0;
>+
> return buf->length;
> }
> EXPORT_SYMBOL_GPL(tpm_buf_length);
[...]
>diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
>index 636acb66a4f6..3ac204a902de 100644
>--- a/security/keys/trusted-keys/trusted_tpm1.c
>+++ b/security/keys/trusted-keys/trusted_tpm1.c
>@@ -310,9 +310,8 @@ static int TSS_checkhmac2(unsigned char *buffer,
> * For key specific tpm requests, we will generate and send our
> * own TPM command packets using the drivers send function.
> */
>-static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
>+static int trusted_tpm_send(void *cmd, size_t buflen)
> {
>- struct tpm_buf buf;
> int rc;
>
> if (!chip)
>@@ -322,15 +321,12 @@ static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
> if (rc)
> return rc;
>
>- buf.flags = 0;
>- buf.length = buflen;
>- buf.data = cmd;
> dump_tpm_buf(cmd);
>- rc = tpm_transmit_cmd(chip, &buf, 4, "sending data");
>+ rc = tpm_transmit_cmd(chip, cmd, 4, "sending data");
Is it fine here to remove the intermediate tpm_buf ?
IIUC tpm_transmit_cmd() needs a tpm_buf, while here we are passing just
the "data", or in some way it's a nested tpm_buf?
> dump_tpm_buf(cmd);
>
>+ /* Convert TPM error to -EPERM. */
> if (rc > 0)
>- /* TPM error */
> rc = -EPERM;
>
> tpm_put_ops(chip);
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH v2 6/6] Implement quiet for optional accesses
From: Tingmao Wang @ 2025-10-19 17:45 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Jan Kara, linux-security-module
In-Reply-To: <20251015.sohxe1NohFei@digikod.net>
On 10/15/25 20:09, Mickaël Salaün wrote:
> [...]
> On Sun, Oct 05, 2025 at 06:55:29PM +0100, Tingmao Wang wrote:
>> [..]
>> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
>> index a647b68e8d06..0f611ad516be 100644
>> --- a/security/landlock/domain.c
>> +++ b/security/landlock/domain.c
>> @@ -212,6 +212,29 @@ landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
>> return deny_masks;
>> }
>>
>
> Just using u8 is confusing. Please document what is the "type" of the
> returned value, and use a dedicated typedef instead of u8 (see my other
> comment about static_assert). This typedef should probably be named
> optional_access_t and have a size less or equal to access_t's one.
>
>> +u8 landlock_get_quiet_optional_accesses(
>> + const access_mask_t all_existing_optional_access,
>> + const deny_masks_t deny_masks,
>> + const struct collected_rule_flags rule_flags)
>> +{
>> + const unsigned long access_opt = all_existing_optional_access;
>> + size_t access_index = 0;
>> + unsigned long access_bit;
>> + u8 quiet_optional_accesses = 0;
>
> As for deny_masks_t, we should define an "optional_access_t" type with
> appropriate safeguard to guarantee that it can always hold all optional
> access rights (see static_assert for deny_masks_t in access.h).
>
> We should also copy the WARN_ON_ONCE() check from
> get_layer_from_deny_masks().
I don't see how that WARN_ON_ONCE is applicable here since we're no longer
dealing with the `optional_access`... Can you clarify?
>
>> [...]
>> @@ -80,13 +88,24 @@ struct landlock_file_security {
>> struct landlock_cred_security fown_subject;
>> };
>>
>> -#ifdef CONFIG_AUDIT
>> +static void build_check_file_security(void)
>
> You can move this function to fs.c and call it in
> hook_file_alloc_security() instead.
>
>> +{
>> + const struct landlock_file_security file_sec = {
>> + .quiet_optional_accesses = ~0,
>> + .fown_layer = ~0,
>> + };
>> +
>> + /*
>> + * Make sure quiet_optional_accesses has enough bits to cover all
>> + * optional accesses
>> + */
>> + BUILD_BUG_ON(__const_hweight8(file_sec.quiet_optional_accesses) <
>
> We should be able to use HWEIGHT() instead.
I tried it and unfortunately it doesn't seem to work :(
security/landlock/fs.c: In function ‘build_check_file_security’:
./include/linux/compiler.h:201:82: error: expression in static assertion is not constant
201 | #define __BUILD_BUG_ON_ZERO_MSG(e, msg, ...) ((int)sizeof(struct {_Static_assert(!(e), msg);}))
| ^~~~
././include/linux/compiler_types.h:577:23: note: in definition of macro ‘__compiletime_assert’
577 | if (!(condition)) \
| ^~~~~~~~~
././include/linux/compiler_types.h:597:9: note: in expansion of macro ‘_compiletime_assert’
597 | _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
| ^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
| ^~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:50:9: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
| ^~~~~~~~~~~~~~~~
security/landlock/fs.c:1769:9: note: in expansion of macro ‘BUILD_BUG_ON’
1769 | BUILD_BUG_ON(HWEIGHT(file_sec.quiet_optional_accesses) <
| ^~~~~~~~~~~~
./include/linux/build_bug.h:17:9: note: in expansion of macro ‘__BUILD_BUG_ON_ZERO_MSG’
17 | __BUILD_BUG_ON_ZERO_MSG(e, ##__VA_ARGS__, #e " is true")
| ^~~~~~~~~~~~~~~~~~~~~~~
./include/asm-generic/bitops/const_hweight.h:37:23: note: in expansion of macro ‘BUILD_BUG_ON_ZERO’
37 | #define HWEIGHT64(w) (BUILD_BUG_ON_ZERO(!__builtin_constant_p(w)) + __const_hweight64(w))
| ^~~~~~~~~~~~~~~~~
./include/asm-generic/bitops/const_hweight.h:42:22: note: in expansion of macro ‘HWEIGHT64’
42 | #define HWEIGHT(w) HWEIGHT64((u64)w)
| ^~~~~~~~~
security/landlock/fs.c:1769:22: note: in expansion of macro ‘HWEIGHT’
1769 | BUILD_BUG_ON(HWEIGHT(file_sec.quiet_optional_accesses) <
| ^~~~~~~
>
>> + __const_hweight64(_LANDLOCK_ACCESS_FS_OPTIONAL));
>> + /* Makes sure all layers can be identified. */
>> + BUILD_BUG_ON(file_sec.fown_layer < LANDLOCK_MAX_NUM_LAYERS - 1);
>> +}
>>
>> -/* Makes sure all layers can be identified. */
>> -/* clang-format off */
>> -static_assert((typeof_member(struct landlock_file_security, fown_layer))~0 >=
>> - LANDLOCK_MAX_NUM_LAYERS);
>> -/* clang-format off */
>> +#ifdef CONFIG_AUDIT
>>
>> #endif /* CONFIG_AUDIT */
>>
>> @@ -107,6 +126,7 @@ struct landlock_superblock_security {
>> static inline struct landlock_file_security *
>> landlock_file(const struct file *const file)
>> {
>> + build_check_file_security();
>> return file->f_security + landlock_blob_sizes.lbs_file;
>> }
>>
>> --
>> 2.51.0
^ permalink raw reply
* Re: [PATCH v2 3/6] landlock/audit: Check for quiet flag in landlock_log_denial
From: Tingmao Wang @ 2025-10-19 17:39 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Jan Kara, linux-security-module
In-Reply-To: <20251015.Iengoh1eeT0c@digikod.net>
On 10/15/25 20:09, Mickaël Salaün wrote:
> Just use "landlock: " as subject prefix.
What about samples/landlock or selftests/landlock (in the next version)?
Should they also just be landlock: ?
^ permalink raw reply
* [PATCH v6 10/10] tpm-buf: Enable managed and stack allocations.
From: Jarkko Sakkinen @ 2025-10-18 11:17 UTC (permalink / raw)
To: linux-integrity
Cc: keyring, dpsmith, ross.philipson, Jonathan McDowell,
Roberto Sassu, Stefano Garzarella, Jarkko Sakkinen, Stefan Berger,
Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, James Bottomley,
Mimi Zohar, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, linux-kernel, keyrings, linux-security-module
In-Reply-To: <20251018111725.3116386-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Decouple kzalloc from buffer creation, so that a managed allocation can be
done trivially:
struct tpm_buf *buf __free(kfree) buf = kzalloc(TPM_BUFSIZE,
GFP_KERNEL);
if (!buf)
return -ENOMEM;
tpm_buf_init(buf, TPM_BUFSIZE);
Alternatively, stack allocations are also possible:
u8 buf_data[512];
struct tpm_buf *buf = (struct tpm_buf *)buf_data;
tpm_buf_init(buf, sizeof(buf_data));
Given that every single tpm_transmit_cmd() call site needs to be changed,
place command names from TCG 1.2 and 2.0 specifications to the @dest
parameter, which will e.g., help tracing bugs.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
---
v6
- Update commit message.
v5:
- There was a spurious change in tpm2_seal_trusted() error
code handling introduce by this patch.
v4:
- Since every single tpm_transmit_cmd() call site needs to be
changed anyhow, use 'dest' parameter more structured and
actually useful way, and pick the string TCG 1.2 and 2.0
specifications.
- tpm1-cmd: Remove useless rc declarations and repliace them
with trivial "return tpm_transmit_cmd" statement.
- Reverted spurious changes in include/linux/tpm.h.
- Use concisely TPM_BUFSIZE instead of PAGE_SIZE.
v3:
- A new patch from the earlier series with more scoped changes and
less abstract commit message.
---
drivers/char/tpm/tpm-buf.c | 122 +++++----
drivers/char/tpm/tpm-sysfs.c | 21 +-
drivers/char/tpm/tpm.h | 1 -
drivers/char/tpm/tpm1-cmd.c | 162 +++++-------
drivers/char/tpm/tpm2-cmd.c | 299 ++++++++++------------
drivers/char/tpm/tpm2-sessions.c | 122 +++++----
drivers/char/tpm/tpm2-space.c | 44 ++--
drivers/char/tpm/tpm_vtpm_proxy.c | 30 +--
include/linux/tpm.h | 18 +-
security/keys/trusted-keys/trusted_tpm1.c | 34 ++-
security/keys/trusted-keys/trusted_tpm2.c | 175 ++++++-------
11 files changed, 484 insertions(+), 544 deletions(-)
diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
index 1b9dee0d0681..a3bf3c3d0c48 100644
--- a/drivers/char/tpm/tpm-buf.c
+++ b/drivers/char/tpm/tpm-buf.c
@@ -7,82 +7,109 @@
#include <linux/module.h>
#include <linux/tpm.h>
-/**
- * tpm_buf_init() - Allocate and initialize a TPM command
- * @buf: A &tpm_buf
- * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS
- * @ordinal: A command ordinal
- *
- * Return: 0 or -ENOMEM
- */
-int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+static void __tpm_buf_size_invariant(struct tpm_buf *buf, u16 buf_size)
{
- buf->data = (u8 *)__get_free_page(GFP_KERNEL);
- if (!buf->data)
- return -ENOMEM;
-
- tpm_buf_reset(buf, tag, ordinal);
- return 0;
+ u32 buf_size_2 = (u32)buf->capacity + (u32)sizeof(*buf);
+
+ if (!buf->capacity) {
+ if (buf_size > TPM_BUFSIZE) {
+ WARN(1, "%s: size overflow: %u\n", __func__, buf_size);
+ buf->flags |= TPM_BUF_INVALID;
+ }
+ } else {
+ if (buf_size != buf_size_2) {
+ WARN(1, "%s: size mismatch: %u != %u\n", __func__, buf_size,
+ buf_size_2);
+ buf->flags |= TPM_BUF_INVALID;
+ }
+ }
}
-EXPORT_SYMBOL_GPL(tpm_buf_init);
-/**
- * tpm_buf_reset() - Initialize a TPM command
- * @buf: A &tpm_buf
- * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS
- * @ordinal: A command ordinal
- */
-void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
+static void __tpm_buf_reset(struct tpm_buf *buf, u16 buf_size, u16 tag, u32 ordinal)
{
struct tpm_header *head = (struct tpm_header *)buf->data;
+ __tpm_buf_size_invariant(buf, buf_size);
+
+ if (buf->flags & TPM_BUF_INVALID)
+ return;
+
WARN_ON(tag != TPM_TAG_RQU_COMMAND && tag != TPM2_ST_NO_SESSIONS &&
tag != TPM2_ST_SESSIONS && tag != 0);
buf->flags = 0;
buf->length = sizeof(*head);
+ buf->capacity = buf_size - sizeof(*buf);
+ buf->handles = 0;
head->tag = cpu_to_be16(tag);
head->length = cpu_to_be32(sizeof(*head));
head->ordinal = cpu_to_be32(ordinal);
+}
+
+static void __tpm_buf_reset_sized(struct tpm_buf *buf, u16 buf_size)
+{
+ __tpm_buf_size_invariant(buf, buf_size);
+
+ if (buf->flags & TPM_BUF_INVALID)
+ return;
+
+ buf->flags = TPM_BUF_TPM2B;
+ buf->length = 2;
+ buf->capacity = buf_size - sizeof(*buf);
buf->handles = 0;
+ buf->data[0] = 0;
+ buf->data[1] = 0;
}
-EXPORT_SYMBOL_GPL(tpm_buf_reset);
/**
- * tpm_buf_init_sized() - Allocate and initialize a sized (TPM2B) buffer
- * @buf: A @tpm_buf
- *
- * Return: 0 or -ENOMEM
+ * tpm_buf_init() - Initialize a TPM command
+ * @buf: A &tpm_buf
+ * @buf_size: Size of the buffer.
*/
-int tpm_buf_init_sized(struct tpm_buf *buf)
+void tpm_buf_init(struct tpm_buf *buf, u16 buf_size)
{
- buf->data = (u8 *)__get_free_page(GFP_KERNEL);
- if (!buf->data)
- return -ENOMEM;
+ memset(buf, 0, buf_size);
+ __tpm_buf_reset(buf, buf_size, TPM_TAG_RQU_COMMAND, 0);
+}
+EXPORT_SYMBOL_GPL(tpm_buf_init);
- tpm_buf_reset_sized(buf);
- return 0;
+/**
+ * tpm_buf_init_sized() - Initialize a sized buffer
+ * @buf: A &tpm_buf
+ * @buf_size: Size of the buffer.
+ */
+void tpm_buf_init_sized(struct tpm_buf *buf, u16 buf_size)
+{
+ memset(buf, 0, buf_size);
+ __tpm_buf_reset_sized(buf, buf_size);
}
EXPORT_SYMBOL_GPL(tpm_buf_init_sized);
/**
- * tpm_buf_reset_sized() - Initialize a sized buffer
+ * tpm_buf_reset() - Re-initialize a TPM command
* @buf: A &tpm_buf
+ * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS
+ * @ordinal: A command ordinal
*/
-void tpm_buf_reset_sized(struct tpm_buf *buf)
+void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal)
{
- buf->flags = TPM_BUF_TPM2B;
- buf->length = 2;
- buf->data[0] = 0;
- buf->data[1] = 0;
+ u16 buf_size = buf->capacity + sizeof(*buf);
+
+ __tpm_buf_reset(buf, buf_size, tag, ordinal);
}
-EXPORT_SYMBOL_GPL(tpm_buf_reset_sized);
+EXPORT_SYMBOL_GPL(tpm_buf_reset);
-void tpm_buf_destroy(struct tpm_buf *buf)
+/**
+ * tpm_buf_reset_sized() - Re-initialize a sized buffer
+ * @buf: A &tpm_buf
+ */
+void tpm_buf_reset_sized(struct tpm_buf *buf)
{
- free_page((unsigned long)buf->data);
+ u16 buf_size = buf->capacity + sizeof(*buf);
+
+ __tpm_buf_reset_sized(buf, buf_size);
}
-EXPORT_SYMBOL_GPL(tpm_buf_destroy);
+EXPORT_SYMBOL_GPL(tpm_buf_reset_sized);
/**
* tpm_buf_length() - Return the number of bytes consumed by the data
@@ -92,6 +119,9 @@ EXPORT_SYMBOL_GPL(tpm_buf_destroy);
*/
u32 tpm_buf_length(struct tpm_buf *buf)
{
+ if (buf->flags & TPM_BUF_INVALID)
+ return 0;
+
return buf->length;
}
EXPORT_SYMBOL_GPL(tpm_buf_length);
@@ -104,10 +134,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_length);
*/
void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length)
{
+ u32 total_length = (u32)buf->length + (u32)new_length;
+
if (buf->flags & TPM_BUF_INVALID)
return;
- if ((buf->length + new_length) > PAGE_SIZE) {
+ if (total_length > (u32)buf->capacity) {
WARN(1, "tpm_buf: write overflow\n");
buf->flags |= TPM_BUF_INVALID;
return;
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 94231f052ea7..f5dcadb1ab3c 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -32,28 +32,29 @@ struct tpm_readpubek_out {
static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
- struct tpm_buf tpm_buf;
struct tpm_readpubek_out *out;
int i;
char *str = buf;
struct tpm_chip *chip = to_tpm_chip(dev);
char anti_replay[20];
+ struct tpm_buf *tpm_buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!tpm_buf)
+ return -ENOMEM;
+
memset(&anti_replay, 0, sizeof(anti_replay));
if (tpm_try_get_ops(chip))
return 0;
- if (tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK))
- goto out_ops;
-
- tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay));
+ tpm_buf_init(tpm_buf, TPM_BUFSIZE);
+ tpm_buf_reset(tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK);
+ tpm_buf_append(tpm_buf, anti_replay, sizeof(anti_replay));
- if (tpm_transmit_cmd(chip, &tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE,
- "attempting to read the PUBEK"))
- goto out_buf;
+ if (tpm_transmit_cmd(chip, tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE, "TPM_ReadPubek"))
+ goto out_ops;
- out = (struct tpm_readpubek_out *)&tpm_buf.data[10];
+ out = (struct tpm_readpubek_out *)&tpm_buf->data[10];
str +=
sprintf(str,
"Algorithm: %4ph\n"
@@ -71,8 +72,6 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
for (i = 0; i < 256; i += 16)
str += sprintf(str, "%16ph\n", &out->modulus[i]);
-out_buf:
- tpm_buf_destroy(&tpm_buf);
out_ops:
tpm_put_ops(chip);
return str - buf;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a37712c02e44..ca391b2a211c 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -32,7 +32,6 @@
#endif
#define TPM_MINOR 224 /* officially assigned */
-#define TPM_BUFSIZE 4096
#define TPM_NUM_DEVICES 65536
#define TPM_RETRY 50
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 5c49bdff33de..10cb5d008c9f 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -323,20 +323,14 @@ unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
*/
static int tpm1_startup(struct tpm_chip *chip)
{
- struct tpm_buf buf;
- int rc;
-
- dev_info(&chip->dev, "starting up the TPM manually\n");
-
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_STARTUP);
- if (rc < 0)
- return rc;
-
- tpm_buf_append_u16(&buf, TPM_ST_CLEAR);
-
- rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to start the TPM");
- tpm_buf_destroy(&buf);
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_STARTUP);
+ tpm_buf_append_u16(buf, TPM_ST_CLEAR);
+ return tpm_transmit_cmd(chip, buf, 0, "TPM_Startup");
}
int tpm1_get_timeouts(struct tpm_chip *chip)
@@ -463,50 +457,45 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
const char *log_msg)
{
- struct tpm_buf buf;
- int rc;
-
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- tpm_buf_append_u32(&buf, pcr_idx);
- tpm_buf_append(&buf, hash, TPM_DIGEST_SIZE);
-
- rc = tpm_transmit_cmd(chip, &buf, TPM_DIGEST_SIZE, log_msg);
- tpm_buf_destroy(&buf);
- return rc;
+ tpm_buf_append_u32(buf, pcr_idx);
+ tpm_buf_append(buf, hash, TPM_DIGEST_SIZE);
+ return tpm_transmit_cmd(chip, buf, TPM_DIGEST_SIZE, log_msg);
}
#define TPM_ORD_GET_CAP 101
ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
const char *desc, size_t min_cap_length)
{
- struct tpm_buf buf;
int rc;
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_CAP);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_CAP);
if (subcap_id == TPM_CAP_VERSION_1_1 ||
subcap_id == TPM_CAP_VERSION_1_2) {
- tpm_buf_append_u32(&buf, subcap_id);
- tpm_buf_append_u32(&buf, 0);
+ tpm_buf_append_u32(buf, subcap_id);
+ tpm_buf_append_u32(buf, 0);
} else {
if (subcap_id == TPM_CAP_FLAG_PERM ||
subcap_id == TPM_CAP_FLAG_VOL)
- tpm_buf_append_u32(&buf, TPM_CAP_FLAG);
+ tpm_buf_append_u32(buf, TPM_CAP_FLAG);
else
- tpm_buf_append_u32(&buf, TPM_CAP_PROP);
+ tpm_buf_append_u32(buf, TPM_CAP_PROP);
- tpm_buf_append_u32(&buf, 4);
- tpm_buf_append_u32(&buf, subcap_id);
+ tpm_buf_append_u32(buf, 4);
+ tpm_buf_append_u32(buf, subcap_id);
}
- rc = tpm_transmit_cmd(chip, &buf, min_cap_length, desc);
+ rc = tpm_transmit_cmd(chip, buf, min_cap_length, desc);
if (!rc)
- *cap = *(cap_t *)&buf.data[TPM_HEADER_SIZE + 4];
- tpm_buf_destroy(&buf);
+ *cap = *(cap_t *)&buf->data[TPM_HEADER_SIZE + 4];
return rc;
}
EXPORT_SYMBOL_GPL(tpm1_getcap);
@@ -531,81 +520,69 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
{
struct tpm1_get_random_out *out;
u32 num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
- struct tpm_buf buf;
u32 total = 0;
int retries = 5;
u32 recd;
int rc;
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_RANDOM);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+ tpm_buf_init(buf, TPM_BUFSIZE);
do {
- tpm_buf_append_u32(&buf, num_bytes);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_RANDOM);
+ tpm_buf_append_u32(buf, num_bytes);
- rc = tpm_transmit_cmd(chip, &buf, sizeof(out->rng_data_len),
- "attempting get random");
+ rc = tpm_transmit_cmd(chip, buf, sizeof(out->rng_data_len), "TPM_GetRandom");
if (rc) {
if (rc > 0)
rc = -EIO;
- goto out;
+ return rc;
}
- out = (struct tpm1_get_random_out *)&buf.data[TPM_HEADER_SIZE];
+ out = (struct tpm1_get_random_out *)&buf->data[TPM_HEADER_SIZE];
recd = be32_to_cpu(out->rng_data_len);
- if (recd > num_bytes) {
- rc = -EFAULT;
- goto out;
- }
+ if (recd > num_bytes)
+ return -EFAULT;
+
+ if (buf->length < TPM_HEADER_SIZE +
+ sizeof(out->rng_data_len) + recd)
+ return -EFAULT;
- if (tpm_buf_length(&buf) < TPM_HEADER_SIZE +
- sizeof(out->rng_data_len) + recd) {
- rc = -EFAULT;
- goto out;
- }
memcpy(dest, out->rng_data, recd);
dest += recd;
total += recd;
num_bytes -= recd;
-
- tpm_buf_reset(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_RANDOM);
} while (retries-- && total < max);
rc = total ? (int)total : -EIO;
-out:
- tpm_buf_destroy(&buf);
return rc;
}
#define TPM_ORD_PCRREAD 21
int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
{
- struct tpm_buf buf;
int rc;
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCRREAD);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- tpm_buf_append_u32(&buf, pcr_idx);
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCRREAD);
+ tpm_buf_append_u32(buf, pcr_idx);
- rc = tpm_transmit_cmd(chip, &buf, TPM_DIGEST_SIZE,
- "attempting to read a pcr value");
+ rc = tpm_transmit_cmd(chip, buf, TPM_DIGEST_SIZE, "TPM_PCRRead");
if (rc)
- goto out;
-
- if (tpm_buf_length(&buf) < TPM_DIGEST_SIZE) {
- rc = -EFAULT;
- goto out;
- }
+ return rc;
- memcpy(res_buf, &buf.data[TPM_HEADER_SIZE], TPM_DIGEST_SIZE);
+ if (buf->length < TPM_DIGEST_SIZE)
+ return -EFAULT;
-out:
- tpm_buf_destroy(&buf);
+ memcpy(res_buf, &buf->data[TPM_HEADER_SIZE], TPM_DIGEST_SIZE);
return rc;
}
@@ -619,16 +596,13 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
*/
static int tpm1_continue_selftest(struct tpm_chip *chip)
{
- struct tpm_buf buf;
- int rc;
-
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_CONTINUE_SELFTEST);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- rc = tpm_transmit_cmd(chip, &buf, 0, "continue selftest");
- tpm_buf_destroy(&buf);
- return rc;
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_CONTINUE_SELFTEST);
+ return tpm_transmit_cmd(chip, buf, 0, "TPM_ContinueSelfTest");
}
/**
@@ -742,22 +716,24 @@ int tpm1_auto_startup(struct tpm_chip *chip)
int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
{
u8 dummy_hash[TPM_DIGEST_SIZE] = { 0 };
- struct tpm_buf buf;
unsigned int try;
int rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
/* for buggy tpm, flush pcrs with extend to selected dummy */
if (tpm_suspend_pcr)
rc = tpm1_pcr_extend(chip, tpm_suspend_pcr, dummy_hash,
"extending dummy pcr before suspend");
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE);
- if (rc)
- return rc;
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE);
+
/* now do the actual savestate */
for (try = 0; try < TPM_RETRY; try++) {
- rc = tpm_transmit_cmd(chip, &buf, 0, NULL);
+ rc = tpm_transmit_cmd(chip, buf, 0, NULL);
/*
* If the TPM indicates that it is too busy to respond to
* this command then retry before giving up. It can take
@@ -772,7 +748,7 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
break;
tpm_msleep(TPM_TIMEOUT_RETRY);
- tpm_buf_reset(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE);
}
if (rc)
@@ -782,7 +758,5 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
dev_warn(&chip->dev, "TPM savestate took %dms\n",
try * TPM_TIMEOUT_RETRY);
- tpm_buf_destroy(&buf);
-
return rc;
}
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index e548b0ac7654..768719bbbf04 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -104,12 +104,15 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
{
int i;
int rc;
- struct tpm_buf buf;
struct tpm2_pcr_read_out *out;
u8 pcr_select[TPM2_PCR_SELECT_MIN] = {0};
u16 digest_size;
u16 expected_digest_size = 0;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
if (pcr_idx >= TPM2_PLATFORM_PCR)
return -EINVAL;
@@ -124,36 +127,31 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
expected_digest_size = chip->allocated_banks[i].digest_size;
}
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ);
- if (rc)
- return rc;
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ);
pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
- tpm_buf_append_u32(&buf, 1);
- tpm_buf_append_u16(&buf, digest->alg_id);
- tpm_buf_append_u8(&buf, TPM2_PCR_SELECT_MIN);
- tpm_buf_append(&buf, (const unsigned char *)pcr_select,
+ tpm_buf_append_u32(buf, 1);
+ tpm_buf_append_u16(buf, digest->alg_id);
+ tpm_buf_append_u8(buf, TPM2_PCR_SELECT_MIN);
+ tpm_buf_append(buf, (const unsigned char *)pcr_select,
sizeof(pcr_select));
- rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to read a pcr value");
+ rc = tpm_transmit_cmd(chip, buf, 0, "TPM2_PCR_Read");
if (rc)
- goto out;
+ return rc;
- out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
+ out = (struct tpm2_pcr_read_out *)&buf->data[TPM_HEADER_SIZE];
digest_size = be16_to_cpu(out->digest_size);
if (digest_size > sizeof(digest->digest) ||
- (!digest_size_ptr && digest_size != expected_digest_size)) {
- rc = -EINVAL;
- goto out;
- }
+ (!digest_size_ptr && digest_size != expected_digest_size))
+ return rc;
if (digest_size_ptr)
*digest_size_ptr = digest_size;
memcpy(digest->digest, out->digest, digest_size);
-out:
- tpm_buf_destroy(&buf);
return rc;
}
@@ -169,46 +167,45 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digests)
{
- struct tpm_buf buf;
int rc;
int i;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
if (!disable_pcr_integrity) {
rc = tpm2_start_auth_session(chip);
if (rc)
return rc;
}
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
- if (rc) {
- if (!disable_pcr_integrity)
- tpm2_end_auth_session(chip);
- return rc;
- }
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND);
if (!disable_pcr_integrity) {
- tpm_buf_append_name(chip, &buf, pcr_idx, NULL);
- tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0);
+ tpm_buf_append_name(chip, buf, pcr_idx, NULL);
+ tpm_buf_append_hmac_session(chip, buf, 0, NULL, 0);
} else {
- tpm_buf_append_handle(&buf, pcr_idx);
- tpm_buf_append_auth(chip, &buf, NULL, 0);
+ tpm_buf_append_handle(buf, pcr_idx);
+ tpm_buf_append_auth(chip, buf, NULL, 0);
}
- tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
+ tpm_buf_append_u32(buf, chip->nr_allocated_banks);
for (i = 0; i < chip->nr_allocated_banks; i++) {
- tpm_buf_append_u16(&buf, digests[i].alg_id);
- tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
+ tpm_buf_append_u16(buf, digests[i].alg_id);
+ tpm_buf_append(buf, (const unsigned char *)&digests[i].digest,
chip->allocated_banks[i].digest_size);
}
+ if (buf->flags & TPM_BUF_INVALID)
+ return -EINVAL;
if (!disable_pcr_integrity)
- tpm_buf_fill_hmac_session(chip, &buf);
- rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
+ tpm_buf_fill_hmac_session(chip, buf);
+ rc = tpm_transmit_cmd(chip, buf, 0, "TPM2_PCR_Extend");
if (!disable_pcr_integrity)
- rc = tpm_buf_check_hmac_response(chip, &buf, rc);
-
- tpm_buf_destroy(&buf);
+ rc = tpm_buf_check_hmac_response(chip, buf, rc);
return rc;
}
@@ -233,7 +230,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
{
struct tpm2_get_random_out *out;
struct tpm_header *head;
- struct tpm_buf buf;
u32 recd;
u32 num_bytes = max;
int err;
@@ -245,51 +241,50 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
if (!num_bytes || max > TPM_MAX_RNG_DATA)
return -EINVAL;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
err = tpm2_start_auth_session(chip);
if (err)
return err;
- err = tpm_buf_init(&buf, 0, 0);
- if (err) {
- tpm2_end_auth_session(chip);
- return err;
- }
-
+ tpm_buf_init(buf, TPM_BUFSIZE);
do {
- tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
if (tpm2_chip_auth(chip)) {
- tpm_buf_append_hmac_session(chip, &buf,
+ tpm_buf_append_hmac_session(chip, buf,
TPM2_SA_ENCRYPT |
TPM2_SA_CONTINUE_SESSION,
NULL, 0);
} else {
- offset = buf.handles * 4 + TPM_HEADER_SIZE;
- head = (struct tpm_header *)buf.data;
- if (tpm_buf_length(&buf) == offset)
+ offset = buf->handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf->data;
+ if (tpm_buf_length(buf) == offset)
head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
}
- tpm_buf_append_u16(&buf, num_bytes);
- tpm_buf_fill_hmac_session(chip, &buf);
- err = tpm_transmit_cmd(chip, &buf,
+ tpm_buf_append_u16(buf, num_bytes);
+ tpm_buf_fill_hmac_session(chip, buf);
+ err = tpm_transmit_cmd(chip, buf,
offsetof(struct tpm2_get_random_out,
buffer),
- "attempting get random");
- err = tpm_buf_check_hmac_response(chip, &buf, err);
+ "TPM2_GetRandom");
+ err = tpm_buf_check_hmac_response(chip, buf, err);
if (err) {
if (err > 0)
err = -EIO;
goto out;
}
- head = (struct tpm_header *)buf.data;
+ head = (struct tpm_header *)buf->data;
offset = TPM_HEADER_SIZE;
/* Skip the parameter size field: */
if (be16_to_cpu(head->tag) == TPM2_ST_SESSIONS)
offset += 4;
- out = (struct tpm2_get_random_out *)&buf.data[offset];
+ out = (struct tpm2_get_random_out *)&buf->data[offset];
recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
- if (tpm_buf_length(&buf) <
+ if (tpm_buf_length(buf) <
TPM_HEADER_SIZE +
offsetof(struct tpm2_get_random_out, buffer) +
recd) {
@@ -303,11 +298,9 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
num_bytes -= recd;
} while (retries-- && total < max);
- tpm_buf_destroy(&buf);
-
return total ? total : -EIO;
+
out:
- tpm_buf_destroy(&buf);
tpm2_end_auth_session(chip);
return err;
}
@@ -319,20 +312,18 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
*/
void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
{
- struct tpm_buf buf;
- int rc;
-
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
- if (rc) {
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf) {
dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
handle);
return;
}
- tpm_buf_append_u32(&buf, handle);
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
+ tpm_buf_append_u32(buf, handle);
- tpm_transmit_cmd(chip, &buf, 0, "flushing context");
- tpm_buf_destroy(&buf);
+ tpm_transmit_cmd(chip, buf, 0, "TPM2_FlushContext");
}
EXPORT_SYMBOL_GPL(tpm2_flush_context);
@@ -359,19 +350,20 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value,
const char *desc)
{
struct tpm2_get_cap_out *out;
- struct tpm_buf buf;
int rc;
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
- if (rc)
- return rc;
- tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
- tpm_buf_append_u32(&buf, property_id);
- tpm_buf_append_u32(&buf, 1);
- rc = tpm_transmit_cmd(chip, &buf, 0, NULL);
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+ tpm_buf_append_u32(buf, TPM2_CAP_TPM_PROPERTIES);
+ tpm_buf_append_u32(buf, property_id);
+ tpm_buf_append_u32(buf, 1);
+ rc = tpm_transmit_cmd(chip, buf, 0, NULL);
if (!rc) {
- out = (struct tpm2_get_cap_out *)
- &buf.data[TPM_HEADER_SIZE];
+ out = (struct tpm2_get_cap_out *)&buf->data[TPM_HEADER_SIZE];
/*
* To prevent failing boot up of some systems, Infineon TPM2.0
* returns SUCCESS on TPM2_Startup in field upgrade mode. Also
@@ -383,7 +375,6 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value,
else
rc = -ENODATA;
}
- tpm_buf_destroy(&buf);
return rc;
}
EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
@@ -400,15 +391,14 @@ EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
*/
void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
{
- struct tpm_buf buf;
- int rc;
-
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
- if (rc)
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
return;
- tpm_buf_append_u16(&buf, shutdown_type);
- tpm_transmit_cmd(chip, &buf, 0, "stopping the TPM");
- tpm_buf_destroy(&buf);
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
+ tpm_buf_append_u16(buf, shutdown_type);
+ tpm_transmit_cmd(chip, buf, 0, "TPM2_Shutdown");
}
/**
@@ -426,20 +416,19 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
*/
static int tpm2_do_selftest(struct tpm_chip *chip)
{
- struct tpm_buf buf;
int full;
int rc;
- for (full = 0; full < 2; full++) {
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST);
- if (rc)
- return rc;
-
- tpm_buf_append_u8(&buf, full);
- rc = tpm_transmit_cmd(chip, &buf, 0,
- "attempting the self test");
- tpm_buf_destroy(&buf);
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST);
+ for (full = 0; full < 2; full++) {
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST);
+ tpm_buf_append_u8(buf, full);
+ rc = tpm_transmit_cmd(chip, buf, 0, "TPM2_SelfTest");
if (rc == TPM2_RC_TESTING)
rc = TPM2_RC_SUCCESS;
if (rc == TPM2_RC_INITIALIZE || rc == TPM2_RC_SUCCESS)
@@ -464,23 +453,24 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
int tpm2_probe(struct tpm_chip *chip)
{
struct tpm_header *out;
- struct tpm_buf buf;
int rc;
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
- if (rc)
- return rc;
- tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
- tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
- tpm_buf_append_u32(&buf, 1);
- rc = tpm_transmit_cmd(chip, &buf, 0, NULL);
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+ tpm_buf_append_u32(buf, TPM2_CAP_TPM_PROPERTIES);
+ tpm_buf_append_u32(buf, TPM_PT_TOTAL_COMMANDS);
+ tpm_buf_append_u32(buf, 1);
+ rc = tpm_transmit_cmd(chip, buf, 0, NULL);
/* We ignore TPM return codes on purpose. */
if (rc >= 0) {
- out = (struct tpm_header *)buf.data;
+ out = (struct tpm_header *)buf->data;
if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
chip->flags |= TPM_CHIP_FLAG_TPM2;
}
- tpm_buf_destroy(&buf);
return 0;
}
EXPORT_SYMBOL_GPL(tpm2_probe);
@@ -520,7 +510,6 @@ struct tpm2_pcr_selection {
ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
{
struct tpm2_pcr_selection pcr_selection;
- struct tpm_buf buf;
void *marker;
void *end;
void *pcr_select_offset;
@@ -532,39 +521,37 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
int rc;
int i = 0;
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- tpm_buf_append_u32(&buf, TPM2_CAP_PCRS);
- tpm_buf_append_u32(&buf, 0);
- tpm_buf_append_u32(&buf, 1);
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+ tpm_buf_append_u32(buf, TPM2_CAP_PCRS);
+ tpm_buf_append_u32(buf, 0);
+ tpm_buf_append_u32(buf, 1);
- rc = tpm_transmit_cmd(chip, &buf, 9, "get tpm pcr allocation");
+ rc = tpm_transmit_cmd(chip, buf, 9, "TPM2_GetCapability(PCRS)");
if (rc)
- goto out;
+ return rc;
- nr_possible_banks = be32_to_cpup(
- (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
+ nr_possible_banks = be32_to_cpup((__be32 *)&buf->data[TPM_HEADER_SIZE + 5]);
if (nr_possible_banks > TPM2_MAX_PCR_BANKS) {
pr_err("tpm: unexpected number of banks: %u > %u",
nr_possible_banks, TPM2_MAX_PCR_BANKS);
- rc = -ENOMEM;
- goto out;
+ return -ENOMEM;
}
- marker = &buf.data[TPM_HEADER_SIZE + 9];
+ marker = &buf->data[TPM_HEADER_SIZE + 9];
- rsp_len = be32_to_cpup((__be32 *)&buf.data[2]);
- end = &buf.data[rsp_len];
+ rsp_len = be32_to_cpup((__be32 *)&buf->data[2]);
+ end = &buf->data[rsp_len];
for (i = 0; i < nr_possible_banks; i++) {
pcr_select_offset = marker +
offsetof(struct tpm2_pcr_selection, size_of_select);
- if (pcr_select_offset >= end) {
- rc = -EFAULT;
- break;
- }
+ if (pcr_select_offset >= end)
+ return -EFAULT;
memcpy(&pcr_selection, marker, sizeof(pcr_selection));
hash_alg = be16_to_cpu(pcr_selection.hash_alg);
@@ -576,7 +563,7 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
rc = tpm2_init_bank_info(chip, nr_alloc_banks);
if (rc < 0)
- break;
+ return rc;
nr_alloc_banks++;
}
@@ -588,21 +575,21 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
}
chip->nr_allocated_banks = nr_alloc_banks;
-out:
- tpm_buf_destroy(&buf);
-
- return rc;
+ return 0;
}
int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
{
- struct tpm_buf buf;
u32 nr_commands;
__be32 *attrs;
u32 cc;
int i;
int rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
rc = tpm2_get_tpm_pt(chip, TPM_PT_TOTAL_COMMANDS, &nr_commands, NULL);
if (rc)
goto out;
@@ -619,30 +606,24 @@ int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
goto out;
}
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
- if (rc)
- goto out;
-
- tpm_buf_append_u32(&buf, TPM2_CAP_COMMANDS);
- tpm_buf_append_u32(&buf, TPM2_CC_FIRST);
- tpm_buf_append_u32(&buf, nr_commands);
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+ tpm_buf_append_u32(buf, TPM2_CAP_COMMANDS);
+ tpm_buf_append_u32(buf, TPM2_CC_FIRST);
+ tpm_buf_append_u32(buf, nr_commands);
- rc = tpm_transmit_cmd(chip, &buf, 9 + 4 * nr_commands, NULL);
- if (rc) {
- tpm_buf_destroy(&buf);
+ rc = tpm_transmit_cmd(chip, buf, 9 + 4 * nr_commands, NULL);
+ if (rc)
goto out;
- }
- if (nr_commands !=
- be32_to_cpup((__be32 *)&buf.data[TPM_HEADER_SIZE + 5])) {
+ if (nr_commands != be32_to_cpup((__be32 *)&buf->data[TPM_HEADER_SIZE + 5])) {
rc = -EFAULT;
- tpm_buf_destroy(&buf);
goto out;
}
chip->nr_commands = nr_commands;
- attrs = (__be32 *)&buf.data[TPM_HEADER_SIZE + 9];
+ attrs = (__be32 *)&buf->data[TPM_HEADER_SIZE + 9];
for (i = 0; i < nr_commands; i++, attrs++) {
chip->cc_attrs_tbl[i] = be32_to_cpup(attrs);
cc = chip->cc_attrs_tbl[i] & 0xFFFF;
@@ -654,8 +635,6 @@ int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
}
}
- tpm_buf_destroy(&buf);
-
out:
if (rc > 0)
rc = -ENODEV;
@@ -676,20 +655,14 @@ EXPORT_SYMBOL_GPL(tpm2_get_cc_attrs_tbl);
static int tpm2_startup(struct tpm_chip *chip)
{
- struct tpm_buf buf;
- int rc;
-
- dev_info(&chip->dev, "starting up the TPM manually\n");
-
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_STARTUP);
- if (rc < 0)
- return rc;
-
- tpm_buf_append_u16(&buf, TPM2_SU_CLEAR);
- rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to start the TPM");
- tpm_buf_destroy(&buf);
-
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_STARTUP);
+ tpm_buf_append_u16(buf, TPM2_SU_CLEAR);
+ return tpm_transmit_cmd(chip, buf, 0, "TPM2_Startup");
}
/**
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index bbc05f0997a8..378b8d21b10c 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -182,19 +182,18 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf)
static int tpm2_read_public(struct tpm_chip *chip, u32 handle, char *name)
{
- struct tpm_buf buf;
int rc;
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- tpm_buf_append_u32(&buf, handle);
- rc = tpm_transmit_cmd(chip, &buf, 0, "read public");
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC);
+ tpm_buf_append_u32(buf, handle);
+ rc = tpm_transmit_cmd(chip, buf, 0, "TPM2_ReadPublic");
if (rc == TPM2_RC_SUCCESS)
- rc = tpm2_parse_read_public(name, &buf);
-
- tpm_buf_destroy(&buf);
+ rc = tpm2_parse_read_public(name, buf);
return rc;
}
@@ -924,7 +923,6 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)
int tpm2_start_auth_session(struct tpm_chip *chip)
{
struct tpm2_auth *auth;
- struct tpm_buf buf;
u32 null_key;
int rc;
@@ -933,6 +931,10 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
return 0;
}
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
auth = kzalloc(sizeof(*auth), GFP_KERNEL);
if (!auth)
return -ENOMEM;
@@ -943,41 +945,37 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
auth->session = TPM_HEADER_SIZE;
- rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
- if (rc)
- goto out;
-
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS);
/* salt key handle */
- tpm_buf_append_u32(&buf, null_key);
+ tpm_buf_append_u32(buf, null_key);
/* bind key handle */
- tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+ tpm_buf_append_u32(buf, TPM2_RH_NULL);
/* nonce caller */
get_random_bytes(auth->our_nonce, sizeof(auth->our_nonce));
- tpm_buf_append_u16(&buf, sizeof(auth->our_nonce));
- tpm_buf_append(&buf, auth->our_nonce, sizeof(auth->our_nonce));
+ tpm_buf_append_u16(buf, sizeof(auth->our_nonce));
+ tpm_buf_append(buf, auth->our_nonce, sizeof(auth->our_nonce));
/* append encrypted salt and squirrel away unencrypted in auth */
- tpm_buf_append_salt(&buf, chip, auth);
+ tpm_buf_append_salt(buf, chip, auth);
/* session type (HMAC, audit or policy) */
- tpm_buf_append_u8(&buf, TPM2_SE_HMAC);
+ tpm_buf_append_u8(buf, TPM2_SE_HMAC);
/* symmetric encryption parameters */
/* symmetric algorithm */
- tpm_buf_append_u16(&buf, TPM_ALG_AES);
+ tpm_buf_append_u16(buf, TPM_ALG_AES);
/* bits for symmetric algorithm */
- tpm_buf_append_u16(&buf, AES_KEY_BITS);
+ tpm_buf_append_u16(buf, AES_KEY_BITS);
/* symmetric algorithm mode (must be CFB) */
- tpm_buf_append_u16(&buf, TPM_ALG_CFB);
+ tpm_buf_append_u16(buf, TPM_ALG_CFB);
/* hash algorithm for session */
- tpm_buf_append_u16(&buf, TPM_ALG_SHA256);
+ tpm_buf_append_u16(buf, TPM_ALG_SHA256);
- rc = tpm_ret_to_err(tpm_transmit_cmd(chip, &buf, 0, "StartAuthSession"));
+ rc = tpm_ret_to_err(tpm_transmit_cmd(chip, buf, 0, "TPM2_StartAuthSession"));
tpm2_flush_context(chip, null_key);
if (rc == TPM2_RC_SUCCESS)
- rc = tpm2_parse_start_auth_session(auth, &buf);
-
- tpm_buf_destroy(&buf);
+ rc = tpm2_parse_start_auth_session(auth, buf);
if (rc == TPM2_RC_SUCCESS) {
chip->auth = auth;
@@ -1199,18 +1197,18 @@ static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy,
u32 *handle, u8 *name)
{
int rc;
- struct tpm_buf buf;
- struct tpm_buf template;
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE_PRIMARY);
- if (rc)
- return rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- rc = tpm_buf_init_sized(&template);
- if (rc) {
- tpm_buf_destroy(&buf);
- return rc;
- }
+ struct tpm_buf *template __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!template)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE_PRIMARY);
+ tpm_buf_init_sized(template, TPM_BUFSIZE);
/*
* create the template. Note: in order for userspace to
@@ -1222,75 +1220,71 @@ static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy,
*/
/* key type */
- tpm_buf_append_u16(&template, TPM_ALG_ECC);
+ tpm_buf_append_u16(template, TPM_ALG_ECC);
/* name algorithm */
- tpm_buf_append_u16(&template, TPM_ALG_SHA256);
+ tpm_buf_append_u16(template, TPM_ALG_SHA256);
/* object properties */
- tpm_buf_append_u32(&template, TPM2_OA_NULL_KEY);
+ tpm_buf_append_u32(template, TPM2_OA_NULL_KEY);
/* sauth policy (empty) */
- tpm_buf_append_u16(&template, 0);
+ tpm_buf_append_u16(template, 0);
/* BEGIN parameters: key specific; for ECC*/
/* symmetric algorithm */
- tpm_buf_append_u16(&template, TPM_ALG_AES);
+ tpm_buf_append_u16(template, TPM_ALG_AES);
/* bits for symmetric algorithm */
- tpm_buf_append_u16(&template, AES_KEY_BITS);
+ tpm_buf_append_u16(template, AES_KEY_BITS);
/* algorithm mode (must be CFB) */
- tpm_buf_append_u16(&template, TPM_ALG_CFB);
+ tpm_buf_append_u16(template, TPM_ALG_CFB);
/* scheme (NULL means any scheme) */
- tpm_buf_append_u16(&template, TPM_ALG_NULL);
+ tpm_buf_append_u16(template, TPM_ALG_NULL);
/* ECC Curve ID */
- tpm_buf_append_u16(&template, TPM2_ECC_NIST_P256);
+ tpm_buf_append_u16(template, TPM2_ECC_NIST_P256);
/* KDF Scheme */
- tpm_buf_append_u16(&template, TPM_ALG_NULL);
+ tpm_buf_append_u16(template, TPM_ALG_NULL);
/* unique: key specific; for ECC it is two zero size points */
- tpm_buf_append_u16(&template, 0);
- tpm_buf_append_u16(&template, 0);
+ tpm_buf_append_u16(template, 0);
+ tpm_buf_append_u16(template, 0);
/* END parameters */
/* primary handle */
- tpm_buf_append_u32(&buf, hierarchy);
- tpm_buf_append_empty_auth(&buf, TPM2_RS_PW);
+ tpm_buf_append_u32(buf, hierarchy);
+ tpm_buf_append_empty_auth(buf, TPM2_RS_PW);
/* sensitive create size is 4 for two empty buffers */
- tpm_buf_append_u16(&buf, 4);
+ tpm_buf_append_u16(buf, 4);
/* sensitive create auth data (empty) */
- tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u16(buf, 0);
/* sensitive create sensitive data (empty) */
- tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u16(buf, 0);
/* the public template */
- tpm_buf_append(&buf, template.data, template.length);
- tpm_buf_destroy(&template);
+ tpm_buf_append(buf, template->data, template->length);
/* outside info (empty) */
- tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u16(buf, 0);
/* creation PCR (none) */
- tpm_buf_append_u32(&buf, 0);
+ tpm_buf_append_u32(buf, 0);
- rc = tpm_transmit_cmd(chip, &buf, 0,
- "attempting to create NULL primary");
+ rc = tpm_transmit_cmd(chip, buf, 0, "TPM2_CreatePrimary");
if (rc == TPM2_RC_SUCCESS)
- rc = tpm2_parse_create_primary(chip, &buf, handle, hierarchy,
+ rc = tpm2_parse_create_primary(chip, buf, handle, hierarchy,
name);
- tpm_buf_destroy(&buf);
-
return rc;
}
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 60354cd53b5c..cbf86ff5931f 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -71,24 +71,25 @@ void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space)
int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
unsigned int *offset, u32 *handle)
{
- struct tpm_buf tbuf;
struct tpm2_context *ctx;
unsigned int body_size;
int rc;
- rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_LOAD);
- if (rc)
- return rc;
+ struct tpm_buf *tbuf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!tbuf)
+ return -ENOMEM;
+
+ tpm_buf_init(tbuf, TPM_BUFSIZE);
+ tpm_buf_reset(tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_LOAD);
ctx = (struct tpm2_context *)&buf[*offset];
body_size = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
- tpm_buf_append(&tbuf, &buf[*offset], body_size);
+ tpm_buf_append(tbuf, &buf[*offset], body_size);
- rc = tpm_transmit_cmd(chip, &tbuf, 4, NULL);
+ rc = tpm_transmit_cmd(chip, tbuf, 4, NULL);
if (rc < 0) {
dev_warn(&chip->dev, "%s: failed with a system error %d\n",
__func__, rc);
- tpm_buf_destroy(&tbuf);
return -EFAULT;
} else if (tpm2_rc_value(rc) == TPM2_RC_HANDLE ||
rc == TPM2_RC_REFERENCE_H0) {
@@ -103,64 +104,55 @@ int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
* flushed outside the space
*/
*handle = 0;
- tpm_buf_destroy(&tbuf);
return -ENOENT;
} else if (tpm2_rc_value(rc) == TPM2_RC_INTEGRITY) {
- tpm_buf_destroy(&tbuf);
return -EINVAL;
} else if (rc > 0) {
dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
__func__, rc);
- tpm_buf_destroy(&tbuf);
return -EFAULT;
}
- *handle = be32_to_cpup((__be32 *)&tbuf.data[TPM_HEADER_SIZE]);
+ *handle = be32_to_cpup((__be32 *)&tbuf->data[TPM_HEADER_SIZE]);
*offset += body_size;
-
- tpm_buf_destroy(&tbuf);
return 0;
}
int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
unsigned int buf_size, unsigned int *offset)
{
- struct tpm_buf tbuf;
unsigned int body_size;
int rc;
- rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_SAVE);
- if (rc)
- return rc;
+ struct tpm_buf *tbuf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!tbuf)
+ return -ENOMEM;
- tpm_buf_append_u32(&tbuf, handle);
+ tpm_buf_init(tbuf, TPM_BUFSIZE);
+ tpm_buf_reset(tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_SAVE);
+ tpm_buf_append_u32(tbuf, handle);
- rc = tpm_transmit_cmd(chip, &tbuf, 0, NULL);
+ rc = tpm_transmit_cmd(chip, tbuf, 0, NULL);
if (rc < 0) {
dev_warn(&chip->dev, "%s: failed with a system error %d\n",
__func__, rc);
- tpm_buf_destroy(&tbuf);
return -EFAULT;
} else if (tpm2_rc_value(rc) == TPM2_RC_REFERENCE_H0) {
- tpm_buf_destroy(&tbuf);
return -ENOENT;
} else if (rc) {
dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
__func__, rc);
- tpm_buf_destroy(&tbuf);
return -EFAULT;
}
- body_size = tpm_buf_length(&tbuf) - TPM_HEADER_SIZE;
+ body_size = tpm_buf_length(tbuf) - TPM_HEADER_SIZE;
if ((*offset + body_size) > buf_size) {
dev_warn(&chip->dev, "%s: out of backing storage\n", __func__);
- tpm_buf_destroy(&tbuf);
return -ENOMEM;
}
- memcpy(&buf[*offset], &tbuf.data[TPM_HEADER_SIZE], body_size);
+ memcpy(&buf[*offset], &tbuf->data[TPM_HEADER_SIZE], body_size);
*offset += body_size;
- tpm_buf_destroy(&tbuf);
return 0;
}
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 0818bb517805..682dfc93845d 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -395,40 +395,36 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip *chip, u8 status)
static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
{
- struct tpm_buf buf;
int rc;
const struct tpm_header *header;
struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUFSIZE);
if (chip->flags & TPM_CHIP_FLAG_TPM2)
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS,
- TPM2_CC_SET_LOCALITY);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_SET_LOCALITY);
else
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND,
- TPM_ORD_SET_LOCALITY);
- if (rc)
- return rc;
- tpm_buf_append_u8(&buf, locality);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SET_LOCALITY);
+
+ tpm_buf_append_u8(buf, locality);
proxy_dev->state |= STATE_DRIVER_COMMAND;
- rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to set locality");
+ rc = tpm_transmit_cmd(chip, buf, 0, "attempting to set locality");
proxy_dev->state &= ~STATE_DRIVER_COMMAND;
- if (rc < 0) {
- locality = rc;
- goto out;
- }
+ if (rc < 0)
+ return rc;
- header = (const struct tpm_header *)buf.data;
+ header = (const struct tpm_header *)buf->data;
rc = be32_to_cpu(header->return_code);
if (rc)
locality = -1;
-out:
- tpm_buf_destroy(&buf);
-
return locality;
}
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 6ff76a4b8a53..a453271933cf 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -25,7 +25,8 @@
#include <crypto/hash_info.h>
#include <crypto/aes.h>
-#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
+#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */
+#define TPM_BUFSIZE 4096
#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
#define TPM2_MAX_PCR_BANKS 8
@@ -376,13 +377,15 @@ enum tpm_buf_flags {
};
/*
- * A string buffer type for constructing TPM commands.
+ * A buffer for constructing and parsing TPM commands, responses and sized
+ * (TPM2B) buffers.
*/
struct tpm_buf {
- u32 flags;
- u32 length;
- u8 *data;
+ u8 flags;
u8 handles;
+ u16 length;
+ u16 capacity;
+ u8 data[];
};
enum tpm2_object_attributes {
@@ -413,11 +416,10 @@ struct tpm2_hash {
unsigned int tpm_id;
};
-int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal);
+void tpm_buf_init(struct tpm_buf *buf, u16 buf_size);
+void tpm_buf_init_sized(struct tpm_buf *buf, u16 buf_size);
void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal);
-int tpm_buf_init_sized(struct tpm_buf *buf);
void tpm_buf_reset_sized(struct tpm_buf *buf);
-void tpm_buf_destroy(struct tpm_buf *buf);
u32 tpm_buf_length(struct tpm_buf *buf);
void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length);
void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value);
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 636acb66a4f6..3ac204a902de 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -310,9 +310,8 @@ static int TSS_checkhmac2(unsigned char *buffer,
* For key specific tpm requests, we will generate and send our
* own TPM command packets using the drivers send function.
*/
-static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
+static int trusted_tpm_send(void *cmd, size_t buflen)
{
- struct tpm_buf buf;
int rc;
if (!chip)
@@ -322,15 +321,12 @@ static int trusted_tpm_send(unsigned char *cmd, size_t buflen)
if (rc)
return rc;
- buf.flags = 0;
- buf.length = buflen;
- buf.data = cmd;
dump_tpm_buf(cmd);
- rc = tpm_transmit_cmd(chip, &buf, 4, "sending data");
+ rc = tpm_transmit_cmd(chip, cmd, 4, "sending data");
dump_tpm_buf(cmd);
+ /* Convert TPM error to -EPERM. */
if (rc > 0)
- /* TPM error */
rc = -EPERM;
tpm_put_ops(chip);
@@ -624,23 +620,23 @@ static int tpm_unseal(struct tpm_buf *tb,
static int key_seal(struct trusted_key_payload *p,
struct trusted_key_options *o)
{
- struct tpm_buf tb;
int ret;
- ret = tpm_buf_init(&tb, 0, 0);
- if (ret)
- return ret;
+ struct tpm_buf *tb __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!tb)
+ return -ENOMEM;
+
+ tpm_buf_init(tb, TPM_BUFSIZE);
/* include migratable flag at end of sealed key */
p->key[p->key_len] = p->migratable;
- ret = tpm_seal(&tb, o->keytype, o->keyhandle, o->keyauth,
+ ret = tpm_seal(tb, o->keytype, o->keyhandle, o->keyauth,
p->key, p->key_len + 1, p->blob, &p->blob_len,
o->blobauth, o->pcrinfo, o->pcrinfo_len);
if (ret < 0)
pr_info("srkseal failed (%d)\n", ret);
- tpm_buf_destroy(&tb);
return ret;
}
@@ -650,14 +646,15 @@ static int key_seal(struct trusted_key_payload *p,
static int key_unseal(struct trusted_key_payload *p,
struct trusted_key_options *o)
{
- struct tpm_buf tb;
int ret;
- ret = tpm_buf_init(&tb, 0, 0);
- if (ret)
- return ret;
+ struct tpm_buf *tb __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!tb)
+ return -ENOMEM;
+
+ tpm_buf_init(tb, TPM_BUFSIZE);
- ret = tpm_unseal(&tb, o->keyhandle, o->keyauth, p->blob, p->blob_len,
+ ret = tpm_unseal(tb, o->keyhandle, o->keyauth, p->blob, p->blob_len,
o->blobauth, p->key, &p->key_len);
if (ret < 0)
pr_info("srkunseal failed (%d)\n", ret);
@@ -665,7 +662,6 @@ static int key_unseal(struct trusted_key_payload *p,
/* pull migratable flag out of sealed key */
p->migratable = p->key[--p->key_len];
- tpm_buf_destroy(&tb);
return ret;
}
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 991f9af92eb1..79e2209fb369 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -210,13 +210,20 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
struct trusted_key_options *options)
{
off_t offset = TPM_HEADER_SIZE;
- struct tpm_buf buf, sized;
int blob_len = 0;
u32 hash;
u32 flags;
int i;
int rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ struct tpm_buf *sized __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!sized)
+ return -ENOMEM;
+
for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
if (options->hash == tpm2_hash_map[i].crypto_id) {
hash = tpm2_hash_map[i].tpm_id;
@@ -238,91 +245,79 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
if (rc)
goto out_put;
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
- if (rc) {
- tpm2_end_auth_session(chip);
- goto out_put;
- }
-
- rc = tpm_buf_init_sized(&sized);
- if (rc) {
- tpm_buf_destroy(&buf);
- tpm2_end_auth_session(chip);
- goto out_put;
- }
-
- tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
- tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT,
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_init_sized(sized, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
+ tpm_buf_append_name(chip, buf, options->keyhandle, NULL);
+ tpm_buf_append_hmac_session(chip, buf, TPM2_SA_DECRYPT,
options->keyauth, TPM_DIGEST_SIZE);
/* sensitive */
- tpm_buf_append_u16(&sized, options->blobauth_len);
+ tpm_buf_append_u16(sized, options->blobauth_len);
if (options->blobauth_len)
- tpm_buf_append(&sized, options->blobauth, options->blobauth_len);
+ tpm_buf_append(sized, options->blobauth, options->blobauth_len);
- tpm_buf_append_u16(&sized, payload->key_len);
- tpm_buf_append(&sized, payload->key, payload->key_len);
- tpm_buf_append(&buf, sized.data, sized.length);
+ tpm_buf_append_u16(sized, payload->key_len);
+ tpm_buf_append(sized, payload->key, payload->key_len);
+ tpm_buf_append(buf, sized->data, sized->length);
/* public */
- tpm_buf_reset_sized(&sized);
- tpm_buf_append_u16(&sized, TPM_ALG_KEYEDHASH);
- tpm_buf_append_u16(&sized, hash);
+ tpm_buf_init_sized(sized, TPM_BUFSIZE);
+ tpm_buf_append_u16(sized, TPM_ALG_KEYEDHASH);
+ tpm_buf_append_u16(sized, hash);
/* key properties */
flags = 0;
flags |= options->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH;
flags |= payload->migratable ? 0 : (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT);
- tpm_buf_append_u32(&sized, flags);
+ tpm_buf_append_u32(sized, flags);
/* policy */
- tpm_buf_append_u16(&sized, options->policydigest_len);
+ tpm_buf_append_u16(sized, options->policydigest_len);
if (options->policydigest_len)
- tpm_buf_append(&sized, options->policydigest, options->policydigest_len);
+ tpm_buf_append(sized, options->policydigest, options->policydigest_len);
/* public parameters */
- tpm_buf_append_u16(&sized, TPM_ALG_NULL);
- tpm_buf_append_u16(&sized, 0);
+ tpm_buf_append_u16(sized, TPM_ALG_NULL);
+ tpm_buf_append_u16(sized, 0);
- tpm_buf_append(&buf, sized.data, sized.length);
+ tpm_buf_append(buf, sized->data, sized->length);
/* outside info */
- tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u16(buf, 0);
/* creation PCR */
- tpm_buf_append_u32(&buf, 0);
+ tpm_buf_append_u32(buf, 0);
- if (buf.flags & TPM_BUF_INVALID) {
+ if (buf->flags & TPM_BUF_INVALID) {
rc = -E2BIG;
tpm2_end_auth_session(chip);
goto out;
}
- tpm_buf_fill_hmac_session(chip, &buf);
- rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
- rc = tpm_buf_check_hmac_response(chip, &buf, rc);
+ tpm_buf_fill_hmac_session(chip, buf);
+ rc = tpm_transmit_cmd(chip, buf, 4, "sealing data");
+ rc = tpm_buf_check_hmac_response(chip, buf, rc);
if (rc)
goto out;
- blob_len = tpm_buf_read_u32(&buf, &offset);
- if (blob_len > MAX_BLOB_SIZE || buf.flags & TPM_BUF_INVALID) {
+ blob_len = tpm_buf_read_u32(buf, &offset);
+ if (blob_len > MAX_BLOB_SIZE || buf->flags & TPM_BUF_INVALID) {
rc = -E2BIG;
goto out;
}
- if (buf.length - offset < blob_len) {
+ if (buf->length - offset < blob_len) {
rc = -EFAULT;
goto out;
}
- blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len);
+ blob_len = tpm2_key_encode(payload, options, &buf->data[offset],
+ blob_len);
if (blob_len < 0)
rc = blob_len;
out:
- tpm_buf_destroy(&sized);
- tpm_buf_destroy(&buf);
-
if (!rc)
payload->blob_len = blob_len;
@@ -349,7 +344,6 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
struct trusted_key_options *options,
u32 *blob_handle)
{
- struct tpm_buf buf;
unsigned int private_len;
unsigned int public_len;
unsigned int blob_len;
@@ -401,33 +395,29 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
if (rc)
return rc;
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
- if (rc) {
- tpm2_end_auth_session(chip);
- return rc;
- }
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
- tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
+ tpm_buf_append_name(chip, buf, options->keyhandle, NULL);
+ tpm_buf_append_hmac_session(chip, buf, 0, options->keyauth,
TPM_DIGEST_SIZE);
- tpm_buf_append(&buf, blob, blob_len);
+ tpm_buf_append(buf, blob, blob_len);
- if (buf.flags & TPM_BUF_INVALID) {
- rc = -E2BIG;
+ if (buf->flags & TPM_BUF_INVALID) {
tpm2_end_auth_session(chip);
- goto out;
+ return -E2BIG;
}
- tpm_buf_fill_hmac_session(chip, &buf);
- rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
- rc = tpm_buf_check_hmac_response(chip, &buf, rc);
+ tpm_buf_fill_hmac_session(chip, buf);
+ rc = tpm_transmit_cmd(chip, buf, 4, "loading blob");
+ rc = tpm_buf_check_hmac_response(chip, buf, rc);
if (!rc)
*blob_handle = be32_to_cpup(
- (__be32 *) &buf.data[TPM_HEADER_SIZE]);
-
-out:
- tpm_buf_destroy(&buf);
+ (__be32 *)&buf->data[TPM_HEADER_SIZE]);
return tpm_ret_to_err(rc);
}
@@ -450,26 +440,25 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
u32 blob_handle)
{
struct tpm_header *head;
- struct tpm_buf buf;
u16 data_len;
int offset;
u8 *data;
int rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
rc = tpm2_start_auth_session(chip);
if (rc)
return rc;
- rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
- if (rc) {
- tpm2_end_auth_session(chip);
- return rc;
- }
-
- tpm_buf_append_name(chip, &buf, blob_handle, NULL);
+ tpm_buf_init(buf, TPM_BUFSIZE);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
+ tpm_buf_append_name(chip, buf, blob_handle, NULL);
if (!options->policyhandle) {
- tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT,
+ tpm_buf_append_hmac_session(chip, buf, TPM2_SA_ENCRYPT,
options->blobauth,
options->blobauth_len);
} else {
@@ -478,39 +467,35 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
* the password will end up being unencrypted on the bus, as
* HMAC nonce cannot be calculated for it.
*/
- tpm_buf_append_u32(&buf, 9 + options->blobauth_len);
- tpm_buf_append_u32(&buf, options->policyhandle);
- tpm_buf_append_u16(&buf, 0);
- tpm_buf_append_u8(&buf, 0);
- tpm_buf_append_u16(&buf, options->blobauth_len);
- tpm_buf_append(&buf, options->blobauth, options->blobauth_len);
+ tpm_buf_append_u32(buf, 9 + options->blobauth_len);
+ tpm_buf_append_u32(buf, options->policyhandle);
+ tpm_buf_append_u16(buf, 0);
+ tpm_buf_append_u8(buf, 0);
+ tpm_buf_append_u16(buf, options->blobauth_len);
+ tpm_buf_append(buf, options->blobauth, options->blobauth_len);
if (tpm2_chip_auth(chip)) {
- tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
+ tpm_buf_append_hmac_session(chip, buf, TPM2_SA_ENCRYPT, NULL, 0);
} else {
- offset = buf.handles * 4 + TPM_HEADER_SIZE;
- head = (struct tpm_header *)buf.data;
- if (tpm_buf_length(&buf) == offset)
+ offset = buf->handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf->data;
+ if (tpm_buf_length(buf) == offset)
head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
}
}
- tpm_buf_fill_hmac_session(chip, &buf);
- rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
- rc = tpm_buf_check_hmac_response(chip, &buf, rc);
+ tpm_buf_fill_hmac_session(chip, buf);
+ rc = tpm_transmit_cmd(chip, buf, 6, "unsealing");
+ rc = tpm_buf_check_hmac_response(chip, buf, rc);
if (!rc) {
data_len = be16_to_cpup(
- (__be16 *) &buf.data[TPM_HEADER_SIZE + 4]);
- if (data_len < MIN_KEY_SIZE || data_len > MAX_KEY_SIZE) {
- rc = -EFAULT;
- goto out;
- }
+ (__be16 *)&buf->data[TPM_HEADER_SIZE + 4]);
+ if (data_len < MIN_KEY_SIZE || data_len > MAX_KEY_SIZE)
+ return -EFAULT;
- if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) {
- rc = -EFAULT;
- goto out;
- }
- data = &buf.data[TPM_HEADER_SIZE + 6];
+ if (tpm_buf_length(buf) < TPM_HEADER_SIZE + 6 + data_len)
+ return -EFAULT;
+ data = &buf->data[TPM_HEADER_SIZE + 6];
if (payload->old_format) {
/* migratable flag is at the end of the key */
@@ -527,8 +512,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
}
}
-out:
- tpm_buf_destroy(&buf);
return tpm_ret_to_err(rc);
}
--
2.39.5
^ permalink raw reply related
* [PATCH v6 08/10] tpm-buf: unify TPM_BUF_BOUNDARY_ERROR and TPM_BUF_OVERFLOW
From: Jarkko Sakkinen @ 2025-10-18 11:17 UTC (permalink / raw)
To: linux-integrity
Cc: keyring, dpsmith, ross.philipson, Jonathan McDowell,
Roberto Sassu, Stefano Garzarella, Jarkko Sakkinen,
Jonathan McDowell, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, linux-kernel, keyrings,
linux-security-module
In-Reply-To: <20251018111725.3116386-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Unify TPM_BUF_BOUNDARY_ERROR and TPM_BUF_OVERFLOW into TPM_BUF_INVALID,
given that they are identical. The only difference are the log messages.
In addition, add a missing TPM_BUF_INVALID check to tpm_buf_append_handle()
following the pattern from other functions in tpm-buf.c.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Reviewed-by: Jonathan McDowell <noodles@meta.com>
---
v6:
- Change short summary to something more descriptive.
v5:
- No changes.
v4:
- No changes.
v3:
- No changes.
v2:
- A new patch.
---
drivers/char/tpm/tpm-buf.c | 14 ++++++++------
include/linux/tpm.h | 8 +++-----
security/keys/trusted-keys/trusted_tpm2.c | 6 +++---
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
index dc882fc9fa9e..69ee77400539 100644
--- a/drivers/char/tpm/tpm-buf.c
+++ b/drivers/char/tpm/tpm-buf.c
@@ -104,13 +104,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_length);
*/
void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length)
{
- /* Return silently if overflow has already happened. */
- if (buf->flags & TPM_BUF_OVERFLOW)
+ if (buf->flags & TPM_BUF_INVALID)
return;
if ((buf->length + new_length) > PAGE_SIZE) {
WARN(1, "tpm_buf: write overflow\n");
- buf->flags |= TPM_BUF_OVERFLOW;
+ buf->flags |= TPM_BUF_INVALID;
return;
}
@@ -157,8 +156,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_append_u32);
*/
void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle)
{
+ if (buf->flags & TPM_BUF_INVALID)
+ return;
+
if (buf->flags & TPM_BUF_TPM2B) {
dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n");
+ buf->flags |= TPM_BUF_INVALID;
return;
}
@@ -177,14 +180,13 @@ static void tpm_buf_read(struct tpm_buf *buf, off_t *offset, size_t count, void
{
off_t next_offset;
- /* Return silently if overflow has already happened. */
- if (buf->flags & TPM_BUF_BOUNDARY_ERROR)
+ if (buf->flags & TPM_BUF_INVALID)
return;
next_offset = *offset + count;
if (next_offset > buf->length) {
WARN(1, "tpm_buf: read out of boundary\n");
- buf->flags |= TPM_BUF_BOUNDARY_ERROR;
+ buf->flags |= TPM_BUF_INVALID;
return;
}
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 973458a38250..3eae22a06558 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -369,12 +369,10 @@ struct tpm_header {
} __packed;
enum tpm_buf_flags {
- /* the capacity exceeded: */
- TPM_BUF_OVERFLOW = BIT(0),
/* TPM2B format: */
- TPM_BUF_TPM2B = BIT(1),
- /* read out of boundary: */
- TPM_BUF_BOUNDARY_ERROR = BIT(2),
+ TPM_BUF_TPM2B = BIT(0),
+ /* The buffer is in invalid and unusable state: */
+ TPM_BUF_INVALID = BIT(1),
};
/*
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 74eb1223c3da..991f9af92eb1 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -293,7 +293,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
/* creation PCR */
tpm_buf_append_u32(&buf, 0);
- if (buf.flags & TPM_BUF_OVERFLOW) {
+ if (buf.flags & TPM_BUF_INVALID) {
rc = -E2BIG;
tpm2_end_auth_session(chip);
goto out;
@@ -306,7 +306,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
goto out;
blob_len = tpm_buf_read_u32(&buf, &offset);
- if (blob_len > MAX_BLOB_SIZE || buf.flags & TPM_BUF_BOUNDARY_ERROR) {
+ if (blob_len > MAX_BLOB_SIZE || buf.flags & TPM_BUF_INVALID) {
rc = -E2BIG;
goto out;
}
@@ -413,7 +413,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
tpm_buf_append(&buf, blob, blob_len);
- if (buf.flags & TPM_BUF_OVERFLOW) {
+ if (buf.flags & TPM_BUF_INVALID) {
rc = -E2BIG;
tpm2_end_auth_session(chip);
goto out;
--
2.39.5
^ permalink raw reply related
* [PATCH v6 07/10] KEYS: trusted: Open code tpm2_buf_append()
From: Jarkko Sakkinen @ 2025-10-18 11:17 UTC (permalink / raw)
To: linux-integrity
Cc: keyring, dpsmith, ross.philipson, Jonathan McDowell,
Roberto Sassu, Stefano Garzarella, Jarkko Sakkinen,
James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <20251018111725.3116386-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
tpm2_buf_append_auth() has only single call site and most of its parameters
are redundant. Open code it to the call site. Remove illegit FIXME comment
as there is no categorized bug and replace it with more sane comment about
implementation (i.e. "non-opionated inline comment").
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Reviewed-by: Jonathan McDowell <noodles@earth.li>
---
v6:
- No changes.
v5:
- No changes.
v4:
- No changes.
v3:
- No changes.
v2:
- No changes.
---
security/keys/trusted-keys/trusted_tpm2.c | 51 ++++-------------------
1 file changed, 9 insertions(+), 42 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index a8cc18d6e9f0..74eb1223c3da 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -196,36 +196,6 @@ int tpm2_key_priv(void *context, size_t hdrlen,
return 0;
}
-/**
- * tpm2_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
- *
- * @buf: an allocated tpm_buf instance
- * @session_handle: session handle
- * @nonce: the session nonce, may be NULL if not used
- * @nonce_len: the session nonce length, may be 0 if not used
- * @attributes: the session attributes
- * @hmac: the session HMAC or password, may be NULL if not used
- * @hmac_len: the session HMAC or password length, maybe 0 if not used
- */
-static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
- const u8 *nonce, u16 nonce_len,
- u8 attributes,
- const u8 *hmac, u16 hmac_len)
-{
- tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len);
- tpm_buf_append_u32(buf, session_handle);
- tpm_buf_append_u16(buf, nonce_len);
-
- if (nonce && nonce_len)
- tpm_buf_append(buf, nonce, nonce_len);
-
- tpm_buf_append_u8(buf, attributes);
- tpm_buf_append_u16(buf, hmac_len);
-
- if (hmac && hmac_len)
- tpm_buf_append(buf, hmac, hmac_len);
-}
-
/**
* tpm2_seal_trusted() - seal the payload of a trusted key
*
@@ -504,19 +474,16 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
options->blobauth_len);
} else {
/*
- * FIXME: The policy session was generated outside the
- * kernel so we don't known the nonce and thus can't
- * calculate a HMAC on it. Therefore, the user can
- * only really use TPM2_PolicyPassword and we must
- * send down the plain text password, which could be
- * intercepted. We can still encrypt the returned
- * key, but that's small comfort since the interposer
- * could repeat our actions with the exfiltrated
- * password.
+ * The policy session is generated outside the kernel, and thus
+ * the password will end up being unencrypted on the bus, as
+ * HMAC nonce cannot be calculated for it.
*/
- tpm2_buf_append_auth(&buf, options->policyhandle,
- NULL /* nonce */, 0, 0,
- options->blobauth, options->blobauth_len);
+ tpm_buf_append_u32(&buf, 9 + options->blobauth_len);
+ tpm_buf_append_u32(&buf, options->policyhandle);
+ tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u8(&buf, 0);
+ tpm_buf_append_u16(&buf, options->blobauth_len);
+ tpm_buf_append(&buf, options->blobauth, options->blobauth_len);
if (tpm2_chip_auth(chip)) {
tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
} else {
--
2.39.5
^ permalink raw reply related
* [PATCH v6 06/10] tpm2-sessions: Unmask tpm_buf_append_hmac_session()
From: Jarkko Sakkinen @ 2025-10-18 11:17 UTC (permalink / raw)
To: linux-integrity
Cc: keyring, dpsmith, ross.philipson, Jonathan McDowell,
Roberto Sassu, Stefano Garzarella, Jarkko Sakkinen,
Jonathan McDowell, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, linux-kernel, keyrings,
linux-security-module
In-Reply-To: <20251018111725.3116386-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Open code tpm_buf_append_hmac_session_opt() in order to unmask the code
paths in the call sites of tpm_buf_append_hmac_session().
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Reviewed-by: Jonathan McDowell <noodles@meta.com>
---
v6:
- No changes.
v5:
- No changes.
v4:
- Fixed typo in short summary.
v3:
- No changes.
v2:
- Uncorrupt the patch.
---
drivers/char/tpm/tpm2-cmd.c | 14 +++++++++++---
include/linux/tpm.h | 23 -----------------------
security/keys/trusted-keys/trusted_tpm2.c | 12 ++++++++++--
3 files changed, 21 insertions(+), 28 deletions(-)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index bf8294e3d8f2..254003e5dd8b 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -257,9 +257,17 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
do {
tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT
- | TPM2_SA_CONTINUE_SESSION,
- NULL, 0);
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf,
+ TPM2_SA_ENCRYPT |
+ TPM2_SA_CONTINUE_SESSION,
+ NULL, 0);
+ } else {
+ offset = buf.handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf.data;
+ if (tpm_buf_length(&buf) == offset)
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
tpm_buf_append_u16(&buf, num_bytes);
tpm_buf_fill_hmac_session(chip, &buf);
err = tpm_transmit_cmd(chip, &buf,
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 004680757923..973458a38250 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -535,29 +535,6 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
int passphraselen);
void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf,
u8 *passphrase, int passphraselen);
-static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip,
- struct tpm_buf *buf,
- u8 attributes,
- u8 *passphrase,
- int passphraselen)
-{
- struct tpm_header *head;
- int offset;
-
- if (tpm2_chip_auth(chip)) {
- tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen);
- } else {
- offset = buf->handles * 4 + TPM_HEADER_SIZE;
- head = (struct tpm_header *)buf->data;
-
- /*
- * If the only sessions are optional, the command tag must change to
- * TPM2_ST_NO_SESSIONS.
- */
- if (tpm_buf_length(buf) == offset)
- head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
- }
-}
#ifdef CONFIG_TCG_TPM2_HMAC
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 1cd00aad033a..a8cc18d6e9f0 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -479,8 +479,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
struct trusted_key_options *options,
u32 blob_handle)
{
+ struct tpm_header *head;
struct tpm_buf buf;
u16 data_len;
+ int offset;
u8 *data;
int rc;
@@ -515,8 +517,14 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
tpm2_buf_append_auth(&buf, options->policyhandle,
NULL /* nonce */, 0, 0,
options->blobauth, options->blobauth_len);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT,
- NULL, 0);
+ if (tpm2_chip_auth(chip)) {
+ tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0);
+ } else {
+ offset = buf.handles * 4 + TPM_HEADER_SIZE;
+ head = (struct tpm_header *)buf.data;
+ if (tpm_buf_length(&buf) == offset)
+ head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+ }
}
tpm_buf_fill_hmac_session(chip, &buf);
--
2.39.5
^ permalink raw reply related
* [PATCH v6 04/10] KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2
From: Jarkko Sakkinen @ 2025-10-18 11:17 UTC (permalink / raw)
To: linux-integrity
Cc: keyring, dpsmith, ross.philipson, Jonathan McDowell,
Roberto Sassu, Stefano Garzarella, Jarkko Sakkinen,
James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, keyrings,
linux-security-module, linux-kernel
In-Reply-To: <20251018111725.3116386-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Use tpm_ret_to_err() to transmute TPM return codes in trusted_tpm2.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Acked-by: Stefano Garzarella <sgarzare@redhat.com>
---
v6:
- No changes.
v5:
- No changes.
v4:
- No changes.
v3:
- No changes.
v2:
- New patch split out from the fix.
---
security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++-----------------
1 file changed, 7 insertions(+), 19 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 468a4a339541..1cd00aad033a 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -346,25 +346,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
}
blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len);
+ if (blob_len < 0)
+ rc = blob_len;
out:
tpm_buf_destroy(&sized);
tpm_buf_destroy(&buf);
- if (rc > 0) {
- if (tpm2_rc_value(rc) == TPM2_RC_HASH)
- rc = -EINVAL;
- else
- rc = -EPERM;
- }
- if (blob_len < 0)
- rc = blob_len;
- else
+ if (!rc)
payload->blob_len = blob_len;
out_put:
tpm_put_ops(chip);
- return rc;
+ return tpm_ret_to_err(rc);
}
/**
@@ -465,10 +459,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
out:
tpm_buf_destroy(&buf);
- if (rc > 0)
- rc = -EPERM;
-
- return rc;
+ return tpm_ret_to_err(rc);
}
/**
@@ -531,8 +522,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
tpm_buf_fill_hmac_session(chip, &buf);
rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
rc = tpm_buf_check_hmac_response(chip, &buf, rc);
- if (rc > 0)
- rc = -EPERM;
if (!rc) {
data_len = be16_to_cpup(
@@ -565,7 +554,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
out:
tpm_buf_destroy(&buf);
- return rc;
+ return tpm_ret_to_err(rc);
}
/**
@@ -597,6 +586,5 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
out:
tpm_put_ops(chip);
-
- return rc;
+ return tpm_ret_to_err(rc);
}
--
2.39.5
^ permalink raw reply related
* [PATCH v6 03/10] KEYS: trusted: Fix memory leak in tpm2_load()
From: Jarkko Sakkinen @ 2025-10-18 11:17 UTC (permalink / raw)
To: linux-integrity
Cc: keyring, dpsmith, ross.philipson, Jonathan McDowell,
Roberto Sassu, Stefano Garzarella, Jarkko Sakkinen, stable,
James Bottomley, Mimi Zohar, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, keyrings, linux-security-module,
linux-kernel
In-Reply-To: <20251018111725.3116386-1-jarkko@kernel.org>
tpm2_load() allocates a blob indirectly via tpm2_key_decode() but it is
not freed in all failure paths. Address this with a scope-based cleanup
helper __free(). For legacy blobs, the implicit de-allocation is gets
disable by no_free_ptr().
Cc: stable@vger.kernel.org # v5.13+
Fixes: f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v6:
- A new patch in this version.
---
security/keys/trusted-keys/trusted_tpm2.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 024be262702f..468a4a339541 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -106,9 +106,8 @@ struct tpm2_key_context {
u32 priv_len;
};
-static int tpm2_key_decode(struct trusted_key_payload *payload,
- struct trusted_key_options *options,
- u8 **buf)
+static void *tpm2_key_decode(struct trusted_key_payload *payload,
+ struct trusted_key_options *options)
{
int ret;
struct tpm2_key_context ctx;
@@ -119,16 +118,15 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
ret = asn1_ber_decoder(&tpm2key_decoder, &ctx, payload->blob,
payload->blob_len);
if (ret < 0)
- return ret;
+ return ERR_PTR(ret);
if (ctx.priv_len + ctx.pub_len > MAX_BLOB_SIZE)
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
blob = kmalloc(ctx.priv_len + ctx.pub_len + 4, GFP_KERNEL);
if (!blob)
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
- *buf = blob;
options->keyhandle = ctx.parent;
memcpy(blob, ctx.priv, ctx.priv_len);
@@ -136,7 +134,7 @@ static int tpm2_key_decode(struct trusted_key_payload *payload,
memcpy(blob, ctx.pub, ctx.pub_len);
- return 0;
+ return blob;
}
int tpm2_key_parent(void *context, size_t hdrlen,
@@ -391,13 +389,14 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
unsigned int private_len;
unsigned int public_len;
unsigned int blob_len;
- u8 *blob, *pub;
+ u8 *pub;
int rc;
u32 attrs;
- rc = tpm2_key_decode(payload, options, &blob);
- if (rc) {
+ u8 *blob __free(kfree) = tpm2_key_decode(payload, options);
+ if (IS_ERR(blob)) {
/* old form */
+ no_free_ptr(blob);
blob = payload->blob;
payload->old_format = 1;
}
@@ -464,8 +463,6 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
(__be32 *) &buf.data[TPM_HEADER_SIZE]);
out:
- if (blob != payload->blob)
- kfree(blob);
tpm_buf_destroy(&buf);
if (rc > 0)
--
2.39.5
^ permalink raw reply related
* Re: [PATCH] ima: Fall back to default kernel module signature verification
From: Coiby Xu @ 2025-10-17 23:19 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, Dmitry Torokhov, Karel Srot, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris,
Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <27bb0c218084f51eba07f041d0fffea8971865b9.camel@linux.ibm.com>
On Fri, Oct 17, 2025 at 01:49:58PM -0400, Mimi Zohar wrote:
>On Fri, 2025-10-17 at 11:19 +0800, Coiby Xu wrote:
>> On Thu, Oct 16, 2025 at 10:31:35PM -0400, Mimi Zohar wrote:
>> > On Thu, 2025-10-16 at 11:46 +0800, Coiby Xu wrote:
>> > > On Tue, Sep 30, 2025 at 04:28:14PM -0400, Mimi Zohar wrote:
>> > > > On Tue, 2025-09-30 at 09:57 -0400, Mimi Zohar wrote:
>> > > > > On Sun, 2025-09-28 at 11:03 +0800, Coiby Xu wrote:
>> > > > > > Currently, for any IMA policy that requires appraisal for kernel modules
>> > > > > > e.g. ima_policy=secure_boot, PowerPC architecture specific policy,
>> > > > > > booting will fail because IMA will reject a kernel module which will
>> > > > > > be decompressed in the kernel space and then have its signature
>> > > > > > verified.
>> > > > > >
>> > > > > > This happens because when in-kernel module decompression
>> > > > > > (CONFIG_MODULE_DECOMPRESS) is enabled, kmod will use finit_module
>> > > > > > syscall instead of init_module to load a module. And IMA mandates IMA
>> > > > > > xattr verification for finit_module unless appraise_type=imasig|modsig
>> > > > > > is specified in the rule. However currently initramfs doesn't support
>> > > > > > xattr. And IMA rule "func=MODULE_CHECK appraise_type=imasig|modsig"
>> > > > > > doesn't work either because IMA will treat to-be-decompressed kernel
>> > > > > > module as not having module signature as it can't decompress kernel
>> > > > > > module to check if signature exists.
>> > > > > >
>> > > > > > So fall back to default kernel module signature verification when we have
>> > > > > > no way to verify IMA xattr.
>> > > > > >
>> > > > > > Reported-by: Karel Srot <ksrot@redhat.com>
>> > > > > > Signed-off-by: Coiby Xu <coxu@redhat.com>
>> > > > > > ---
>> > > > > > Another approach will be to make IMA decompress the kernel module to
>> > > > > > check the signature. This requires refactoring kernel module code to
>> > > > > > make the in-kernel module decompressing feature modular and seemingly
>> > > > > > more efforts are needed. A second disadvantage is it feels
>> > > > > > counter-intuitive to verify the same kernel module signature twice. And
>> > > > > > we still need to make ima_policy=secure_boot allow verifying appended
>> > > > > > module signature.
>> > > > > >
>> > > > > > Anyways, I'm open to suggestions and can try the latter approach if
>> > > > > > there are some benefits I'm not aware of or a better approach.
>> > > > >
>> > > > > Coiby, there are multiple issues being discussed here. Before deciding on an
>> > > > > appropriate solution, let's frame the issues(s) properly.
>> > >
>> > > Hi Mimi,
>> > >
>> > > Thanks for listing and framing the issues! Sorry, it took me a while to
>> > > go through your feedback as I also had a 8-day holiday.
>> > >
>> > > > >
>> > > > > 1. The finit_module syscall eventually calls init_module_from_file() to read the
>> > > > > module into memory and then decompress it. The problem is that the kernel
>> > > > > module signature verification occurs during the kernel_read_file(), before the
>> > > > > kernel module is decompressed. Thus, the appended kernel module signature
>> > > > > cannot be verified.
>> > >
>> > > Since IMA only accesses a kernel module as a fd or struct file*, even if
>> > > IMA hook is triggerd after kernel module is decompressed, IMA still
>> > > doesn't know the default verification has passed or can't access the
>> > > decompressed kernel buffer [2] to do the verification by itself.
>> > >
>> > > [2] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/module/main.c?h=v6.17#n3689
>> > >
>> > > > >
>> > > > > 2. CPIO doesn't have xattr support. There were multiple attempts at including
>> > > > > xattrs in CPIO, but none were upstreamed [1]. If file signatures stored in
>> > > > > security.ima were available in the initramfs, then finit_module() could verify
>> > > > > them, as opposed to the appended kernel module signature.
>> > >
>> > > Thanks you for pointing me to the work [1]. I'll take a more careful
>> > > look at [1]. I think CPIO supporting xattr can be a long-term solution
>> > > and also close a important security gap.
>> > >
>> > > > >
>> > > > > 3. The issues described above are generic, not limited to Power. When
>> > > > > CONFIG_MODULE_SIG is configured, the arch specific IMA policy rules do not
>> > > > > include an "appraise func=MODULE_CHECK".
>> > >
>> > > Yes, the issue is not limited to Power. And thanks for correcting me
>> > > that Power arch specific IMA policy rules don't have this problem! Sorry
>> > > I misread the code.
>> > >
>> > > > >
>> > > > > 4. Unlike the arch specific IMA policy rules, the built-in secure boot IMA
>> > > > > policy, specified on the boot command line as "ima_policy=secure_boot", always
>> > > > > enforces the IMA signature stored in security.ima.
>> > > > >
>> > > > > Partial solutions without kernel changes:
>> > > > > - Enable CONFIG_MODULE_SIG (Doesn't solve 4)
>> > > > > - Disable kernel module compression.
>> > > > >
>> > > > > Complete solution:
>> > > > > - Pick up and upstream Roberto Sassu's last version of initramfs support [1].
>> > > > > - Somehow prevent kernel_read_file() from failing when the kernel_read_file_id
>> > > > > enumeration is READING_MODULE and the kernel module is compressed. The change
>> > > > > might be limited to ima_post_read_file().
>> > > >
>> > > > or perhaps not totally.
>> > > >
>> > > > init_module_from_file() doesn't pass the flags variable to kernel_read_file().
>> > > > You might want to consider defining a new kernel_read_file_id enumeration named
>> > > > READING_COMPRESSED_MODULE.
>> > >
>> > > Thanks for suggesting the solutions! I like the solution of CPIO
>> > > supporting xattr but it seems it won't land in upstream soon. So I
>> > > prefer the last approach. I've implemented one [3] by defining a new
>> > > kernel_read_file_id enumeration, would you like me to post the patches
>> > > to the mailing list directly?
>> > >
>> > > [3] https://github.com/coiby/linux/tree/in_kernel_decompression_ima
>> >
>> > A few thoughts, before you post the patch.
>>
>> Thank you for sharing your thoughts!
>>
>> >
>> > 1. In the general case, the kernel module could be compressed, but without an
>> > appended signature. The new code should only defer appended signature
>> > verification, if there isn't an xattr or appended signature.
>>
>> I'll add these two condition checks, thanks!
>>
>> >
>> > 2. Instead of defining an additional process_measurement() argument to identify
>> > compressed kernel modules, to simplify the code it might be possible to define a
>> > new "func" named COMPRESSED_MODULE_CHECK.
>> >
>> > + [READING_COMPRESSED_MODULE] = MODULE_CHECK, -> COMPRESSED_MODULE_CHECK
>>
>> I also thought about this approach. But IMA rule maps kernel module
>> loading to MODULE_CHECK. If we define a new rule and ask users to use
>> this new rule, ima_policy=secure_boot still won't work.
>
>I don't have a problem with extending the "secure-boot" policy to support
>uncompressed kernel modules appended signatures, based on whether
>CONFIG_MODULE_SIG is enabled. The new rule would be in addition to the existing
>MODULE_CHECK rule.
I assume once the new rule get added, we can't remove it for userspace
backward compatibility, right? And with CPIO xattr supported, it seems
there is no need to keep this rule. So if this concern is valid, do you
think we shall switch to another approach i.e. to make IMA support
verifying decompressed module and then make "secure-boot" to allow
appended module signature?
Another thought is to make CPIO support xattr. Today I realize that
ima_policy=secure_boot can also cause failure of loading kdump kernel.
So the issue this patch tries to resolves has much less impact than I
thought. Maybe we can wait until CPIO xattr support is ready? I'll help
review and test Roberto's patches if this is the best way forward.
>
>>
>> >
>> > 3. The patch title "ima: Use default kernel module signature verification for
>> > compressed ones" is a bit off. It should be something along the lines of "ima:
>> > defer compressed kernel module appended signature verification".
>>
>> >
>> > 4. Simplify the patch description.
>>
>> I'll rephrase the title and try simplifying it. Thanks!
>
>Thank you.
>
--
Best regards,
Coiby
^ permalink raw reply
* Re: [PATCH v5 0/34] Rework the LSM initialization
From: Paul Moore @ 2025-10-17 20:59 UTC (permalink / raw)
To: linux-security-module, linux-integrity, selinux
Cc: John Johansen, Mimi Zohar, Roberto Sassu, Fan Wu,
Mickaël Salaün, Günther Noack, Kees Cook,
Micah Morton, Casey Schaufler, Tetsuo Handa, Nicolas Bouchinet,
Xiu Jianfeng
In-Reply-To: <20251017202456.484010-36-paul@paul-moore.com>
On Fri, Oct 17, 2025 at 4:28 PM Paul Moore <paul@paul-moore.com> wrote:
>
> This is the fifth, and likely final, revision of the LSM rework patchset.
> The number of changes in this revision are very minor and barring any
> surprises I expect to merge this into the lsm/dev branch next week; I'll
> send a notice when I do. While there isn't anything in this revision
> that people haven't seen previously, if you do have any concerns or
> feedback, please let me know. Once again, thank you to all of you that
> have taken the time to review these patches.
>
> I've aldo updated the working-lsm_init_rework branch of the main LSM
> tree to contain the latest v5 revision of the patchset:
> https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git/log/?h=working-lsm_init_rework
>
> The v4 patchset:
> https://lore.kernel.org/linux-security-module/20250916220355.252592-36-paul@paul-moore.com/
>
> The RFC/v3 patchset:
> https://lore.kernel.org/linux-security-module/20250814225159.275901-36-paul@paul-moore.com/
>
> The RFC/v2 patchset:
> https://lore.kernel.org/linux-security-module/20250721232142.77224-36-paul@paul-moore.com/
>
> The RFC/v1 patchset is below, the cover letter provides some background
> and motivation for this series which still applies:
> https://lore.kernel.org/linux-security-module/20250409185019.238841-31-paul@paul-moore.com/
>
> CHANGELOG
> v5:
> - rebased to lsm/dev branch post v6.18-rc1
> - fixed a !CONFIG_SECURITYFS bug (kernel test robot)
> - fixed a missing "__rcu" annotation on a cast (kernel test robot)
> v4:
> - reworked the lsm_read() function (John, Roberto, Tetsuo)
> - replaced the IMA/EVM patch with one from Roberto
> RFC/v3:
> - rebased to lsm/dev branch
> - fixed IMA/EVM initcall comment (Roberto)
> - fixed CONFIG_IMA and CONFIG_EVM problems (Nicolas, Roberto)
> - fixed CONFIG_SECURITY_SMACK_NETFILTER problems (Roberto)
> - fixed the IMA/EVM header file include macro protections
> - fixed an off-by-one string length issue in lsm_read() (Casey)
> RFC/v2:
> - rename lsm_prep_single() to lsm_prepare()
> - drop the lsm_prop counting patch
> - drop the platform_certs changes from the IMA/EVM patch (Mimi)
> - split/reorder anough patches in the patchset that I lost track
> - added missing function comment blocks in the SELinux patches
> - split patch 04/29 into smaller patches (Kees)
> - fix an LSM list output problem in an intermediate patch (Kees)
> - preserve the "lsm_active_cnt" variable name (Casey)
> - cache the lsm_read() string (Kees)
> - squashed, split, and reordered the enabled/ordering patches
> - reworked the Smack patch (Casey)
> - conditionalized the SELinux IB init code (Stephen)
> - fixed missing Smack "__init" annotation (Fan)
> - fixed a potential unused variable warning in IMA/EVM (John)
> - fixed the placeholder commit descriptions (various)
> RFC/v1:
> - initial version
>
> --
> Paul Moore (33):
> lsm: split the notifier code out into lsm_notifier.c
> lsm: split the init code out into lsm_init.c
> lsm: consolidate lsm_allowed() and prepare_lsm() into
> lsm_prepare()
> lsm: introduce looping macros for the initialization code
> lsm: integrate report_lsm_order() code into caller
> lsm: integrate lsm_early_cred() and lsm_early_task() into caller
> lsm: rename ordered_lsm_init() to lsm_init_ordered()
> lsm: replace the name field with a pointer to the lsm_id struct
> lsm: rename the lsm order variables for consistency
> lsm: rework lsm_active_cnt and lsm_idlist[]
> lsm: get rid of the lsm_names list and do some cleanup
> lsm: rework the LSM enable/disable setter/getter functions
> lsm: rename exists_ordered_lsm() to lsm_order_exists()
> lsm: rename/rework append_ordered_lsm() into lsm_order_append()
> lsm: rename/rework ordered_lsm_parse() to lsm_order_parse()
> lsm: cleanup the LSM blob size code
> lsm: cleanup initialize_lsm() and rename to lsm_init_single()
> lsm: fold lsm_init_ordered() into security_init()
> lsm: add/tweak function header comment blocks in lsm_init.c
> lsm: cleanup the debug and console output in lsm_init.c
> lsm: output available LSMs when debugging
> lsm: group lsm_order_parse() with the other lsm_order_*()
> functions
> lsm: introduce an initcall mechanism into the LSM framework
> loadpin: move initcalls to the LSM framework
> ipe: move initcalls to the LSM framework
> smack: move initcalls to the LSM framework
> tomoyo: move initcalls to the LSM framework
> safesetid: move initcalls to the LSM framework
> apparmor: move initcalls to the LSM framework
> lockdown: move initcalls to the LSM framework
> selinux: move initcalls to the LSM framework
> lsm: consolidate all of the LSM framework initcalls
> lsm: add a LSM_STARTED_ALL notification event
>
> Roberto Sassu (1):
> ima,evm: move initcalls to the LSM framework
Unfortunately I ran into a mail throttling issue when sending this
patchset and my initial post only contained the first 22 patches, so I
resent patches 23 through 34. While I set the reply-to header
properly, the remaining patches should be threaded under the cover
letter, the numbering was reset to that patch 23/34 appears as patch
01/11. Sigh.
My apologies, I thought this would be preferable to resending the full
patchset, and potentially getting throttled again.
As this revision is fairly minor, and everyone that *needs* to review
the patchset has already, I'm going to leave things as-is on the list.
All of the patches are present in that thread, the numbering is just a
little wonky. If there is any concern about ordering, you can always
check the git repo mentioned in the cover letter.
Once again, sorry for the confusion, I've never hit throttling like
that before, but if it becomes a regular problem I'll make adjustments
to prevent it from becoming more of an issue.
--
paul-moore.com
^ permalink raw reply
* [PATCH v5 11/11] lsm: add a LSM_STARTED_ALL notification event
From: Paul Moore @ 2025-10-17 20:48 UTC (permalink / raw)
To: linux-security-module, linux-integrity, selinux
Cc: john.johansen, zohar, roberto.sassu, wufan, mic, gnoack, kees,
mortonm, casey, penguin-kernel, nicolas.bouchinet, xiujianfeng
In-Reply-To: <20251017204815.505363-12-paul@paul-moore.com>
Add a new LSM notifier event, LSM_STARTED_ALL, which is fired once at
boot when all of the LSMs have been started.
Reviewed-by: Kees Cook <kees@kernel.org>
Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: John Johansen <john.johhansen@canonical.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
include/linux/security.h | 1 +
security/lsm_init.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/include/linux/security.h b/include/linux/security.h
index 556890ea2e83..eb36451ce41f 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -85,6 +85,7 @@ struct timezone;
enum lsm_event {
LSM_POLICY_CHANGE,
+ LSM_STARTED_ALL,
};
struct dm_verity_digest {
diff --git a/security/lsm_init.c b/security/lsm_init.c
index 0f668bca98f9..6bb67d41ce52 100644
--- a/security/lsm_init.c
+++ b/security/lsm_init.c
@@ -556,6 +556,7 @@ static int __init security_initcall_late(void)
rc = lsm_initcall(late);
lsm_pr_dbg("all enabled LSMs fully activated\n");
+ call_blocking_lsm_notifier(LSM_STARTED_ALL, NULL);
return rc;
}
--
2.51.1.dirty
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox