* [PATCH v10 0/4] tpm: robust stack allocations
@ 2025-09-21 2:08 Jarkko Sakkinen
2025-09-21 2:08 ` [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust Jarkko Sakkinen
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Jarkko Sakkinen @ 2025-09-21 2:08 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.
Jarkko Sakkinen (4):
tpm: Make TPM buffer allocations more robust
tpm, tpm1-cmd: Use stack for trivial cases
tpm, tpm2-cmd: Use stack for trivial cases
tpm_vpm_proxy: Use stack for TPM_CC_SET_LOCALITY
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 | 151 +++++------
drivers/char/tpm/tpm2-cmd.c | 297 ++++++++++------------
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 | 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, 493 insertions(+), 546 deletions(-)
--
2.39.5
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust 2025-09-21 2:08 [PATCH v10 0/4] tpm: robust stack allocations Jarkko Sakkinen @ 2025-09-21 2:08 ` Jarkko Sakkinen 2025-09-21 8:53 ` kernel test robot 2025-09-22 8:44 ` Stefano Garzarella 2025-09-21 2:08 ` [PATCH v10 2/4] tpm, tpm1-cmd: Use stack for trivial cases Jarkko Sakkinen ` (3 subsequent siblings) 4 siblings, 2 replies; 12+ messages in thread From: Jarkko Sakkinen @ 2025-09-21 2:08 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 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); + 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); + 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 [flat|nested] 12+ messages in thread
* Re: [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust 2025-09-21 2:08 ` [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust Jarkko Sakkinen @ 2025-09-21 8:53 ` kernel test robot 2025-09-22 8:44 ` Stefano Garzarella 1 sibling, 0 replies; 12+ messages in thread From: kernel test robot @ 2025-09-21 8:53 UTC (permalink / raw) To: Jarkko Sakkinen, linux-integrity Cc: llvm, oe-kbuild-all, Stefano Garzarella, Jarkko Sakkinen, Stefan Berger, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, linux-kernel, linux-security-module Hi Jarkko, kernel test robot noticed the following build warnings: [auto build test WARNING on next-20250919] [cannot apply to char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.17-rc6 v6.17-rc5 v6.17-rc4 v6.17-rc6] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Jarkko-Sakkinen/tpm-Make-TPM-buffer-allocations-more-robust/20250921-102024 base: next-20250919 patch link: https://lore.kernel.org/r/20250921020804.1088824-2-jarkko%40kernel.org patch subject: [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust config: x86_64-buildonly-randconfig-001-20250921 (https://download.01.org/0day-ci/archive/20250921/202509211636.ZaHdTpHP-lkp@intel.com/config) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250921/202509211636.ZaHdTpHP-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202509211636.ZaHdTpHP-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/char/tpm/tpm-buf.c:20:9: warning: format specifies type 'unsigned int' but the argument has type 'unsigned long' [-Wformat] 19 | WARN(1, "%s: size mismatch: %u != %u\n", __func__, buf_size, | ~~ | %lu 20 | buf->capacity + sizeof(*buf)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/asm-generic/bug.h:141:29: note: expanded from macro 'WARN' 141 | __WARN_printf(TAINT_WARN, format); \ | ^~~~~~ include/asm-generic/bug.h:112:17: note: expanded from macro '__WARN_printf' 112 | __warn_printk(arg); \ | ^~~ 1 warning generated. vim +20 drivers/char/tpm/tpm-buf.c 9 10 static void __tpm_buf_size_invariant(struct tpm_buf *buf, u16 buf_size) 11 { 12 if (!buf->capacity) { 13 if (buf_size > TPM_BUF_MAX_SIZE) { 14 WARN(1, "%s: size overflow: %u\n", __func__, buf_size); 15 buf->flags |= TPM_BUF_ERROR; 16 } 17 } else { 18 if (buf_size != buf->capacity + sizeof(*buf)) { 19 WARN(1, "%s: size mismatch: %u != %u\n", __func__, buf_size, > 20 buf->capacity + sizeof(*buf)); 21 buf->flags |= TPM_BUF_ERROR; 22 } 23 } 24 } 25 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust 2025-09-21 2:08 ` [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust Jarkko Sakkinen 2025-09-21 8:53 ` kernel test robot @ 2025-09-22 8:44 ` Stefano Garzarella 2025-09-23 14:30 ` Jarkko Sakkinen 1 sibling, 1 reply; 12+ messages in thread From: Stefano Garzarella @ 2025-09-22 8:44 UTC (permalink / raw) To: Jarkko Sakkinen 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 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? >+ 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; } Thanks, Stefano ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust 2025-09-22 8:44 ` Stefano Garzarella @ 2025-09-23 14:30 ` Jarkko Sakkinen 2025-09-24 10:10 ` Stefano Garzarella 0 siblings, 1 reply; 12+ messages in thread 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 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 [flat|nested] 12+ messages in thread
* Re: [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust 2025-09-23 14:30 ` Jarkko Sakkinen @ 2025-09-24 10:10 ` Stefano Garzarella 0 siblings, 0 replies; 12+ messages in thread From: Stefano Garzarella @ 2025-09-24 10:10 UTC (permalink / raw) To: Jarkko Sakkinen 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 On Tue, Sep 23, 2025 at 05:30:20PM +0300, Jarkko Sakkinen wrote: >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. Oh, I see, thanks! > >> >> > + 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). Yep, I see. Stefano ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v10 2/4] tpm, tpm1-cmd: Use stack for trivial cases 2025-09-21 2:08 [PATCH v10 0/4] tpm: robust stack allocations Jarkko Sakkinen 2025-09-21 2:08 ` [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust Jarkko Sakkinen @ 2025-09-21 2:08 ` Jarkko Sakkinen 2025-09-21 2:08 ` [PATCH v10 3/4] tpm, tpm2-cmd: " Jarkko Sakkinen ` (2 subsequent siblings) 4 siblings, 0 replies; 12+ messages in thread From: Jarkko Sakkinen @ 2025-09-21 2:08 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 From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Use stack allocation for trivial "low-hanging fruit" sites, which are often also critical code paths associated with probing and power management. Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> --- drivers/char/tpm/tpm1-cmd.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c index ca3e7f9a105d..11c16ad9b2a7 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_INT_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; } -- 2.39.5 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v10 3/4] tpm, tpm2-cmd: Use stack for trivial cases 2025-09-21 2:08 [PATCH v10 0/4] tpm: robust stack allocations Jarkko Sakkinen 2025-09-21 2:08 ` [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust Jarkko Sakkinen 2025-09-21 2:08 ` [PATCH v10 2/4] tpm, tpm1-cmd: Use stack for trivial cases Jarkko Sakkinen @ 2025-09-21 2:08 ` Jarkko Sakkinen 2025-09-21 2:08 ` [PATCH v10 4/4] tpm_vpm_proxy: Use stack for TPM_CC_SET_LOCALITY Jarkko Sakkinen 2025-09-21 12:54 ` [PATCH v10 0/4] tpm: robust stack allocations Jarkko Sakkinen 4 siblings, 0 replies; 12+ messages in thread From: Jarkko Sakkinen @ 2025-09-21 2:08 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 From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Use stack allocation for trivial "low-hanging fruit" sites, which are often also critical code paths associated with probing and power management. Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> --- drivers/char/tpm/tpm2-cmd.c | 57 ++++++++++++++----------------------- 1 file changed, 22 insertions(+), 35 deletions(-) diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index 245c7c952e82..3c55f60ae4c2 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -382,14 +382,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 +406,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 +641,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; - - dev_info(&chip->dev, "starting up the TPM manually\n"); + u8 buf_data[TPM_BUF_MIN_SIZE]; + struct tpm_buf *buf = (struct tpm_buf *)buf_data; - 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"); } /** -- 2.39.5 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v10 4/4] tpm_vpm_proxy: Use stack for TPM_CC_SET_LOCALITY 2025-09-21 2:08 [PATCH v10 0/4] tpm: robust stack allocations Jarkko Sakkinen ` (2 preceding siblings ...) 2025-09-21 2:08 ` [PATCH v10 3/4] tpm, tpm2-cmd: " Jarkko Sakkinen @ 2025-09-21 2:08 ` Jarkko Sakkinen 2025-09-22 8:46 ` Stefano Garzarella 2025-09-21 12:54 ` [PATCH v10 0/4] tpm: robust stack allocations Jarkko Sakkinen 4 siblings, 1 reply; 12+ messages in thread From: Jarkko Sakkinen @ 2025-09-21 2:08 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 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); 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, "TPM_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 [flat|nested] 12+ messages in thread
* Re: [PATCH v10 4/4] tpm_vpm_proxy: Use stack for TPM_CC_SET_LOCALITY 2025-09-21 2:08 ` [PATCH v10 4/4] tpm_vpm_proxy: Use stack for TPM_CC_SET_LOCALITY Jarkko Sakkinen @ 2025-09-22 8:46 ` Stefano Garzarella 2025-09-23 14:30 ` Jarkko Sakkinen 0 siblings, 1 reply; 12+ messages in thread From: Stefano Garzarella @ 2025-09-22 8:46 UTC (permalink / raw) To: Jarkko Sakkinen 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 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? Thanks, Stefano > 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, "TPM_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 [flat|nested] 12+ messages in thread
* Re: [PATCH v10 4/4] tpm_vpm_proxy: Use stack for TPM_CC_SET_LOCALITY 2025-09-22 8:46 ` Stefano Garzarella @ 2025-09-23 14:30 ` Jarkko Sakkinen 0 siblings, 0 replies; 12+ messages in thread 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 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 [flat|nested] 12+ messages in thread
* Re: [PATCH v10 0/4] tpm: robust stack allocations 2025-09-21 2:08 [PATCH v10 0/4] tpm: robust stack allocations Jarkko Sakkinen ` (3 preceding siblings ...) 2025-09-21 2:08 ` [PATCH v10 4/4] tpm_vpm_proxy: Use stack for TPM_CC_SET_LOCALITY Jarkko Sakkinen @ 2025-09-21 12:54 ` Jarkko Sakkinen 4 siblings, 0 replies; 12+ messages in thread From: Jarkko Sakkinen @ 2025-09-21 12:54 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, ross.philipson On Sun, Sep 21, 2025 at 05:08:00AM +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. I highly unlikely would put this to 6.18 PR. The first patch huge but at the same time it is still a "logical change". Splitting it would make it less understandable. It's also quite stable and the despite many changes, most of them are mechanical changes (some of them I've even done originally with an LLM called 'sed'). IMHO reason to split should not be based no size but exactly on logical steps. After these few quick rounds I take some time and work on tpm1/tpm2-cmd to make them follow a builder pattern. This helps e.g., TrenchBoot to accomplish their goals more easily as builders translate easily to early boot situations. In addition, making memory management more stack oriented usually tends to help with such situations. So the next thing for this patch set is to make things work with parse, build and transmission decoupled from each other, and it will be as substantial change as the first patch but can be split into smaller patches as it does not change the global economy. An example with tpm2_pcr_read: 1. tpm2_build_pcr_read(), which takes pre-initialized tpm_buf() and same parameter as today. 2. tpm_transmit() 3. tpm2_parse_pcr_read(), which takes resulting tpm_buf and parses it to a struct, let's say tpm2_pcr_read_result. And similar decoupling is done for TPM 1 commands as they also need to translate between environments. Obviously outer facing tpm-interface.c API can still respond to old API calls for the time being. Other stuff: 1. tpm_buf_* from include/linux/tpm.j migrate to include/linux/tpm_buf.h 2. builders: tpm1_build.c, tpm2_build.c 3. parsers: tpm1_parse.c, tpm2_parse.c Most likely at least at first landing some redundancy is required for Trenchboto and physical transmission path. By doing what I have described we can set a limit to the amount redunancy :-) > > Jarkko Sakkinen (4): > tpm: Make TPM buffer allocations more robust > tpm, tpm1-cmd: Use stack for trivial cases > tpm, tpm2-cmd: Use stack for trivial cases > tpm_vpm_proxy: Use stack for TPM_CC_SET_LOCALITY > > 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 | 151 +++++------ > drivers/char/tpm/tpm2-cmd.c | 297 ++++++++++------------ > 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 | 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, 493 insertions(+), 546 deletions(-) > > -- > 2.39.5 > BR, Jarkko ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-09-24 10:10 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-21 2:08 [PATCH v10 0/4] tpm: robust stack allocations Jarkko Sakkinen 2025-09-21 2:08 ` [PATCH v10 1/4] tpm: Make TPM buffer allocations more robust Jarkko Sakkinen 2025-09-21 8:53 ` kernel test robot 2025-09-22 8:44 ` Stefano Garzarella 2025-09-23 14:30 ` Jarkko Sakkinen 2025-09-24 10:10 ` Stefano Garzarella 2025-09-21 2:08 ` [PATCH v10 2/4] tpm, tpm1-cmd: Use stack for trivial cases Jarkko Sakkinen 2025-09-21 2:08 ` [PATCH v10 3/4] tpm, tpm2-cmd: " Jarkko Sakkinen 2025-09-21 2:08 ` [PATCH v10 4/4] tpm_vpm_proxy: Use stack for TPM_CC_SET_LOCALITY Jarkko Sakkinen 2025-09-22 8:46 ` Stefano Garzarella 2025-09-23 14:30 ` Jarkko Sakkinen 2025-09-21 12:54 ` [PATCH v10 0/4] tpm: robust stack allocations Jarkko Sakkinen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).