* [PATCH 4/4] keys, trusted: Remove redundant helper
From: Jarkko Sakkinen @ 2025-09-22 16:43 UTC (permalink / raw)
To: linux-integrity
Cc: Jarkko Sakkinen, David Howells, Jarkko Sakkinen, Paul Moore,
James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20250922164318.3540792-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>
---
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 c414a7006d78..8e3b283a59b2 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -198,36 +198,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
*
@@ -507,19 +477,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
* Re: [PATCH v3] ima: don't clear IMA_DIGSIG flag when setting or removing non-IMA xattr
From: Mimi Zohar @ 2025-09-22 17:11 UTC (permalink / raw)
To: Coiby Xu, linux-integrity
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM,
open list
In-Reply-To: <20250915055524.2187783-1-coxu@redhat.com>
On Mon, 2025-09-15 at 13:55 +0800, Coiby Xu wrote:
> Currently when both IMA and EVM are in fix mode, the IMA signature will
> be reset to IMA hash if a program first stores IMA signature in
> security.ima and then writes/removes some other security xattr for the
> file.
>
> For example, on Fedora, after booting the kernel with "ima_appraise=fix
> evm=fix ima_policy=appraise_tcb" and installing rpm-plugin-ima,
> installing/reinstalling a package will not make good reference IMA
> signature generated. Instead IMA hash is generated,
>
> # getfattr -m - -d -e hex /usr/bin/bash
> # file: usr/bin/bash
> security.ima=0x0404...
>
> This happens because when setting security.selinux, the IMA_DIGSIG flag
> that had been set early was cleared. As a result, IMA hash is generated
> when the file is closed.
>
> Similarly, IMA signature can be cleared on file close after removing
> security xattr like security.evm or setting/removing ACL.
>
> Prevent replacing the IMA file signature with a file hash, by preventing
> the IMA_DIGSIG flag from being reset.
>
> Here's a minimal C reproducer which sets security.selinux as the last
> step which can also replaced by removing security.evm or setting ACL,
>
> #include <stdio.h>
> #include <sys/xattr.h>
> #include <fcntl.h>
> #include <unistd.h>
> #include <string.h>
> #include <stdlib.h>
>
> int main() {
> const char* file_path = "/usr/sbin/test_binary";
> const char* hex_string = "030204d33204490066306402304";
> int length = strlen(hex_string);
> char* ima_attr_value;
> int fd;
>
> fd = open(file_path, O_WRONLY|O_CREAT|O_EXCL, 0644);
> if (fd == -1) {
> perror("Error opening file");
> return 1;
> }
>
> ima_attr_value = (char*)malloc(length / 2 );
> for (int i = 0, j = 0; i < length; i += 2, j++) {
> sscanf(hex_string + i, "%2hhx", &ima_attr_value[j]);
> }
>
> if (fsetxattr(fd, "security.ima", ima_attr_value, length/2, 0) == -1) {
> perror("Error setting extended attribute");
> close(fd);
> return 1;
> }
>
> const char* selinux_value= "system_u:object_r:bin_t:s0";
> if (fsetxattr(fd, "security.selinux", selinux_value, strlen(selinux_value), 0) == -1) {
> perror("Error setting extended attribute");
> close(fd);
> return 1;
> }
>
> close(fd);
>
> return 0;
> }
>
> Signed-off-by: Coiby Xu <coxu@redhat.com>
Thanks, Coiby. The patch is now queued in next-integrity.
> ---
> security/integrity/ima/ima_appraise.c | 23 ++++++++++++++++++-----
> 1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index f435eff4667f..5149ff4fd50d 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -694,6 +694,15 @@ static int ima_protect_xattr(struct dentry *dentry, const char *xattr_name,
> return 0;
> }
>
> +/*
> + * ima_reset_appraise_flags - reset ima_iint_cache flags
> + *
> + * @digsig: whether to clear/set IMA_DIGSIG flag, tristate values
> + * 0: clear IMA_DIGSIG
> + * 1: set IMA_DIGSIG
> + * -1: don't change IMA_DIGSIG
> + *
> + */
> static void ima_reset_appraise_flags(struct inode *inode, int digsig)
> {
> struct ima_iint_cache *iint;
> @@ -706,9 +715,9 @@ static void ima_reset_appraise_flags(struct inode *inode, int digsig)
> return;
> iint->measured_pcrs = 0;
> set_bit(IMA_CHANGE_XATTR, &iint->atomic_flags);
> - if (digsig)
> + if (digsig == 1)
> set_bit(IMA_DIGSIG, &iint->atomic_flags);
> - else
> + else if (digsig == 0)
> clear_bit(IMA_DIGSIG, &iint->atomic_flags);
> }
>
> @@ -794,6 +803,8 @@ static int ima_inode_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
> digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
> } else if (!strcmp(xattr_name, XATTR_NAME_EVM) && xattr_value_len > 0) {
> digsig = (xvalue->type == EVM_XATTR_PORTABLE_DIGSIG);
> + } else {
> + digsig = -1;
> }
> if (result == 1 || evm_revalidate_status(xattr_name)) {
> ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
> @@ -807,7 +818,7 @@ static int ima_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> const char *acl_name, struct posix_acl *kacl)
> {
> if (evm_revalidate_status(acl_name))
> - ima_reset_appraise_flags(d_backing_inode(dentry), 0);
> + ima_reset_appraise_flags(d_backing_inode(dentry), -1);
>
> return 0;
> }
> @@ -815,11 +826,13 @@ static int ima_inode_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
> static int ima_inode_removexattr(struct mnt_idmap *idmap, struct dentry *dentry,
> const char *xattr_name)
> {
> - int result;
> + int result, digsig = -1;
>
> result = ima_protect_xattr(dentry, xattr_name, NULL, 0);
> if (result == 1 || evm_revalidate_status(xattr_name)) {
> - ima_reset_appraise_flags(d_backing_inode(dentry), 0);
> + if (!strcmp(xattr_name, XATTR_NAME_IMA))
> + digsig = 0;
> + ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
> if (result == 1)
> result = 0;
> }
>
> base-commit: 7aac71907bdea16e2754a782b9d9155449a9d49d
^ permalink raw reply
* Re: [PATCH v3 00/35] Compiler-Based Capability- and Locking-Analysis
From: Christoph Hellwig @ 2025-09-22 17:11 UTC (permalink / raw)
To: Marco Elver
Cc: Christoph Hellwig, Nathan Chancellor, Peter Zijlstra, Boqun Feng,
Ingo Molnar, Will Deacon, David S. Miller, Luc Van Oostenryck,
Paul E. McKenney, Alexander Potapenko, Arnd Bergmann,
Bart Van Assche, Bill Wendling, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Jonathan Corbet, Josh Triplett,
Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Neeraj Upadhyay,
Nick Desaulniers, Steven Rostedt, Tetsuo Handa, Thomas Gleixner,
Thomas Graf, Uladzislau Rezki, Waiman Long, kasan-dev,
linux-crypto, linux-doc, linux-kbuild, linux-kernel, linux-mm,
linux-security-module, linux-sparse, llvm, rcu
In-Reply-To: <aNEX46WJh2IWhVUc@elver.google.com>
On Mon, Sep 22, 2025 at 11:33:23AM +0200, Marco Elver wrote:
> I gave this a try, and with the below patch and the Clang fix [1],
> fs/xfs compiles cleanly. I think the fundamental limitation are the
> conditional locking wrappers. I suspect it's possible to do better than
> disabling the analysis here, by overapproximating the lock set taken
> (like you did elsewhere), so that at least the callers are checked, but
> when I tried it showed lots of callers need annotating as well, so I
> gave up at that point. Still, it might be better than no checking at
> all.
I guess this at least allows us to work with the analysis, even if it
drops coverage for one of the most important locks. I guess you also
have CONFIG_XFS_QUOTA disabled as that would lead to similar warnings,
and also currently has the lock the object on return if it's not a
NULL return case? I now have a local series to remove that instance,
but I've seen that pattern elsewhere in the kernel code.
Besides the conditional locking these two also do another thing that
is nasty to the analysis, the locked state can be attached to a
transaction and unlocked at transaction commit. Not sure how to best
model that.
> [1] https://github.com/llvm/llvm-project/pull/159921
Thanks for all the work!
^ permalink raw reply
* Re: [PATCH v3 00/35] Compiler-Based Capability- and Locking-Analysis
From: Christoph Hellwig @ 2025-09-22 17:12 UTC (permalink / raw)
To: Bart Van Assche
Cc: Christoph Hellwig, Nathan Chancellor, Marco Elver, Peter Zijlstra,
Boqun Feng, Ingo Molnar, Will Deacon, David S. Miller,
Luc Van Oostenryck, Paul E. McKenney, Alexander Potapenko,
Arnd Bergmann, Bill Wendling, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Jonathan Corbet, Josh Triplett,
Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Neeraj Upadhyay,
Nick Desaulniers, Steven Rostedt, Tetsuo Handa, Thomas Gleixner,
Thomas Graf, Uladzislau Rezki, Waiman Long, kasan-dev,
linux-crypto, linux-doc, linux-kbuild, linux-kernel, linux-mm,
linux-security-module, linux-sparse, llvm, rcu
In-Reply-To: <a75f7b70-2b72-4bb0-a940-52835f290502@acm.org>
On Fri, Sep 19, 2025 at 10:20:37AM -0700, Bart Van Assche wrote:
> locking annotations to kernel code. I ended up annotating multiple XFS
> functions with NO_THREAD_SAFETY_ANALYSIS. Maybe the locking patterns in
> XFS are too complex for compile-time analysis?
If our locking patterns are too complex for analysis, either the code or
the analysis has problems that need addressing. Potentially both.
^ permalink raw reply
* Re: [PATCH 3/4] tpm2-sessions: Remove unnecessary wrapper
From: Ben Boeckel @ 2025-09-22 17:22 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20250922164318.3540792-4-jarkko@kernel.org>
On Mon, Sep 22, 2025 at 19:43:16 +0300, Jarkko Sakkinen wrote:
> From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
>
> Open code tpm_buf_append_hmac_session_opt() because it adds unnecessary
> disperancy to the call sites (and reduces the amount of code).
^^^^^^^^^^
"discrepancy" as in "difference"? But that doesn't feel like the right
usage either. Perhaps "unnecessary abstraction"? Also, open coding it
reduces the amount of code, so some clarification to not read as
something else that "it" (`tpm_buf_append_hmac_session_opt`) does would
be clearer.
Thanks,
--Ben
^ permalink raw reply
* Re: [PATCH v3] memfd,selinux: call security_inode_init_security_anon
From: Paul Moore @ 2025-09-22 19:30 UTC (permalink / raw)
To: Stephen Smalley
Cc: Thiébaud Weksteen, Hugh Dickins, James Morris,
Jeff Vander Stoep, Nick Kralevich, Jeff Xu, Baolin Wang,
Isaac Manjarres, linux-kernel, linux-security-module, selinux,
linux-mm
In-Reply-To: <CAEjxPJ5GidA9oT_fbKRe_nH1J3mER0ggM-dBW=Nuo765JDuQKg@mail.gmail.com>
On Mon, Sep 22, 2025 at 9:12 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> When would you recommend that I re-apply the corresponding userspace
> patch to reserve this policy capability number for memfd_class?
> After it is moved to selinux/dev? Understand that it isn't truly
> reserved until it lands in a kernel.org kernel but would prefer to
> reapply it sooner than that since there may be other policy capability
> requests queueing up (e.g. bpf token) that should be done relative to
> it. Can always revert it again if necessary, at least until another
> userspace release is made (not sure on timeline for that).
When it comes to API issues like this, my standard answer is "tagged
release from Linus" as it is the safest option, but you know that
already.
The fuzzier answer is that unless something crazy happens, I'm likely
going to move the patches, in order, from selinux/dev-staging into
selinux/dev when the merge window closes. This means that any
policycap API additions for the next cycle are going to start with the
memfd_class policycap, so it *should* be fairly safe to merge the
userspace bits now, I just wouldn't do a userspace release with that
API change until we see a tagged release from Linus.
--
paul-moore.com
^ permalink raw reply
* Re: what's going on with aa_destroy_aafs() call in apparmor_init()?
From: John Johansen @ 2025-09-22 20:46 UTC (permalink / raw)
To: Al Viro, John Johansen; +Cc: linux-fsdevel, linux-security-module
In-Reply-To: <20250921073655.GM39973@ZenIV>
On 9/21/25 00:36, Al Viro wrote:
> Correct me if I'm wrong, but as far as I can tell apparmor_init()
> ends up being called from security_init(), which is called before the call
> of vfs_caches_init(), not to mention fs_initcall stuff.
sigh, yes that is what is happening.
> If that's the case, what is this doing there?
> error:
> aa_destroy_aafs();
> AA_ERROR("Error creating AppArmor securityfs\n");
> return error;
it shouldn't be it should have been dropped ages ago. Its a mess up on a
patch and then it just got carried forward
> aa_create_aafs() is called via fs_initcall; moreover, it will bail out
> if called before apparmor_initialized has become true, so...
>
> While we are at it, what will happen if apparmor_init() succeeds, but
> aa_create_fs() fails afterwards?
Yeah that one is ugly. I will have to poke at the best solution atm.
For current kernels, no policy will load. Backing out the LSM hooks
isn't going to happen but we will need to just run in an unconfined
state and make sure any references to the fs handle the null condition.
> If nothing else, aa_null_path will be left {NULL, NULL}, which will
> immediately oops dentry_open() in aa_inherit_files()...
indeed, I have to wonder if it was always that way or I just messed up.
I borrowed the pattern from selinux but should have also dug into what
it was doing.
regardless it needs to be fixed.
We can check that its null and set the file to null for replace fds.
Give me a bit to see about some patches.
^ permalink raw reply
* Re: [PATCH 31/39] convert selinuxfs
From: John Johansen @ 2025-09-22 21:12 UTC (permalink / raw)
To: Al Viro, Paul Moore
Cc: linux-fsdevel, torvalds, brauner, jack, kees, casey,
linux-security-module
In-Reply-To: <20250921214110.GN39973@ZenIV>
On 9/21/25 14:41, Al Viro wrote:
> On Sun, Sep 21, 2025 at 04:44:28PM -0400, Paul Moore wrote:
>>> + dput(dentry);
>>> + return dentry; // borrowed
>>> }
>>
>> Prefer C style comments on their own line:
>>
>> dput(dentry);
>> /* borrowed dentry */
>> return dentry;
>
> Umm... IMO that's more of an annotation along the lines of "fallthru"...
>
>>> @@ -2079,15 +2088,14 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc)
>>> goto err;
>>> }
>>>
>>> - fsi->policycap_dir = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME,
>>> + dentry = sel_make_dir(sb->s_root, POLICYCAP_DIR_NAME,
>>> &fsi->last_ino);
>>
>> I'd probably keep fsi->policycap_dir in this patch simply to limit the
>> scope of this patch to just the DCACHE_PERSISTENT related changes, but
>> I'm not going to make a big fuss about that.
>
> Not hard to split that way. Will do...
>
>
> BTW, an unrelated question: does userland care about selinuxfs /null being
> called that (and being on selinuxfs, for that matter)? Same for the
> apparmor's securityfs /apparmor/.null...
>
For apparmor the userspace doesn't care, ideally userspace wouldn't even
see it exists.
> If nobody cares, I would rather add an internal-only filesystem with
> root being a character device (1,3) and whatever markings selinux et.al.
> need for it. With open_devnull(creds) provided for selinux,
> apparmor and whoever else wants to play with neutering descriptors
> on exec...
^ permalink raw reply
* Re: [PATCH v4 11/34] lsm: get rid of the lsm_names list and do some cleanup
From: Paul Moore @ 2025-09-22 21:52 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-integrity, selinux, John Johansen,
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: <63cf73f5ed8ceda0d68df416c2ba18334e7a9b83.camel@linux.ibm.com>
On Mon, Sep 22, 2025 at 6:53 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Sun, 2025-09-21 at 15:23 -0400, Paul Moore wrote:
> > On Fri, Sep 19, 2025 at 3:16 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > >
> > > On Tue, 2025-09-16 at 18:03 -0400, Paul Moore wrote:
> > > > The LSM currently has a lot of code to maintain a list of the currently
> > > > active LSMs in a human readable string, with the only user being the
> > > > "/sys/kernel/security/lsm" code. Let's drop all of that code and
> > > > generate the string on first use and then cache it for subsequent use.
> > > >
> > > > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > >
> > > FYI, checkpatch.pl complains of unbalanced braces, otherwise
> >
> > Looks good to me?
> >
> > % stg export --stdout lsm-lsm_names_cleanup | ./scripts/checkpatch.pl -
> > total: 0 errors, 0 warnings, 139 lines checked
> >
> > Your patch has no obvious style problems and is ready for submission.
>
> Try adding "--strict", which enforces
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#placing-braces-and-spaces
Ah, yes, sure. FWIW, I view checkpatch's findings mostly as
"advisory"; oftentimes it can help catch important things, other times
I think it's kinda silly (and no, I don't have a list of each, so
please don't ask). I often tell people new to kernel development that
it is generally better to follow checkpatch's suggestions if you are
uncertain, however don't be surprised if a maintainer prefers
something slightly different.
For those reasons I don't ever bother with the "strict" checkpatch mode.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH bpf-next v7 0/5] Signed BPF programs
From: Paul Moore @ 2025-09-23 1:26 UTC (permalink / raw)
To: KP Singh, bboscaccy, ast; +Cc: bpf, linux-security-module, kys, daniel, andrii
In-Reply-To: <20250921160120.9711-1-kpsingh@kernel.org>
On Sun, Sep 21, 2025 at 12:01 PM KP Singh <kpsingh@kernel.org> wrote:
>
> BPF Signing has gone over multiple discussions in various conferences with the
> kernel and BPF community ...
In addition to the signature scheme KP has proposed, if we want to
increase adoption of BPF to those who currently disable it due to
strict code provenance and integrity concerns, we need to merge
Blaise's optional signature scheme as described most recently in the
patch posting linked below:
https://lore.kernel.org/linux-security-module/20250909162345.569889-1-bboscaccy@linux.microsoft.com/
It is important to note that Blaise's patch allows both signature
schemes to coexist together in the same kernel, allowing users to
select which scheme to use on each program load, and enables the
kernel to support policy to selectively enforce rules around which
signature scheme is permitted for each load.
While we've discussed this in the past, Blaise's signature scheme is
important as it satisfies requirements that aren't otherwise met:
* Both the lskel and original BPF program are verified for provenance
and integrity prior to the security_bpf_prog_load() hook. This allows
LSMs, including BPF LSMs, to make access control decisions on the
entirety of the verification, not just the verification of the lskel.
For those users who have very strict security requirements which
require analysis of the system's security policy as part of their
vetting, this limits the scope of the BPF signature verification
policy analysis to just the LSM's policy. This approach is in keeping
with most every other security significant operation in the kernel,
and is also seen as a best practice, since it significantly simplifies
the analysis of the system's security properties. KP's signature
scheme, while it does verify the original program, it offloads the
digest verification of the original BPF program to the lskel. Aside
from breaking with most other kernel-based access control approaches,
this means that analysis of only the LSM's policy is no longer
sufficient, security officers must vet both the LSM's policy as well
as the digest verification code in every lskel used on the system.
* Verification of both the lskel and the original BPF program prior to
the security_bpf_prog_load() hook also makes it possible for
observability, measurement, and attestation focused LSMs to more
easily audit, measure, and attest the verified lskel and original BPF,
as all of the components have passed provenance and integrity
verification while the measurements/digests still remain stable as
none of the lskel based transforms have yet to take place. Using KP's
signature scheme this isn't quite possible as the original BPF program
is verified during the execution of the lskel which potentially
mutates the original BPF program.
As an aside, Blaise's signature scheme also has the nice property that
BPF ELF objects using Blaise's signature scheme are backwards
compatible with existing kernels, allowing the signed BPF to be loaded
on older kernels, albeit without signature verification for obvious
reasons. While this may not be important to some users, BPF ELF
objects using KP's signature scheme will likely fail when loaded on
older kernels.
Taken individually, KP's and Blaise's signature schemes may not
satisfy all of the use cases and requirements for BPF signing that
have been discussed over the years, but I believe that the two schemes
together can satisfy everyone's needs.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH bpf-next v7 0/5] Signed BPF programs
From: patchwork-bot+netdevbpf @ 2025-09-23 2:30 UTC (permalink / raw)
To: KP Singh
Cc: bpf, linux-security-module, bboscaccy, paul, kys, ast, daniel,
andrii
In-Reply-To: <20250921160120.9711-1-kpsingh@kernel.org>
Hello:
This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Sun, 21 Sep 2025 18:01:15 +0200 you wrote:
> # v6 -> v7
>
> * list header mess up in Patch subject
>
> # v5 -> v6
>
> * Rebase again removing the first 7 patches as they are merged already.
>
> [...]
Here is the summary with links:
- [bpf-next,v7,1/5] bpf: Implement signature verification for BPF programs
https://git.kernel.org/bpf/bpf-next/c/349271568303
- [bpf-next,v7,2/5] libbpf: Update light skeleton for signing
(no matching commit)
- [bpf-next,v7,3/5] libbpf: Embed and verify the metadata hash in the loader
https://git.kernel.org/bpf/bpf-next/c/ea923080c145
- [bpf-next,v7,4/5] bpftool: Add support for signing BPF programs
https://git.kernel.org/bpf/bpf-next/c/40863f4d6ef2
- [bpf-next,v7,5/5] selftests/bpf: Enable signature verification for some lskel tests
https://git.kernel.org/bpf/bpf-next/c/b720903e2b14
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH bpf-next v7 4/5] bpftool: Add support for signing BPF programs
From: Alexei Starovoitov @ 2025-09-23 2:31 UTC (permalink / raw)
To: Quentin Monnet
Cc: KP Singh, bpf, LSM List, Blaise Boscaccy, Paul Moore,
K. Y. Srinivasan, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko
In-Reply-To: <ee292661-0ffb-413e-be9c-eb21f5379688@kernel.org>
On Mon, Sep 22, 2025 at 4:24 AM Quentin Monnet <qmo@kernel.org> wrote:
>
> 2025-09-21 18:01 UTC+0200 ~ KP Singh <kpsingh@kernel.org>
> > Two modes of operation being added:
> >
> > Add two modes of operation:
> >
> > * For prog load, allow signing a program immediately before loading. This
> > is essential for command-line testing and administration.
> >
> > bpftool prog load -S -k <private_key> -i <identity_cert> fentry_test.bpf.o
> >
> > * For gen skeleton, embed a pre-generated signature into the C skeleton
> > file. This supports the use of signed programs in compiled applications.
> >
> > bpftool gen skeleton -S -k <private_key> -i <identity_cert> fentry_test.bpf.o
> >
> > Generation of the loader program and its metadata map is implemented in
> > libbpf (bpf_obj__gen_loader). bpftool generates a skeleton that loads
> > the program and automates the required steps: freezing the map, creating
> > an exclusive map, loading, and running. Users can use standard libbpf
> > APIs directly or integrate loader program generation into their own
> > toolchains.
> >
> > Signed-off-by: KP Singh <kpsingh@kernel.org>
>
>
> Acked-by: Quentin Monnet <qmo@kernel.org>
>
> Thanks a lot!
>
>
> > ---
> > .../bpf/bpftool/Documentation/bpftool-gen.rst | 13 +-
> > .../bpftool/Documentation/bpftool-prog.rst | 14 +-
> > tools/bpf/bpftool/Makefile | 6 +-
> > tools/bpf/bpftool/cgroup.c | 4 +
> > tools/bpf/bpftool/gen.c | 68 +++++-
> > tools/bpf/bpftool/main.c | 26 ++-
> > tools/bpf/bpftool/main.h | 11 +
> > tools/bpf/bpftool/prog.c | 29 ++-
> > tools/bpf/bpftool/sign.c | 212 ++++++++++++++++++
> > 9 files changed, 372 insertions(+), 11 deletions(-)
> > create mode 100644 tools/bpf/bpftool/sign.c
> >
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-gen.rst b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> > index ca860fd97d8d..d0a36f442db7 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
> > @@ -16,7 +16,7 @@ SYNOPSIS
> >
> > **bpftool** [*OPTIONS*] **gen** *COMMAND*
> >
> > -*OPTIONS* := { |COMMON_OPTIONS| | { **-L** | **--use-loader** } }
> > +*OPTIONS* := { |COMMON_OPTIONS| | { **-L** | **--use-loader** } | [ { **-S** | **--sign** } {**-k** <private_key.pem>} **-i** <certificate.x509> ] }
> >
> > *COMMAND* := { **object** | **skeleton** | **help** }
> >
> > @@ -186,6 +186,17 @@ OPTIONS
> > skeleton). A light skeleton contains a loader eBPF program. It does not use
> > the majority of the libbpf infrastructure, and does not need libelf.
> >
> > +-S, --sign
> > + For skeletons, generate a signed skeleton. This option must be used with
> > + **-k** and **-i**. Using this flag implicitly enables **--use-loader**.
> > +
> > +-k <private_key.pem>
> > + Path to the private key file in PEM format, required for signing.
> > +
> > +-i <certificate.x509>
> > + Path to the X.509 certificate file in PEM or DER format, required for
> > + signing.
> > +
> > EXAMPLES
> > ========
> > **$ cat example1.bpf.c**
> > diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > index f69fd92df8d8..009633294b09 100644
> > --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > @@ -18,7 +18,7 @@ SYNOPSIS
> >
> > *OPTIONS* := { |COMMON_OPTIONS| |
> > { **-f** | **--bpffs** } | { **-m** | **--mapcompat** } | { **-n** | **--nomount** } |
> > -{ **-L** | **--use-loader** } }
> > +{ **-L** | **--use-loader** } | [ { **-S** | **--sign** } **-k** <private_key.pem> **-i** <certificate.x509> ] }
>
>
> Perfect, thank you!
>
>
> >
> > *COMMANDS* :=
> > { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load** |
> > @@ -248,6 +248,18 @@ OPTIONS
> > creating the maps, and loading the programs (see **bpftool prog tracelog**
> > as a way to dump those messages).
> >
> > +-S, --sign
> > + Enable signing of the BPF program before loading. This option must be
> > + used with **-k** and **-i**. Using this flag implicitly enables
> > + **--use-loader**.
> > +
> > +-k <private_key.pem>
> > + Path to the private key file in PEM format, required when signing.
> > +
> > +-i <certificate.x509>
> > + Path to the X.509 certificate file in PEM or DER format, required when
> > + signing.
> > +
> > EXAMPLES
> > ========
> > **# bpftool prog show**
> > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > index 9e9a5f006cd2..586d1b2595d1 100644
> > --- a/tools/bpf/bpftool/Makefile
> > +++ b/tools/bpf/bpftool/Makefile
> > @@ -130,8 +130,8 @@ include $(FEATURES_DUMP)
> > endif
> > endif
> >
> > -LIBS = $(LIBBPF) -lelf -lz
> > -LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz
> > +LIBS = $(LIBBPF) -lelf -lz -lcrypto
> > +LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz -lcrypto
> >
> > ifeq ($(feature-libelf-zstd),1)
> > LIBS += -lzstd
> > @@ -194,7 +194,7 @@ endif
> >
> > BPFTOOL_BOOTSTRAP := $(BOOTSTRAP_OUTPUT)bpftool
> >
> > -BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o)
> > +BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o sign.o)
> > $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP)
> >
> > OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
> > diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> > index 944ebe21a216..ec356deb27c9 100644
> > --- a/tools/bpf/bpftool/cgroup.c
> > +++ b/tools/bpf/bpftool/cgroup.c
> > @@ -2,6 +2,10 @@
> > // Copyright (C) 2017 Facebook
> > // Author: Roman Gushchin <guro@fb.com>
> >
> > +#undef GCC_VERSION
> > +#ifndef _GNU_SOURCE
> > +#define _GNU_SOURCE
> > +#endif
> > #define _XOPEN_SOURCE 500
> > #include <errno.h>
> > #include <fcntl.h>
> > diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> > index 67a60114368f..993c7d9484a4 100644
> > --- a/tools/bpf/bpftool/gen.c
> > +++ b/tools/bpf/bpftool/gen.c
>
> > @@ -1930,7 +1990,7 @@ static int do_help(int argc, char **argv)
> > " %1$s %2$s help\n"
> > "\n"
> > " " HELP_SPEC_OPTIONS " |\n"
> > - " {-L|--use-loader} }\n"
> > + " {-L|--use-loader} | [ {-S|--sign } {-k} <private_key.pem> {-i} <certificate.x509> ]}\n"
>
>
> With regards to our discussion on v4 - Sorry, I had not realised
> removing the braces would make the sync test fail. ACK for keeping them
> until this is resolved in the test.
>
> As for the bash completion, I agree this should not block this series.
> Please make sure to follow-up with it. I think it should be as follows:
Quentin,
since you wrote the patch can you send it ?
^ permalink raw reply
* Re: [PATCH bpf-next v7 4/5] bpftool: Add support for signing BPF programs
From: Quentin Monnet @ 2025-09-23 8:39 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: KP Singh, bpf, LSM List, Blaise Boscaccy, Paul Moore,
K. Y. Srinivasan, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko
In-Reply-To: <CAADnVQ+S1i5wcW3FK9=KhpTr8nxSBCNCvvZvWShDouTbWt9eig@mail.gmail.com>
2025-09-22 19:31 UTC-0700 ~ Alexei Starovoitov
<alexei.starovoitov@gmail.com>
> On Mon, Sep 22, 2025 at 4:24 AM Quentin Monnet <qmo@kernel.org> wrote:
>>
>> 2025-09-21 18:01 UTC+0200 ~ KP Singh <kpsingh@kernel.org>
>>> Two modes of operation being added:
>>>
>>> Add two modes of operation:
>>>
>>> * For prog load, allow signing a program immediately before loading. This
>>> is essential for command-line testing and administration.
>>>
>>> bpftool prog load -S -k <private_key> -i <identity_cert> fentry_test.bpf.o
>>>
>>> * For gen skeleton, embed a pre-generated signature into the C skeleton
>>> file. This supports the use of signed programs in compiled applications.
>>>
>>> bpftool gen skeleton -S -k <private_key> -i <identity_cert> fentry_test.bpf.o
>>>
>>> Generation of the loader program and its metadata map is implemented in
>>> libbpf (bpf_obj__gen_loader). bpftool generates a skeleton that loads
>>> the program and automates the required steps: freezing the map, creating
>>> an exclusive map, loading, and running. Users can use standard libbpf
>>> APIs directly or integrate loader program generation into their own
>>> toolchains.
>>>
>>> Signed-off-by: KP Singh <kpsingh@kernel.org>
>>
>>
>> Acked-by: Quentin Monnet <qmo@kernel.org>
>>
>> Thanks a lot!
>>
>>
>>> ---
>>> .../bpf/bpftool/Documentation/bpftool-gen.rst | 13 +-
>>> .../bpftool/Documentation/bpftool-prog.rst | 14 +-
>>> tools/bpf/bpftool/Makefile | 6 +-
>>> tools/bpf/bpftool/cgroup.c | 4 +
>>> tools/bpf/bpftool/gen.c | 68 +++++-
>>> tools/bpf/bpftool/main.c | 26 ++-
>>> tools/bpf/bpftool/main.h | 11 +
>>> tools/bpf/bpftool/prog.c | 29 ++-
>>> tools/bpf/bpftool/sign.c | 212 ++++++++++++++++++
>>> 9 files changed, 372 insertions(+), 11 deletions(-)
>>> create mode 100644 tools/bpf/bpftool/sign.c
>>>
>>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-gen.rst b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
>>> index ca860fd97d8d..d0a36f442db7 100644
>>> --- a/tools/bpf/bpftool/Documentation/bpftool-gen.rst
>>> +++ b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
>>> @@ -16,7 +16,7 @@ SYNOPSIS
>>>
>>> **bpftool** [*OPTIONS*] **gen** *COMMAND*
>>>
>>> -*OPTIONS* := { |COMMON_OPTIONS| | { **-L** | **--use-loader** } }
>>> +*OPTIONS* := { |COMMON_OPTIONS| | { **-L** | **--use-loader** } | [ { **-S** | **--sign** } {**-k** <private_key.pem>} **-i** <certificate.x509> ] }
>>>
>>> *COMMAND* := { **object** | **skeleton** | **help** }
>>>
>>> @@ -186,6 +186,17 @@ OPTIONS
>>> skeleton). A light skeleton contains a loader eBPF program. It does not use
>>> the majority of the libbpf infrastructure, and does not need libelf.
>>>
>>> +-S, --sign
>>> + For skeletons, generate a signed skeleton. This option must be used with
>>> + **-k** and **-i**. Using this flag implicitly enables **--use-loader**.
>>> +
>>> +-k <private_key.pem>
>>> + Path to the private key file in PEM format, required for signing.
>>> +
>>> +-i <certificate.x509>
>>> + Path to the X.509 certificate file in PEM or DER format, required for
>>> + signing.
>>> +
>>> EXAMPLES
>>> ========
>>> **$ cat example1.bpf.c**
>>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>>> index f69fd92df8d8..009633294b09 100644
>>> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>>> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>>> @@ -18,7 +18,7 @@ SYNOPSIS
>>>
>>> *OPTIONS* := { |COMMON_OPTIONS| |
>>> { **-f** | **--bpffs** } | { **-m** | **--mapcompat** } | { **-n** | **--nomount** } |
>>> -{ **-L** | **--use-loader** } }
>>> +{ **-L** | **--use-loader** } | [ { **-S** | **--sign** } **-k** <private_key.pem> **-i** <certificate.x509> ] }
>>
>>
>> Perfect, thank you!
>>
>>
>>>
>>> *COMMANDS* :=
>>> { **show** | **list** | **dump xlated** | **dump jited** | **pin** | **load** |
>>> @@ -248,6 +248,18 @@ OPTIONS
>>> creating the maps, and loading the programs (see **bpftool prog tracelog**
>>> as a way to dump those messages).
>>>
>>> +-S, --sign
>>> + Enable signing of the BPF program before loading. This option must be
>>> + used with **-k** and **-i**. Using this flag implicitly enables
>>> + **--use-loader**.
>>> +
>>> +-k <private_key.pem>
>>> + Path to the private key file in PEM format, required when signing.
>>> +
>>> +-i <certificate.x509>
>>> + Path to the X.509 certificate file in PEM or DER format, required when
>>> + signing.
>>> +
>>> EXAMPLES
>>> ========
>>> **# bpftool prog show**
>>> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
>>> index 9e9a5f006cd2..586d1b2595d1 100644
>>> --- a/tools/bpf/bpftool/Makefile
>>> +++ b/tools/bpf/bpftool/Makefile
>>> @@ -130,8 +130,8 @@ include $(FEATURES_DUMP)
>>> endif
>>> endif
>>>
>>> -LIBS = $(LIBBPF) -lelf -lz
>>> -LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz
>>> +LIBS = $(LIBBPF) -lelf -lz -lcrypto
>>> +LIBS_BOOTSTRAP = $(LIBBPF_BOOTSTRAP) -lelf -lz -lcrypto
>>>
>>> ifeq ($(feature-libelf-zstd),1)
>>> LIBS += -lzstd
>>> @@ -194,7 +194,7 @@ endif
>>>
>>> BPFTOOL_BOOTSTRAP := $(BOOTSTRAP_OUTPUT)bpftool
>>>
>>> -BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o)
>>> +BOOTSTRAP_OBJS = $(addprefix $(BOOTSTRAP_OUTPUT),main.o common.o json_writer.o gen.o btf.o sign.o)
>>> $(BOOTSTRAP_OBJS): $(LIBBPF_BOOTSTRAP)
>>>
>>> OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
>>> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
>>> index 944ebe21a216..ec356deb27c9 100644
>>> --- a/tools/bpf/bpftool/cgroup.c
>>> +++ b/tools/bpf/bpftool/cgroup.c
>>> @@ -2,6 +2,10 @@
>>> // Copyright (C) 2017 Facebook
>>> // Author: Roman Gushchin <guro@fb.com>
>>>
>>> +#undef GCC_VERSION
>>> +#ifndef _GNU_SOURCE
>>> +#define _GNU_SOURCE
>>> +#endif
>>> #define _XOPEN_SOURCE 500
>>> #include <errno.h>
>>> #include <fcntl.h>
>>> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
>>> index 67a60114368f..993c7d9484a4 100644
>>> --- a/tools/bpf/bpftool/gen.c
>>> +++ b/tools/bpf/bpftool/gen.c
>>
>>> @@ -1930,7 +1990,7 @@ static int do_help(int argc, char **argv)
>>> " %1$s %2$s help\n"
>>> "\n"
>>> " " HELP_SPEC_OPTIONS " |\n"
>>> - " {-L|--use-loader} }\n"
>>> + " {-L|--use-loader} | [ {-S|--sign } {-k} <private_key.pem> {-i} <certificate.x509> ]}\n"
>>
>>
>> With regards to our discussion on v4 - Sorry, I had not realised
>> removing the braces would make the sync test fail. ACK for keeping them
>> until this is resolved in the test.
>>
>> As for the bash completion, I agree this should not block this series.
>> Please make sure to follow-up with it. I think it should be as follows:
>
> Quentin,
> since you wrote the patch can you send it ?
>
Sure, I will
^ permalink raw reply
* Re: [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust
From: Jarkko Sakkinen @ 2025-09-23 14:30 UTC (permalink / raw)
To: Stefano Garzarella
Cc: linux-integrity, Jarkko Sakkinen, Stefan Berger, Peter Huewe,
Jason Gunthorpe, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar, open list,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <gg6tsuyhnopcwed3zr7p7ikjq3vqi4ijxwfxjqwscx5hjk7lk2@w7e32ofhufov>
On Mon, Sep 22, 2025 at 10:44:34AM +0200, Stefano Garzarella wrote:
> On Sun, Sep 21, 2025 at 05:08:01AM +0300, Jarkko Sakkinen wrote:
> > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> >
> > Create more ergonomic primitives to work with TPM buffers, where
> > 'tpm_buf_init*' initialize the memory and 'tpm_buf_reset*' (re)set a buffer
> > for a particular use and purpose. The new primitives are ubiquitos when
> > it comes to heap and stack usage.
> >
> > Given that the kzalloc is decoupled, a managed allocation can be done
> > trivially:
> >
> > struct tpm_buf *buf __free(kfree) buf = kzalloc(TPM_BUF_MAX_SIZE,
> > GFP_KERNEL);
> >
> > This effectively zeros out the odds having any memory leaks with TPM
> > buffers. The new structures can be later used to widen the use of stack
> > over heap in the subsystem in the critical code paths..
> >
> > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > ---
> > v10:
> > - No changes.
> > v9:
> > - Rename pre-existing TPM_BUFSIZE as TPM_BUF_MAX_SIZE and redeclare
> > it in include/linux/tpm.h, and use this constant instead of
> > PAGE_SIZE in tpm_buf_init*. Define TPM_BUF_MIN_SIZE to be a
> > sane placeholder value for stack allocations.
> > - Fix and restructure invariant for the sake of clarity.
> > - memset buffers to zero in tpm_buf_init* in order to guarantee a legit
> > initial state. This does not cause any regressions but once tpm_buf
> > is allocaed from stack at some site, this will zero out chance of
> > corrupted state.
> > v8:
> > - Decouple memory init i.e. bring tpm_init* back but with new fresh
> > form.
> > - Cap buf_size to page size and also make checks in tpm_buf_append
> > safe:
> > https://lore.kernel.org/linux-integrity/hwsx2t2tkbos3g7k2syemxbvc6sfrsbviwm64ubcdl6ms7ljvo@toetomkhheht/
> > - Fix trusted_tpm_send() and simplify the flow.
> > v7:
> > - Additional function comments and invariant was unfortunately left to
> > my staging area so here's the addition (does not affect functionality).
> > v6:
> > - Removed two empty lines as requested by Stefan:
> > https://lore.kernel.org/linux-integrity/be1c5bef-7c97-4173-b417-986dc90d779c@linux.ibm.com/
> > - Add 'capacity' field as this makes easy to stretch tpm_buf into stack
> > allocation.
> > v5:
> > - I tested this version also with TPM 1.2 by booting up and checking
> > that sysfs attributes work.
> > - Fixed the length check against capacity (James) with TPM_BUF_CAPACITY.
> > - Fixed declaration style: do it at the site (Jason).
> > - Improved commit message (Stefan).
> > - Removed "out" label from tpm2_pcr_read() (Stefan).
> > - Removed spurious "return rc;" from tpm2_get_pcr_allocation() (Stefan).
> > v4:
> > - Wrote a more a descriptive short summary and improved description.
> > - Fixed the error in documentation: there is 4090 bytes of space left
> > for the payload - not 4088 bytes.
> > - Turned tpm_buf_alloc() into inline function.
> > v3:
> > - Removed the cleanup class and moved on using __free(kfree) instead.
> > - Removed `buf_size` (James).
> > - I'm open for the idea of splitting still (Jason) but I'll hold
> > at least this revision just to check that my core idea here
> > is correct.
> > v2:
> > - Implement also memory allocation using the cleanup class.
> > - Rewrote the commit message.
> > - Implemented CLASS_TPM_BUF() helper macro.
> > ---
> > drivers/char/tpm/tpm-buf.c | 137 +++++++----
> > drivers/char/tpm/tpm-dev-common.c | 4 +-
> > drivers/char/tpm/tpm-dev.h | 2 +-
> > drivers/char/tpm/tpm-interface.c | 4 +-
> > drivers/char/tpm/tpm-sysfs.c | 20 +-
> > drivers/char/tpm/tpm.h | 3 +-
> > drivers/char/tpm/tpm1-cmd.c | 149 ++++++------
> > drivers/char/tpm/tpm2-cmd.c | 282 ++++++++++------------
> > drivers/char/tpm/tpm2-sessions.c | 121 +++++-----
> > drivers/char/tpm/tpm2-space.c | 44 ++--
> > drivers/char/tpm/tpm_tis_i2c.c | 4 +-
> > drivers/char/tpm/tpm_vtpm_proxy.c | 32 ++-
> > include/linux/tpm.h | 28 ++-
> > security/keys/trusted-keys/trusted_tpm1.c | 34 ++-
> > security/keys/trusted-keys/trusted_tpm2.c | 156 ++++++------
> > 15 files changed, 493 insertions(+), 527 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
> > index dc882fc9fa9e..82dce0350a41 100644
> > --- a/drivers/char/tpm/tpm-buf.c
> > +++ b/drivers/char/tpm/tpm-buf.c
> > @@ -7,82 +7,107 @@
> > #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;
> > + if (!buf->capacity) {
> > + if (buf_size > TPM_BUF_MAX_SIZE) {
> > + WARN(1, "%s: size overflow: %u\n", __func__, buf_size);
>
> IIUC here we will always print something like:
> "__tpm_buf_size_invariant: size overflow: XXX"
>
> So, should we just remove `__func__` or maybe use a macro to print the name
> of the caller?
It also prints a stack trace.
>
> > + buf->flags |= TPM_BUF_ERROR;
> > + }
> > + } else {
> > + if (buf_size != buf->capacity + sizeof(*buf)) {
> > + WARN(1, "%s: size mismatch: %u != %u\n", __func__, buf_size,
> > + buf->capacity + sizeof(*buf));
> > + buf->flags |= TPM_BUF_ERROR;
> > + }
> > + }
> > }
> > -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_ERROR)
> > + 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_ERROR)
> > + 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 +117,9 @@ EXPORT_SYMBOL_GPL(tpm_buf_destroy);
> > */
> > u32 tpm_buf_length(struct tpm_buf *buf)
> > {
> > + if (buf->flags & TPM_BUF_ERROR)
> > + return 0;
> > +
> > return buf->length;
> > }
> > EXPORT_SYMBOL_GPL(tpm_buf_length);
> > @@ -104,13 +132,14 @@ 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)
> > + u32 total_length = (u32)buf->length + (u32)new_length;
> > +
> > + if (buf->flags & TPM_BUF_ERROR)
> > 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_OVERFLOW;
> > + buf->flags |= TPM_BUF_ERROR;
> > return;
> > }
> >
> > @@ -157,8 +186,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_ERROR)
> > + return;
> > +
> > if (buf->flags & TPM_BUF_TPM2B) {
> > - dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n");
> > + dev_err(&chip->dev, "%s: invalid for buffer type: TPM2B\n", __func__);
> > + buf->flags |= TPM_BUF_ERROR;
> > return;
> > }
> >
> > @@ -178,13 +211,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_ERROR)
> > 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_ERROR;
> > return;
> > }
> >
> > @@ -242,5 +275,3 @@ u32 tpm_buf_read_u32(struct tpm_buf *buf, off_t *offset)
> > return be32_to_cpu(value);
> > }
> > EXPORT_SYMBOL_GPL(tpm_buf_read_u32);
> > -
> > -
> > diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
> > index f2a5e09257dd..4f5893555fb7 100644
> > --- a/drivers/char/tpm/tpm-dev-common.c
> > +++ b/drivers/char/tpm/tpm-dev-common.c
> > @@ -147,7 +147,7 @@ ssize_t tpm_common_read(struct file *file, char __user *buf,
> >
> > rc = copy_to_user(buf, priv->data_buffer + *off, ret_size);
> > if (rc) {
> > - memset(priv->data_buffer, 0, TPM_BUFSIZE);
> > + memset(priv->data_buffer, 0, TPM_BUF_MAX_SIZE);
> > priv->response_length = 0;
> > ret_size = -EFAULT;
> > } else {
> > @@ -173,7 +173,7 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
> > struct file_priv *priv = file->private_data;
> > int ret = 0;
> >
> > - if (size > TPM_BUFSIZE)
> > + if (size > TPM_BUF_MAX_SIZE)
> > return -E2BIG;
> >
> > mutex_lock(&priv->buffer_mutex);
> > diff --git a/drivers/char/tpm/tpm-dev.h b/drivers/char/tpm/tpm-dev.h
> > index f3742bcc73e3..700e3d9d8b64 100644
> > --- a/drivers/char/tpm/tpm-dev.h
> > +++ b/drivers/char/tpm/tpm-dev.h
> > @@ -18,7 +18,7 @@ struct file_priv {
> > bool response_read;
> > bool command_enqueued;
> >
> > - u8 data_buffer[TPM_BUFSIZE];
> > + u8 data_buffer[TPM_BUF_MAX_SIZE];
> > };
> >
> > void tpm_common_open(struct file *file, struct tpm_chip *chip,
> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > index c9f173001d0e..b0d5098fb92b 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -100,8 +100,8 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
> > if (bufsiz < TPM_HEADER_SIZE)
> > return -EINVAL;
> >
> > - if (bufsiz > TPM_BUFSIZE)
> > - bufsiz = TPM_BUFSIZE;
> > + if (bufsiz > TPM_BUF_MAX_SIZE)
> > + bufsiz = TPM_BUF_MAX_SIZE;
> >
> > count = be32_to_cpu(header->length);
> > ordinal = be32_to_cpu(header->ordinal);
> > diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> > index 94231f052ea7..4213a8285ed0 100644
> > --- a/drivers/char/tpm/tpm-sysfs.c
> > +++ b/drivers/char/tpm/tpm-sysfs.c
> > @@ -32,28 +32,30 @@ 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(PAGE_SIZE, GFP_KERNEL);
>
> We're using PAGE_SIZE instead of TPM_BUF_MAX_SIZE to reduce the pressure on
> the allocator, right?
>
> I was wondering if we could create an inline function or a macro that calls
> kzalloc() and tpm_buf_init().
> Just because we do it often, it's not a strong opinion, just something that
> came to mind while doing the review.
> I mean something like this (untested):
>
> struct tpm_buf *tpm_buf_alloc_max(void) {
> struct tpm_buf *buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> if (!buf)
> return NULL;
>
> tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
> return buf;
> }
With or without this, about same about of boiler plate is introduced to
the call site. It's easier to understand the code later on when kzalloc
alloc is clearly at site. Finally I think for most uses the driver could
favor stack over heap (but that requires prepare the tpm_buf like this).
>
> Thanks,
> Stefano
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v10 4/4] tpm_vpm_proxy: Use stack for TPM_CC_SET_LOCALITY
From: Jarkko Sakkinen @ 2025-09-23 14:30 UTC (permalink / raw)
To: Stefano Garzarella
Cc: linux-integrity, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <swkdunznhlk5atrdfsiud57uzcvkmknnarzfh6esq3xzy74tmk@2qqsv6tvdtlw>
On Mon, Sep 22, 2025 at 10:46:36AM +0200, Stefano Garzarella wrote:
> On Sun, Sep 21, 2025 at 05:08:04AM +0300, Jarkko Sakkinen wrote:
> > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> >
> > Use stack allocation for TPM_CC_SET_LOCALITY, as it has known fixed size.
> >
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > ---
> > drivers/char/tpm/tpm1-cmd.c | 2 +-
> > drivers/char/tpm/tpm_vtpm_proxy.c | 12 +++++-------
> > 2 files changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> > index 11c16ad9b2a7..433908cfb4a9 100644
> > --- a/drivers/char/tpm/tpm1-cmd.c
> > +++ b/drivers/char/tpm/tpm1-cmd.c
> > @@ -328,7 +328,7 @@ static int tpm1_startup(struct tpm_chip *chip)
> > int rc;
> >
> > dev_info(&chip->dev, "TPM_Startup\n");
> > - tpm_buf_init(buf, TPM_BUF_INT_SIZE);
> > + tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
>
> This change should be squashed in patch 2, right?
Yep, sure thanks :-)
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] tpm: Use -EPERM as fallback error code in tpm_ret_to_err
From: Jarkko Sakkinen @ 2025-09-23 14:35 UTC (permalink / raw)
To: Stefano Garzarella
Cc: linux-integrity, Jarkko Sakkinen, stable, Peter Huewe,
Jason Gunthorpe, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar, open list,
open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <tnxfamnvxoanaihka3em7ktmzkervoea43zn2l3mqxvnuivb6n@p5nn34vns3zf>
On Mon, Sep 22, 2025 at 11:25:42AM +0200, Stefano Garzarella wrote:
> On Mon, Sep 22, 2025 at 10:23:32AM +0300, Jarkko Sakkinen wrote:
> > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> >
> > Using -EFAULT here was not the best idea for tpm_ret_to_err as the fallback
> > error code as it is no concise with trusted keys.
> >
> > Change the fallback as -EPERM, process TPM_RC_HASH also in tpm_ret_to_err,
> > and by these changes make the helper applicable for trusted keys.
> >
> > Cc: stable@vger.kernel.org # v6.15+
> > Fixes: 539fbab37881 ("tpm: Mask TPM RC in tpm2_start_auth_session()")
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> > ---
> > include/linux/tpm.h | 9 +++++---
> > security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++-----------------
> > 2 files changed, 13 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> > index dc0338a783f3..667d290789ca 100644
> > --- a/include/linux/tpm.h
> > +++ b/include/linux/tpm.h
> > @@ -449,13 +449,16 @@ static inline ssize_t tpm_ret_to_err(ssize_t ret)
> > if (ret < 0)
> > return ret;
> >
> > - switch (tpm2_rc_value(ret)) {
> > - case TPM2_RC_SUCCESS:
>
> I slightly prefer the `case TPM2_RC_SUCCESS` but I don't have a strong
> opinion.
>
> > + if (!ret)
> > return 0;
>
> If we want to remove the `case TPM2_RC_SUCCESS`, can we just merge this
> condition with the if on top, I mean:
>
> if (ret <= 0)
> return ret;
I can cope with this i.e. revert back, it's not really part of the scope
and was totally intentional
>
> > +
> > + switch (tpm2_rc_value(ret)) {
> > case TPM2_RC_SESSION_MEMORY:
> > return -ENOMEM;
> > + case TPM2_RC_HASH:
> > + return -EINVAL;
> > default:
> > - return -EFAULT;
> > + return -EPERM;
> > }
> > }
> >
> > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> > index 024be262702f..e165b117bbca 100644
> > --- a/security/keys/trusted-keys/trusted_tpm2.c
> > +++ b/security/keys/trusted-keys/trusted_tpm2.c
> > @@ -348,25 +348,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)
>
> nit: since `blob_len` is not accessed anymore in the error path, can we
> avoid to set it to 0 when declaring it?
>
> Thanks,
> Stefano
>
> > - 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);
> > }
> >
> > /**
> > @@ -468,10 +462,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> > kfree(blob);
> > tpm_buf_destroy(&buf);
> >
> > - if (rc > 0)
> > - rc = -EPERM;
> > -
> > - return rc;
> > + return tpm_ret_to_err(rc);
> > }
> >
> > /**
> > @@ -534,8 +525,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(
> > @@ -568,7 +557,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
> >
> > out:
> > tpm_buf_destroy(&buf);
> > - return rc;
> > + return tpm_ret_to_err(rc);
> > }
> >
> > /**
> > @@ -600,6 +589,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
> >
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH 3/4] tpm2-sessions: Remove unnecessary wrapper
From: Jarkko Sakkinen @ 2025-09-23 14:45 UTC (permalink / raw)
To: list.lkml.keyrings
Cc: linux-integrity, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe,
David Howells, Paul Moore, James Morris, Serge E. Hallyn,
James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM
In-Reply-To: <aNGFv-nGZF5chGIb@rotor>
On Mon, Sep 22, 2025 at 01:22:13PM -0400, Ben Boeckel wrote:
> On Mon, Sep 22, 2025 at 19:43:16 +0300, Jarkko Sakkinen wrote:
> > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
> >
> > Open code tpm_buf_append_hmac_session_opt() because it adds unnecessary
> > disperancy to the call sites (and reduces the amount of code).
> ^^^^^^^^^^
>
> "discrepancy" as in "difference"? But that doesn't feel like the right
> usage either. Perhaps "unnecessary abstraction"? Also, open coding it
> reduces the amount of code, so some clarification to not read as
> something else that "it" (`tpm_buf_append_hmac_session_opt`) does would
> be clearer.
Fair points. I'll re-edit the commit message and try to address the
issues you reported.
Intend of these changes is to essentially uncover the code paths so that
we know how to wrap it up better than it is wrapped up right now. Also,
they help to reveal possible regression paths. So while not functional
per se, they do serve a purpose.
Once these fixes have been applied I'll start to look up the call
patterns and try to find a model where essentially we can transform
a TPM command to HMAC wrapped TPM command i.e., from tpm_buf to tpm_buf
operation where both sides of the function are TPM commands.
That way we can better selectively use the feature and it is easier
to fixup up e.g., a persistent parent key because key generation is
a huge bottleneck.
>
> Thanks,
>
> --Ben
^ permalink raw reply
* [PATCH v11 0/3] tpm: robust stack allocations
From: Jarkko Sakkinen @ 2025-09-23 17:07 UTC (permalink / raw)
To: linux-integrity
Cc: Stefano Garzarella, Jarkko Sakkinen, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
1. These are previous changes to tpm_buf, which make stack allocations
much more feasible than previously.
2. Migrate low-hanging fruit to use stack allocations.
3. Re-orchestrate tpm_get_random().
Jarkko Sakkinen (3):
tpm: Make TPM buffer allocations more robust
tpm: Use TPM_MIN_BUF_SIZE in driver commands
tpm orchestrate tpm_get_random() in the function
drivers/char/tpm/tpm-buf.c | 139 +++++----
drivers/char/tpm/tpm-dev-common.c | 4 +-
drivers/char/tpm/tpm-dev.h | 2 +-
drivers/char/tpm/tpm-interface.c | 36 ++-
drivers/char/tpm/tpm-sysfs.c | 20 +-
drivers/char/tpm/tpm.h | 16 +-
drivers/char/tpm/tpm1-cmd.c | 177 +++++------
drivers/char/tpm/tpm2-cmd.c | 349 +++++++++-------------
drivers/char/tpm/tpm2-sessions.c | 127 ++++----
drivers/char/tpm/tpm2-space.c | 44 ++-
drivers/char/tpm/tpm_tis_i2c.c | 4 +-
drivers/char/tpm/tpm_vtpm_proxy.c | 34 +--
include/linux/tpm.h | 28 +-
security/keys/trusted-keys/trusted_tpm1.c | 34 +--
security/keys/trusted-keys/trusted_tpm2.c | 156 +++++-----
15 files changed, 556 insertions(+), 614 deletions(-)
--
2.39.5
^ permalink raw reply
* [PATCH v11 1/3] tpm: Make TPM buffer allocations more robust
From: Jarkko Sakkinen @ 2025-09-23 17:07 UTC (permalink / raw)
To: linux-integrity
Cc: Stefano Garzarella, Jarkko Sakkinen, Stefan Berger, Peter Huewe,
Jarkko Sakkinen, Jason Gunthorpe, David Howells, Paul Moore,
James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM
In-Reply-To: <20250923170744.1749132-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Create more ergonomic primitives to work with TPM buffers, where
'tpm_buf_init*' initialize the memory and 'tpm_buf_reset*' (re)set a buffer
for a particular use and purpose. The new primitives are ubiquitos when
it comes to heap and stack usage.
Given that the kzalloc is decoupled, a managed allocation can be done
trivially:
struct tpm_buf *buf __free(kfree) buf = kzalloc(TPM_BUF_MAX_SIZE,
GFP_KERNEL);
This effectively zeros out the odds having any memory leaks with TPM
buffers. The new structures can be later used to widen the use of stack
over heap in the subsystem in the critical code paths..
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
v11
- No changes.
v10:
- Fix compiler warning about format string in __tpm_buf_size_invariant().
v9:
- Rename pre-existing TPM_BUFSIZE as TPM_BUF_MAX_SIZE and redeclare
it in include/linux/tpm.h, and use this constant instead of
PAGE_SIZE in tpm_buf_init*. Define TPM_BUF_MIN_SIZE to be a
sane placeholder value for stack allocations.
- Fix and restructure invariant for the sake of clarity.
- memset buffers to zero in tpm_buf_init* in order to guarantee a legit
initial state. This does not cause any regressions but once tpm_buf
is allocaed from stack at some site, this will zero out chance of
corrupted state.
v8:
- Decouple memory init i.e. bring tpm_init* back but with new fresh
form.
- Cap buf_size to page size and also make checks in tpm_buf_append
safe:
https://lore.kernel.org/linux-integrity/hwsx2t2tkbos3g7k2syemxbvc6sfrsbviwm64ubcdl6ms7ljvo@toetomkhheht/
- Fix trusted_tpm_send() and simplify the flow.
v7:
- Additional function comments and invariant was unfortunately left to
my staging area so here's the addition (does not affect functionality).
v6:
- Removed two empty lines as requested by Stefan:
https://lore.kernel.org/linux-integrity/be1c5bef-7c97-4173-b417-986dc90d779c@linux.ibm.com/
- Add 'capacity' field as this makes easy to stretch tpm_buf into stack
allocation.
v5:
- I tested this version also with TPM 1.2 by booting up and checking
that sysfs attributes work.
- Fixed the length check against capacity (James) with TPM_BUF_CAPACITY.
- Fixed declaration style: do it at the site (Jason).
- Improved commit message (Stefan).
- Removed "out" label from tpm2_pcr_read() (Stefan).
- Removed spurious "return rc;" from tpm2_get_pcr_allocation() (Stefan).
v4:
- Wrote a more a descriptive short summary and improved description.
- Fixed the error in documentation: there is 4090 bytes of space left
for the payload - not 4088 bytes.
- Turned tpm_buf_alloc() into inline function.
v3:
- Removed the cleanup class and moved on using __free(kfree) instead.
- Removed `buf_size` (James).
- I'm open for the idea of splitting still (Jason) but I'll hold
at least this revision just to check that my core idea here
is correct.
v2:
- Implement also memory allocation using the cleanup class.
- Rewrote the commit message.
- Implemented CLASS_TPM_BUF() helper macro.
---
drivers/char/tpm/tpm-buf.c | 139 +++++++----
drivers/char/tpm/tpm-dev-common.c | 4 +-
drivers/char/tpm/tpm-dev.h | 2 +-
drivers/char/tpm/tpm-interface.c | 4 +-
drivers/char/tpm/tpm-sysfs.c | 20 +-
drivers/char/tpm/tpm.h | 3 +-
drivers/char/tpm/tpm1-cmd.c | 149 ++++++------
drivers/char/tpm/tpm2-cmd.c | 282 ++++++++++------------
drivers/char/tpm/tpm2-sessions.c | 121 +++++-----
drivers/char/tpm/tpm2-space.c | 44 ++--
drivers/char/tpm/tpm_tis_i2c.c | 4 +-
drivers/char/tpm/tpm_vtpm_proxy.c | 32 ++-
include/linux/tpm.h | 28 ++-
security/keys/trusted-keys/trusted_tpm1.c | 34 ++-
security/keys/trusted-keys/trusted_tpm2.c | 156 ++++++------
15 files changed, 495 insertions(+), 527 deletions(-)
diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c
index dc882fc9fa9e..b45bf12f7d45 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_BUF_MAX_SIZE) {
+ WARN(1, "%s: size overflow: %u\n", __func__, buf_size);
+ buf->flags |= TPM_BUF_ERROR;
+ }
+ } 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_ERROR;
+ }
+ }
}
-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_ERROR)
+ 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_ERROR)
+ 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_ERROR)
+ return 0;
+
return buf->length;
}
EXPORT_SYMBOL_GPL(tpm_buf_length);
@@ -104,13 +134,14 @@ 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)
+ u32 total_length = (u32)buf->length + (u32)new_length;
+
+ if (buf->flags & TPM_BUF_ERROR)
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_OVERFLOW;
+ buf->flags |= TPM_BUF_ERROR;
return;
}
@@ -157,8 +188,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_ERROR)
+ return;
+
if (buf->flags & TPM_BUF_TPM2B) {
- dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n");
+ dev_err(&chip->dev, "%s: invalid for buffer type: TPM2B\n", __func__);
+ buf->flags |= TPM_BUF_ERROR;
return;
}
@@ -178,13 +213,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_ERROR)
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_ERROR;
return;
}
@@ -242,5 +277,3 @@ u32 tpm_buf_read_u32(struct tpm_buf *buf, off_t *offset)
return be32_to_cpu(value);
}
EXPORT_SYMBOL_GPL(tpm_buf_read_u32);
-
-
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index f2a5e09257dd..4f5893555fb7 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -147,7 +147,7 @@ ssize_t tpm_common_read(struct file *file, char __user *buf,
rc = copy_to_user(buf, priv->data_buffer + *off, ret_size);
if (rc) {
- memset(priv->data_buffer, 0, TPM_BUFSIZE);
+ memset(priv->data_buffer, 0, TPM_BUF_MAX_SIZE);
priv->response_length = 0;
ret_size = -EFAULT;
} else {
@@ -173,7 +173,7 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
struct file_priv *priv = file->private_data;
int ret = 0;
- if (size > TPM_BUFSIZE)
+ if (size > TPM_BUF_MAX_SIZE)
return -E2BIG;
mutex_lock(&priv->buffer_mutex);
diff --git a/drivers/char/tpm/tpm-dev.h b/drivers/char/tpm/tpm-dev.h
index f3742bcc73e3..700e3d9d8b64 100644
--- a/drivers/char/tpm/tpm-dev.h
+++ b/drivers/char/tpm/tpm-dev.h
@@ -18,7 +18,7 @@ struct file_priv {
bool response_read;
bool command_enqueued;
- u8 data_buffer[TPM_BUFSIZE];
+ u8 data_buffer[TPM_BUF_MAX_SIZE];
};
void tpm_common_open(struct file *file, struct tpm_chip *chip,
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index c9f173001d0e..b0d5098fb92b 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -100,8 +100,8 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
if (bufsiz < TPM_HEADER_SIZE)
return -EINVAL;
- if (bufsiz > TPM_BUFSIZE)
- bufsiz = TPM_BUFSIZE;
+ if (bufsiz > TPM_BUF_MAX_SIZE)
+ bufsiz = TPM_BUF_MAX_SIZE;
count = be32_to_cpu(header->length);
ordinal = be32_to_cpu(header->ordinal);
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 94231f052ea7..4213a8285ed0 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -32,28 +32,30 @@ 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(PAGE_SIZE, 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_BUF_MAX_SIZE);
+ 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,
+ if (tpm_transmit_cmd(chip, tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE,
"attempting to read the PUBEK"))
- goto out_buf;
+ 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 +73,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 2726bd38e5ac..1a4abe54db15 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
@@ -224,7 +223,7 @@ enum tpm2_pt_props {
TPM2_PT_AUDIT_COUNTER_1 = TPM2_PT_VAR + 20,
};
-/* 128 bytes is an arbitrary cap. This could be as large as TPM_BUFSIZE - 18
+/* 128 bytes is an arbitrary cap. This could be as large as TPM_BUF_MAX_SIZE - 18
* bytes, but 128 is still a relatively large number of random bytes and
* anything much bigger causes users of struct tpm_cmd_t to start getting
* compiler warnings about stack frame size. */
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index cf64c7385105..ca3e7f9a105d 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -323,19 +323,19 @@ 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");
+ struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_STARTUP);
- if (rc < 0)
- return rc;
+ dev_info(&chip->dev, "starting up the TPM manually\n");
- tpm_buf_append_u16(&buf, TPM_ST_CLEAR);
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_STARTUP);
+ tpm_buf_append_u16(buf, TPM_ST_CLEAR);
- rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to start the TPM");
- tpm_buf_destroy(&buf);
+ rc = tpm_transmit_cmd(chip, buf, 0, "attempting to start the TPM");
return rc;
}
@@ -463,18 +463,18 @@ 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(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- tpm_buf_append_u32(&buf, pcr_idx);
- tpm_buf_append(&buf, hash, TPM_DIGEST_SIZE);
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND);
+ 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);
+ rc = tpm_transmit_cmd(chip, buf, TPM_DIGEST_SIZE, log_msg);
return rc;
}
@@ -482,31 +482,32 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
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(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ 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 +532,71 @@ 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(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
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),
+ rc = tpm_transmit_cmd(chip, buf, sizeof(out->rng_data_len),
"attempting get random");
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(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- tpm_buf_append_u32(&buf, pcr_idx);
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ 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,
+ rc = tpm_transmit_cmd(chip, buf, TPM_DIGEST_SIZE,
"attempting to read a pcr value");
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,15 +610,15 @@ 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(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- rc = tpm_transmit_cmd(chip, &buf, 0, "continue selftest");
- tpm_buf_destroy(&buf);
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_CONTINUE_SELFTEST);
+ rc = tpm_transmit_cmd(chip, buf, 0, "continue selftest");
return rc;
}
@@ -742,22 +733,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(PAGE_SIZE, 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_BUF_MAX_SIZE);
+ 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 +765,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,8 +775,6 @@ 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 7d77f6fbc152..245c7c952e82 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(PAGE_SIZE, 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_BUF_MAX_SIZE);
+ 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, "attempting to read a pcr value");
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,43 @@ 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(PAGE_SIZE, 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_BUF_MAX_SIZE);
+ 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(chip, &buf, pcr_idx);
- tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
+ tpm_buf_append_handle(chip, buf, pcr_idx);
+ tpm_buf_append_auth(chip, buf, 0, 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 (!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, "attempting extend a PCR value");
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 +228,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,43 +239,42 @@ 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(PAGE_SIZE, 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_BUF_MAX_SIZE);
do {
- tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
- tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT
+ 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);
- 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);
+ 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) {
@@ -295,11 +288,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;
}
@@ -311,20 +302,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(PAGE_SIZE, 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_BUF_MAX_SIZE);
+ 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, "flushing context");
}
EXPORT_SYMBOL_GPL(tpm2_flush_context);
@@ -351,19 +340,21 @@ 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(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ 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];
+ &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
@@ -375,7 +366,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);
@@ -392,15 +382,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(PAGE_SIZE, 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_BUF_MAX_SIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
+ tpm_buf_append_u16(buf, shutdown_type);
+ tpm_transmit_cmd(chip, buf, 0, "stopping the TPM");
}
/**
@@ -418,19 +407,20 @@ 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;
+ struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
- tpm_buf_append_u8(&buf, full);
- rc = tpm_transmit_cmd(chip, &buf, 0,
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ 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,
"attempting the self test");
- tpm_buf_destroy(&buf);
if (rc == TPM2_RC_TESTING)
rc = TPM2_RC_SUCCESS;
@@ -456,23 +446,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(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ 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);
@@ -512,7 +503,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;
@@ -524,41 +514,39 @@ 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(PAGE_SIZE, 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_BUF_MAX_SIZE);
+ 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, "get tpm pcr allocation");
if (rc)
- goto out;
+ return rc;
nr_possible_banks = be32_to_cpup(
- (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]);
+ (__be32 *)&buf->data[TPM_HEADER_SIZE + 5]);
chip->allocated_banks = kcalloc(nr_possible_banks,
sizeof(*chip->allocated_banks),
GFP_KERNEL);
- if (!chip->allocated_banks) {
- rc = -ENOMEM;
- goto out;
- }
+ if (!chip->allocated_banks)
+ 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);
@@ -570,7 +558,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++;
}
@@ -582,21 +570,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(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
rc = tpm2_get_tpm_pt(chip, TPM_PT_TOTAL_COMMANDS, &nr_commands, NULL);
if (rc)
goto out;
@@ -613,30 +601,25 @@ 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_BUF_MAX_SIZE);
+ 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])) {
+ 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;
@@ -648,8 +631,6 @@ int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
}
}
- tpm_buf_destroy(&buf);
-
out:
if (rc > 0)
rc = -ENODEV;
@@ -670,20 +651,17 @@ EXPORT_SYMBOL_GPL(tpm2_get_cc_attrs_tbl);
static int tpm2_startup(struct tpm_chip *chip)
{
- struct tpm_buf buf;
- int rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
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);
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_STARTUP);
+ tpm_buf_append_u16(buf, TPM2_SU_CLEAR);
- return rc;
+ return tpm_transmit_cmd(chip, buf, 0, "attempting to start the TPM");
}
/**
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index 6d03c224e6b2..fd0f56e0018b 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(PAGE_SIZE, 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_BUF_MAX_SIZE);
+ 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, "read public");
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;
}
@@ -925,7 +924,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;
@@ -934,6 +932,10 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
return 0;
}
+ struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
auth = kzalloc(sizeof(*auth), GFP_KERNEL);
if (!auth)
return -ENOMEM;
@@ -944,41 +946,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_BUF_MAX_SIZE);
+ 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, "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;
@@ -1200,18 +1198,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(PAGE_SIZE, 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(PAGE_SIZE, GFP_KERNEL);
+ if (!template)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE_PRIMARY);
+ tpm_buf_init_sized(template, TPM_BUF_MAX_SIZE);
/*
* create the template. Note: in order for userspace to
@@ -1223,75 +1221,72 @@ 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,
+ rc = tpm_transmit_cmd(chip, buf, 0,
"attempting to create NULL primary");
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..c52040fcd724 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(PAGE_SIZE, GFP_KERNEL);
+ if (!tbuf)
+ return -ENOMEM;
+
+ tpm_buf_init(tbuf, TPM_BUF_MAX_SIZE);
+ 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(PAGE_SIZE, GFP_KERNEL);
+ if (!tbuf)
+ return -ENOMEM;
- tpm_buf_append_u32(&tbuf, handle);
+ tpm_buf_init(tbuf, TPM_BUF_MAX_SIZE);
+ 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_tis_i2c.c b/drivers/char/tpm/tpm_tis_i2c.c
index 6cd07dd34507..5832713b1a2c 100644
--- a/drivers/char/tpm/tpm_tis_i2c.c
+++ b/drivers/char/tpm/tpm_tis_i2c.c
@@ -232,7 +232,7 @@ static int tpm_tis_i2c_write_bytes(struct tpm_tis_data *data, u32 addr, u16 len,
int ret;
u16 wrote = 0;
- if (len > TPM_BUFSIZE - 1)
+ if (len > TPM_BUF_MAX_SIZE - 1)
return -EIO;
phy->io_buf[0] = reg;
@@ -339,7 +339,7 @@ static int tpm_tis_i2c_probe(struct i2c_client *dev)
if (!phy)
return -ENOMEM;
- phy->io_buf = devm_kzalloc(&dev->dev, TPM_BUFSIZE, GFP_KERNEL);
+ phy->io_buf = devm_kzalloc(&dev->dev, TPM_BUF_MAX_SIZE, GFP_KERNEL);
if (!phy->io_buf)
return -ENOMEM;
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 0818bb517805..e5de14379eb2 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -42,7 +42,7 @@ struct proxy_dev {
size_t req_len; /* length of queued TPM request */
size_t resp_len; /* length of queued TPM response */
- u8 buffer[TPM_BUFSIZE]; /* request/response buffer */
+ u8 buffer[TPM_BUF_MAX_SIZE]; /* request/response buffer */
struct work_struct work; /* task that retrieves TPM timeouts */
};
@@ -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(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
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 dc0338a783f3..a0ce1b565769 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -367,22 +367,25 @@ 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),
+ /* TPM2B format */
+ TPM_BUF_TPM2B = BIT(1),
+ /* overflow or underrun error */
+ TPM_BUF_ERROR = BIT(2),
};
+#define TPM_BUF_MIN_SIZE 256 /* size for stack allocations */
+#define TPM_BUF_MAX_SIZE 4096 /* size for heap allocations */
+
/*
- * 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..fec7b6de7887 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(PAGE_SIZE, GFP_KERNEL);
+ if (!tb)
+ return -ENOMEM;
+
+ tpm_buf_init(tb, TPM_BUF_MAX_SIZE);
/* 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(PAGE_SIZE, GFP_KERNEL);
+ if (!tb)
+ return -ENOMEM;
+
+ tpm_buf_init(tb, TPM_BUF_MAX_SIZE);
- 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 024be262702f..7b43535f1a65 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -242,13 +242,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(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ struct tpm_buf *sized __free(kfree) = kzalloc(PAGE_SIZE, 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;
@@ -270,89 +277,77 @@ 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_BUF_MAX_SIZE);
+ tpm_buf_init_sized(sized, TPM_BUF_MAX_SIZE);
+ 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_BUF_MAX_SIZE);
+ 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_OVERFLOW) {
+ if (buf->flags & TPM_BUF_ERROR) {
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_BOUNDARY_ERROR) {
+ blob_len = tpm_buf_read_u32(buf, &offset);
+ if (blob_len > MAX_BLOB_SIZE || buf->flags & TPM_BUF_ERROR) {
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);
out:
- tpm_buf_destroy(&sized);
- tpm_buf_destroy(&buf);
-
if (rc > 0) {
if (tpm2_rc_value(rc) == TPM2_RC_HASH)
rc = -EINVAL;
@@ -387,7 +382,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;
@@ -395,6 +389,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
int rc;
u32 attrs;
+ struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
rc = tpm2_key_decode(payload, options, &blob);
if (rc) {
/* old form */
@@ -438,35 +436,30 @@ 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;
- }
-
- tpm_buf_append_name(chip, &buf, options->keyhandle, NULL);
- tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth,
+ tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ 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_OVERFLOW) {
+ if (buf->flags & TPM_BUF_ERROR) {
rc = -E2BIG;
tpm2_end_auth_session(chip);
goto out;
}
- 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]);
+ (__be32 *)&buf->data[TPM_HEADER_SIZE]);
out:
if (blob != payload->blob)
kfree(blob);
- tpm_buf_destroy(&buf);
if (rc > 0)
rc = -EPERM;
@@ -491,25 +484,24 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
struct trusted_key_options *options,
u32 blob_handle)
{
- struct tpm_buf buf;
u16 data_len;
u8 *data;
int rc;
+ struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, 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_BUF_MAX_SIZE);
+ 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 {
@@ -524,32 +516,28 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
* could repeat our actions with the exfiltrated
* password.
*/
- tpm2_buf_append_auth(&buf, options->policyhandle,
+ 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,
+ tpm_buf_append_hmac_session_opt(chip, buf, TPM2_SA_ENCRYPT,
NULL, 0);
}
- 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 > 0)
rc = -EPERM;
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 */
@@ -566,8 +554,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
}
}
-out:
- tpm_buf_destroy(&buf);
return rc;
}
--
2.39.5
^ permalink raw reply related
* [PATCH v11 2/3] tpm: Use TPM_MIN_BUF_SIZE in driver commands
From: Jarkko Sakkinen @ 2025-09-23 17:07 UTC (permalink / raw)
To: linux-integrity
Cc: Stefano Garzarella, Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen,
Jason Gunthorpe, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20250923170744.1749132-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
Use TPM_BUF_MIN_SIZE and stack allocation in the following commands:
1. tpm1_startup
2. tpm2_shutdown
3. tpm1_get_random
4. tpm2_do_selftest
5. tpm2_probe
6. tpm2_startup
7. tpm2_get_random
8. vtpm_proxy_request_locality
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
v11:
- A new patch that squashes previous stack updates. It's in the end of
day easier review like this. Thus, also the sanity check list in the
commit message in order to catch up unintended changes if there were
any.
---
drivers/char/tpm/tpm.h | 9 ++---
drivers/char/tpm/tpm1-cmd.c | 22 ++++-------
drivers/char/tpm/tpm2-cmd.c | 65 ++++++++++++-------------------
drivers/char/tpm/tpm_vtpm_proxy.c | 12 +++---
4 files changed, 42 insertions(+), 66 deletions(-)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1a4abe54db15..df426e281f88 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -223,11 +223,10 @@ enum tpm2_pt_props {
TPM2_PT_AUDIT_COUNTER_1 = TPM2_PT_VAR + 20,
};
-/* 128 bytes is an arbitrary cap. This could be as large as TPM_BUF_MAX_SIZE - 18
- * bytes, but 128 is still a relatively large number of random bytes and
- * anything much bigger causes users of struct tpm_cmd_t to start getting
- * compiler warnings about stack frame size. */
-#define TPM_MAX_RNG_DATA 128
+/*
+ * The parameter is capped to fit to a stack allocated TPM buffer.
+ */
+#define TPM_MAX_RNG_DATA (TPM_BUF_MIN_SIZE / 2)
extern const struct class tpm_class;
extern const struct class tpmrm_class;
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index ca3e7f9a105d..fdb55dab80e1 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -323,19 +323,15 @@ unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
*/
static int tpm1_startup(struct tpm_chip *chip)
{
+ u8 buf_data[TPM_BUF_MIN_SIZE];
+ struct tpm_buf *buf = (struct tpm_buf *)buf_data;
int rc;
- struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
- dev_info(&chip->dev, "starting up the TPM manually\n");
-
- tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ dev_info(&chip->dev, "TPM_Startup\n");
+ tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_STARTUP);
tpm_buf_append_u16(buf, TPM_ST_CLEAR);
-
- rc = tpm_transmit_cmd(chip, buf, 0, "attempting to start the TPM");
+ rc = tpm_transmit_cmd(chip, buf, 0, "TPM_Startup");
return rc;
}
@@ -530,6 +526,8 @@ struct tpm1_get_random_out {
*/
int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
{
+ u8 buf_data[TPM_BUF_MIN_SIZE];
+ struct tpm_buf *buf = (struct tpm_buf *)buf_data;
struct tpm1_get_random_out *out;
u32 num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
u32 total = 0;
@@ -537,11 +535,7 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
u32 recd;
int rc;
- struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
- tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
do {
tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_RANDOM);
tpm_buf_append_u32(buf, num_bytes);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 245c7c952e82..320dc6ae6af2 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -226,6 +226,8 @@ struct tpm2_get_random_out {
*/
int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
{
+ u8 buf_data[TPM_BUF_MIN_SIZE];
+ struct tpm_buf *buf = (struct tpm_buf *)buf_data;
struct tpm2_get_random_out *out;
struct tpm_header *head;
u32 recd;
@@ -239,15 +241,11 @@ 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(PAGE_SIZE, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
err = tpm2_start_auth_session(chip);
if (err)
return err;
- tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
do {
tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
tpm_buf_append_hmac_session_opt(chip, buf, TPM2_SA_ENCRYPT
@@ -382,14 +380,13 @@ EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt);
*/
void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
{
- struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
- if (!buf)
- return;
+ u8 buf_data[TPM_BUF_MIN_SIZE];
+ struct tpm_buf *buf = (struct tpm_buf *)buf_data;
- tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN);
tpm_buf_append_u16(buf, shutdown_type);
- tpm_transmit_cmd(chip, buf, 0, "stopping the TPM");
+ tpm_transmit_cmd(chip, buf, 0, "TPM2_Shutdown");
}
/**
@@ -407,58 +404,49 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
*/
static int tpm2_do_selftest(struct tpm_chip *chip)
{
+ u8 buf_data[TPM_BUF_MIN_SIZE];
+ struct tpm_buf *buf = (struct tpm_buf *)buf_data;
int full;
int rc;
- struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
- tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
- tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST);
+ tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
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,
- "attempting the self test");
-
+ 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)
return rc;
}
-
return rc;
}
/**
- * tpm2_probe() - probe for the TPM 2.0 protocol
+ * tpm2_probe() - Probe for the TPM 2.0 protocol
* @chip: a &tpm_chip instance
*
- * Send an idempotent TPM 2.0 command and see whether there is TPM2 chip in the
- * other end based on the response tag. The flag TPM_CHIP_FLAG_TPM2 is set by
- * this function if this is the case.
+ * Sends an idempotent TPM 2.0 command, and based on the response tag deduces
+ * whether a functional TPM2 chip is on the other side. When the result is
+ * positive, TPM_CHIP_FLAG_TPM2 is append to the chip's flags.
*
* Return:
- * 0 on success,
- * -errno otherwise
+ * * 0 on success,
+ * * -errno otherwise
*/
int tpm2_probe(struct tpm_chip *chip)
{
+ u8 buf_data[TPM_BUF_MIN_SIZE];
+ struct tpm_buf *buf = (struct tpm_buf *)buf_data;
struct tpm_header *out;
int rc;
- struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
- tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
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;
if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS)
@@ -651,17 +639,14 @@ EXPORT_SYMBOL_GPL(tpm2_get_cc_attrs_tbl);
static int tpm2_startup(struct tpm_chip *chip)
{
- struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
+ u8 buf_data[TPM_BUF_MIN_SIZE];
+ struct tpm_buf *buf = (struct tpm_buf *)buf_data;
- dev_info(&chip->dev, "starting up the TPM manually\n");
-
- tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ dev_info(&chip->dev, "TPM2_Startup\n");
+ tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
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, "attempting to start the TPM");
+ return tpm_transmit_cmd(chip, buf, 0, "TPM2_Startup");
}
/**
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index e5de14379eb2..0f1b1b67ed4e 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -395,15 +395,13 @@ 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)
{
- int rc;
- const struct tpm_header *header;
struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev);
+ u8 buf_data[TPM_BUF_MIN_SIZE];
+ struct tpm_buf *buf = (struct tpm_buf *)buf_data;
+ const struct tpm_header *header;
+ int rc;
- struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
- tpm_buf_init(buf, TPM_BUF_MAX_SIZE);
+ tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
if (chip->flags & TPM_CHIP_FLAG_TPM2)
tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_SET_LOCALITY);
else
--
2.39.5
^ permalink raw reply related
* [PATCH v11 3/3] tpm orchestrate tpm_get_random() in the function
From: Jarkko Sakkinen @ 2025-09-23 17:07 UTC (permalink / raw)
To: linux-integrity
Cc: Stefano Garzarella, Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen,
Jason Gunthorpe, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM
In-Reply-To: <20250923170744.1749132-1-jarkko@kernel.org>
From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
tpm1_get_random() and tpm2_get_random() repeat the same orchestration.
Move the orchestration into tpm_get_random(). This is especially
useful for TPM 1.2 as the code for that does not get exercised a
lot these days.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com>
---
v11:
- A new patch.
---
drivers/char/tpm/tpm-interface.c | 32 ++++++++++--
drivers/char/tpm/tpm.h | 6 ++-
drivers/char/tpm/tpm1-cmd.c | 54 +++++++++-----------
drivers/char/tpm/tpm2-cmd.c | 84 ++++++++++++--------------------
drivers/char/tpm/tpm2-sessions.c | 6 +++
5 files changed, 90 insertions(+), 92 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index b0d5098fb92b..0c6d25cf7cac 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -487,22 +487,44 @@ EXPORT_SYMBOL_GPL(tpm_pm_resume);
*/
int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
{
+ u8 buf_data[TPM_BUF_MIN_SIZE];
+ struct tpm_buf *buf = (struct tpm_buf *)buf_data;
+ u32 num_bytes = max;
+ u8 *dest_ptr = out;
+ int retries = 5;
+ int total = 0;
int rc;
if (!out || max > TPM_MAX_RNG_DATA)
return -EINVAL;
+ tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
+
chip = tpm_find_get_ops(chip);
if (!chip)
return -ENODEV;
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
- rc = tpm2_get_random(chip, out, max);
- else
- rc = tpm1_get_random(chip, out, max);
+ rc = tpm2_start_auth_session(chip);
+ if (rc)
+ return rc;
+ do {
+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ rc = tpm2_get_random(chip, buf, out, max);
+ else
+ rc = tpm1_get_random(chip, buf, out, max);
+
+ if (rc < 0)
+ break;
+
+ dest_ptr += rc;
+ total += rc;
+ num_bytes -= rc;
+ } while (retries-- && total < max);
+
+ tpm2_end_auth_session(chip);
tpm_put_ops(chip);
- return rc;
+ return total ? total : -EIO;
}
EXPORT_SYMBOL_GPL(tpm_get_random);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index df426e281f88..0dc2397bc55a 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -249,7 +249,8 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
const char *desc, size_t min_cap_length);
-int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max);
+int tpm1_get_random(struct tpm_chip *chip, struct tpm_buf *buf, u8 *out,
+ size_t max);
int tpm1_get_pcr_allocation(struct tpm_chip *chip);
unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
int tpm_pm_suspend(struct device *dev);
@@ -290,7 +291,8 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digest, u16 *digest_size_ptr);
int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
struct tpm_digest *digests);
-int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
+int tpm2_get_random(struct tpm_chip *chip, struct tpm_buf *buf, u8 *dest,
+ size_t max);
ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
u32 *value, const char *desc);
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index fdb55dab80e1..fa807e0f335c 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -517,6 +517,7 @@ struct tpm1_get_random_out {
/**
* tpm1_get_random() - get random bytes from the TPM's RNG
* @chip: a &struct tpm_chip instance
+ * @buf: a &tpm_buf instance
* @dest: destination buffer for the random bytes
* @max: the maximum number of bytes to write to @dest
*
@@ -524,49 +525,38 @@ struct tpm1_get_random_out {
* * number of bytes read
* * -errno (positive TPM return codes are masked to -EIO)
*/
-int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
+int tpm1_get_random(struct tpm_chip *chip, struct tpm_buf *buf, u8 *dest,
+ size_t max)
{
- u8 buf_data[TPM_BUF_MIN_SIZE];
- struct tpm_buf *buf = (struct tpm_buf *)buf_data;
struct tpm1_get_random_out *out;
- u32 num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA);
- u32 total = 0;
- int retries = 5;
u32 recd;
int rc;
- tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
- do {
- 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");
- if (rc) {
- if (rc > 0)
- rc = -EIO;
- return rc;
- }
+ if (!max || max > TPM_MAX_RNG_DATA)
+ return -EINVAL;
- out = (struct tpm1_get_random_out *)&buf->data[TPM_HEADER_SIZE];
+ tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_RANDOM);
+ tpm_buf_append_u32(buf, max);
- recd = be32_to_cpu(out->rng_data_len);
- if (recd > num_bytes)
- return -EFAULT;
+ rc = tpm_transmit_cmd(chip, buf, sizeof(out->rng_data_len),
+ "attempting get random");
+ if (rc) {
+ if (rc > 0)
+ rc = -EIO;
+ return rc;
+ }
- if (buf->length < TPM_HEADER_SIZE +
- sizeof(out->rng_data_len) + recd)
- return -EFAULT;
+ out = (struct tpm1_get_random_out *)&buf->data[TPM_HEADER_SIZE];
- memcpy(dest, out->rng_data, recd);
+ recd = be32_to_cpu(out->rng_data_len);
+ if (recd > max)
+ return -EFAULT;
- dest += recd;
- total += recd;
- num_bytes -= recd;
- } while (retries-- && total < max);
+ if (buf->length < TPM_HEADER_SIZE + sizeof(out->rng_data_len) + recd)
+ return -EFAULT;
- rc = total ? (int)total : -EIO;
- return rc;
+ memcpy(dest, out->rng_data, recd);
+ return recd ? recd : -EIO;
}
#define TPM_ORD_PCRREAD 21
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 320dc6ae6af2..c98873f4bee6 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -217,6 +217,7 @@ struct tpm2_get_random_out {
* tpm2_get_random() - get random bytes from the TPM RNG
*
* @chip: a &tpm_chip instance
+ * @buf: a &tpm_buf instance
* @dest: destination buffer
* @max: the max number of random bytes to pull
*
@@ -224,73 +225,50 @@ struct tpm2_get_random_out {
* size of the buffer on success,
* -errno otherwise (positive TPM return codes are masked to -EIO)
*/
-int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
+int tpm2_get_random(struct tpm_chip *chip, struct tpm_buf *buf, u8 *dest,
+ size_t max)
{
- u8 buf_data[TPM_BUF_MIN_SIZE];
- struct tpm_buf *buf = (struct tpm_buf *)buf_data;
struct tpm2_get_random_out *out;
struct tpm_header *head;
u32 recd;
- u32 num_bytes = max;
int err;
- int total = 0;
- int retries = 5;
- u8 *dest_ptr = dest;
off_t offset;
- if (!num_bytes || max > TPM_MAX_RNG_DATA)
+ if (!max || max > TPM_MAX_RNG_DATA)
return -EINVAL;
- err = tpm2_start_auth_session(chip);
- if (err)
+ 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);
+ tpm_buf_append_u16(buf, max);
+ 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);
+ if (err) {
+ if (err > 0)
+ err = -EIO;
return err;
+ }
- tpm_buf_init(buf, TPM_BUF_MIN_SIZE);
- 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);
- 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);
- if (err) {
- if (err > 0)
- err = -EIO;
- goto out;
- }
-
- 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];
- recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
- if (tpm_buf_length(buf) <
- TPM_HEADER_SIZE +
- offsetof(struct tpm2_get_random_out, buffer) +
- recd) {
- err = -EFAULT;
- goto out;
- }
- memcpy(dest_ptr, out->buffer, recd);
+ head = (struct tpm_header *)buf->data;
+ offset = TPM_HEADER_SIZE;
- dest_ptr += recd;
- total += recd;
- num_bytes -= recd;
- } while (retries-- && total < max);
+ /* Skip the parameter size field: */
+ if (be16_to_cpu(head->tag) == TPM2_ST_SESSIONS)
+ offset += 4;
- return total ? total : -EIO;
+ out = (struct tpm2_get_random_out *)&buf->data[offset];
+ recd = min_t(u32, be16_to_cpu(out->size), max);
+ if (tpm_buf_length(buf) <
+ TPM_HEADER_SIZE +
+ offsetof(struct tpm2_get_random_out, buffer) +
+ recd)
+ return -EFAULT;
-out:
- tpm2_end_auth_session(chip);
- return err;
+ memcpy(dest, out->buffer, recd);
+ return recd ? recd : -EIO;
}
/**
diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index fd0f56e0018b..654b3e326292 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -837,6 +837,9 @@ void tpm2_end_auth_session(struct tpm_chip *chip)
{
struct tpm2_auth *auth = chip->auth;
+ if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+ return;
+
if (!auth)
return;
@@ -927,6 +930,9 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
u32 null_key;
int rc;
+ if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+ return 0;
+
if (chip->auth) {
dev_dbg_once(&chip->dev, "auth session is active\n");
return 0;
--
2.39.5
^ permalink raw reply related
* Re: [PATCH v3 00/35] Compiler-Based Capability- and Locking-Analysis
From: Nathan Chancellor @ 2025-09-23 19:49 UTC (permalink / raw)
To: Marco Elver
Cc: Christoph Hellwig, Peter Zijlstra, Boqun Feng, Ingo Molnar,
Will Deacon, David S. Miller, Luc Van Oostenryck,
Paul E. McKenney, Alexander Potapenko, Arnd Bergmann,
Bart Van Assche, Bill Wendling, Dmitry Vyukov, Eric Dumazet,
Frederic Weisbecker, Greg Kroah-Hartman, Herbert Xu, Ian Rogers,
Jann Horn, Joel Fernandes, Jonathan Corbet, Josh Triplett,
Justin Stitt, Kees Cook, Kentaro Takeda, Lukas Bulwahn,
Mark Rutland, Mathieu Desnoyers, Miguel Ojeda, Neeraj Upadhyay,
Nick Desaulniers, Steven Rostedt, Tetsuo Handa, Thomas Gleixner,
Thomas Graf, Uladzislau Rezki, Waiman Long, kasan-dev,
linux-crypto, linux-doc, linux-kbuild, linux-kernel, linux-mm,
linux-security-module, linux-sparse, llvm, rcu
In-Reply-To: <aNEX46WJh2IWhVUc@elver.google.com>
On Mon, Sep 22, 2025 at 11:33:23AM +0200, Marco Elver wrote:
> [1] https://github.com/llvm/llvm-project/pull/159921
Now that this is merged, I have pushed an updated snapshot for x86_64:
https://mirrors.edge.kernel.org/pub/tools/llvm/files/prerelease/llvm-22.0.0-ca2e8fc928ad103f46ca9f827e147c43db3a5c47-20250923-185804-x86_64.tar.xz
Cheers,
Nathan
^ permalink raw reply
* Re: [PATCH v11 0/3] tpm: robust stack allocations
From: Jarkko Sakkinen @ 2025-09-24 1:31 UTC (permalink / raw)
To: linux-integrity
Cc: Stefano Garzarella, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20250923170744.1749132-1-jarkko@kernel.org>
On Tue, Sep 23, 2025 at 08:07:41PM +0300, Jarkko Sakkinen wrote:
> 1. These are previous changes to tpm_buf, which make stack allocations
> much more feasible than previously.
> 2. Migrate low-hanging fruit to use stack allocations.
> 3. Re-orchestrate tpm_get_random().
>
> Jarkko Sakkinen (3):
> tpm: Make TPM buffer allocations more robust
> tpm: Use TPM_MIN_BUF_SIZE in driver commands
> tpm orchestrate tpm_get_random() in the function
I'm quite soon sending v12 because I randomly noticed a resource
over-cosumption bug. which could be theoretically triggered e.g.
via interposing a bus or a faulty device: tpm2_get_pcr_allocation()
does not have a hard limit for the number of PCR banks.
The fix:
https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?h=tpm-buf&id=3d92f14c204d09babadaa0b7c7a82c40d11696d0
It does a bit of extra (out of scope) grouping those related
constants but I'd like to keep it that way (should not cause
much harm in the context of merge conflicts).
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v11 0/3] tpm: robust stack allocations
From: Jarkko Sakkinen @ 2025-09-24 1:37 UTC (permalink / raw)
To: linux-integrity
Cc: Stefano Garzarella, David Howells, Paul Moore, James Morris,
Serge E. Hallyn, open list:KEYS/KEYRINGS,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <aNNKAIbzki-a121p@kernel.org>
On Wed, Sep 24, 2025 at 04:31:48AM +0300, Jarkko Sakkinen wrote:
> On Tue, Sep 23, 2025 at 08:07:41PM +0300, Jarkko Sakkinen wrote:
> > 1. These are previous changes to tpm_buf, which make stack allocations
> > much more feasible than previously.
> > 2. Migrate low-hanging fruit to use stack allocations.
> > 3. Re-orchestrate tpm_get_random().
> >
> > Jarkko Sakkinen (3):
> > tpm: Make TPM buffer allocations more robust
> > tpm: Use TPM_MIN_BUF_SIZE in driver commands
> > tpm orchestrate tpm_get_random() in the function
>
> I'm quite soon sending v12 because I randomly noticed a resource
> over-cosumption bug. which could be theoretically triggered e.g.
> via interposing a bus or a faulty device: tpm2_get_pcr_allocation()
> does not have a hard limit for the number of PCR banks.
>
> The fix:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?h=tpm-buf&id=3d92f14c204d09babadaa0b7c7a82c40d11696d0
[should have "ret = -E2BIG; goto out;". The reason for that bug is that
I just rebased the patch from tip to the bottom of the series, which
caused some merge conflicts.]
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature
From: Coiby Xu @ 2025-09-24 8:03 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <40e9c7bd15d4ab8b71ac335b5d896ed39c61980c.camel@linux.ibm.com>
On Mon, Sep 15, 2025 at 05:05:42PM -0400, Mimi Zohar wrote:
>On Wed, 2025-09-10 at 09:20 +0800, Coiby Xu wrote:
>> On Tue, Sep 09, 2025 at 11:31:20AM -0400, Mimi Zohar wrote:
>> > On Tue, 2025-09-09 at 12:19 +0800, Coiby Xu wrote:
>> > > When both IMA and EVM fix modes are enabled, accessing a file with IMA
>> > > signature won't cause security.evm to be fixed. But this doesn't happen
>> > > to a file with correct IMA hash already set because accessing it will
>> > > cause setting security.ima again which triggers fixing security.evm
>> > > thanks to security_inode_post_setxattr->evm_update_evmxattr.
>> > >
>> > > Let's use the same mechanism to fix security.evm for a file with IMA
>> > > signature.
>> > >
[...]
>> > > ---
>> > > security/integrity/ima/ima_appraise.c | 27 +++++++++++++++++++++------
>> > > 1 file changed, 21 insertions(+), 6 deletions(-)
>> > >
>> > > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
>> > > index f435eff4667f..18c3907c5e44 100644
>> > > --- a/security/integrity/ima/ima_appraise.c
>> > > +++ b/security/integrity/ima/ima_appraise.c
>> > > @@ -595,12 +595,27 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
>> > > integrity_audit_msg(audit_msgno, inode, filename,
>> > > op, cause, rc, 0);
>> > > } else if (status != INTEGRITY_PASS) {
>> > > - /* Fix mode, but don't replace file signatures. */
>> > > - if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig &&
>> > > - (!xattr_value ||
>> > > - xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
>> > > - if (!ima_fix_xattr(dentry, iint))
>> > > - status = INTEGRITY_PASS;
>> > > + /*
>> > > + * Fix mode, but don't replace file signatures.
>> > > + *
>> > > + * When EVM fix mode is also enabled, security.evm will be
>> > > + * fixed automatically when security.ima is set because of
>> > > + * security_inode_post_setxattr->evm_update_evmxattr.
>> > > + */
>> > > + if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig) {
>> > > + if (!xattr_value ||
>> > > + xattr_value->type != EVM_IMA_XATTR_DIGSIG) {
>> > > + if (ima_fix_xattr(dentry, iint))
>> > > + status = INTEGRITY_PASS;
>> > > + } else if (xattr_value->type == EVM_IMA_XATTR_DIGSIG &&
>> > > + evm_revalidate_status(XATTR_NAME_IMA)) {
>> > > + if (!__vfs_setxattr_noperm(&nop_mnt_idmap,
>> > > + dentry,
>> > > + XATTR_NAME_IMA,
>> > > + xattr_value,
>> > > + xattr_len, 0))
>> > > + status = INTEGRITY_PASS;
>> > > + }
>> > > }
>
>Instead of re-writing the IMA signature without a clear explanation, define a
>new EVM function named evm_fix_hmac() and add a call here in IMA. Only in EVM
>fix mode would evm_fix_hmac() update the EVM hmac.
>
> } else if (status != INTEGRITY_PASS) {
> /*
> * IMA fix mode updates the IMA file hash, which triggers EVM
> * to update security.evm. ....
> *
> * Similarly, trigger fixing EVM HMAC for IMA file signatures.
> */
> if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig) {
> if (!xattr_value ||
> xattr_value->type != EVM_IMA_XATTR_DIGSIG) {
> if (ima_fix_xattr(dentry, iint))
> status = INTEGRITY_PASS;
> } else if (status == INTEGRITY_NOLABEL) {
> evm_fix_hmac(dentry, XATTR_NAME_IMA, ....);
> }
> }
Thanks for the advice! I wonder if we should use existing
evm_update_evmxattr instead of defining a new EVM function.
/*
* Calculate the hmac and update security.evm xattr
*
* Expects to be called with i_mutex locked.
*/
int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
const char *xattr_value, size_t xattr_value_len)
{
}
I already tried evm_update_evmxattr and can confirm it works. But later
I switched to __vfs_setxattr_noperm because I thought it's consistent
with current logic of adding security.evm when there is already correct
security.ima and it's a slightly smaller change.
--
Best regards,
Coiby
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox