[PATCH] tpm: Fix uninitialized name_size_alg in tpm_buf_append_name()

[PATCH] tpm: Fix uninitialized name_size_alg in tpm_buf_append_name()

[Gunnar Kudrjavets]

When tpm_buf_append_name() is called with a non-NULL name for a
handle, the code skips the tpm2_read_public() path (which sets
name_size_alg from the return value) and falls through to memcpy()
with an uninitialized name_size_alg as the size argument.

The contract for tpm_buf_append_name() supports callers passing a
non-NULL name. No current in-tree callers do so, making this a latent
bug that would trigger if a caller ever provides a pre-computed name
for a handle.

Fix this by restructuring the if/else to call name_size() when name
is provided, sharing the error check and name_size_alg assignment
with the existing tpm2_read_public() path. This restores the type of
validation that existed before commit bda1cbf73c6e ("tpm2-sessions:
Fix tpm2_read_public range checks") refactored the function.

Tested with KASAN by assigning 0xDEAD to name_size_alg to simulate an
undefined initial value. Calling tpm_buf_append_name() with a non-NULL
value for name results in the following warnings from KASAN:

  BUG: KASAN: stack-out-of-bounds in tpm_buf_append_name+0x1e0/0x680
  Read of size 57005 at addr ffff80009e5e79f0 by task sh/49616

  Call trace:
   show_stack+0x34/0xa0 (C)
   dump_stack_lvl+0x5c/0x80
   print_report+0x160/0x4b8
   kasan_report+0x7c/0xd0
   kasan_check_range+0xe8/0x190
   __asan_memcpy+0x3c/0xa0
   tpm_buf_append_name+0x1e0/0x680
   run_test.isra.0+0x14c/0x1d8

There are no KASAN errors with the fix applied, and the function
behaves as expected.

Fixes: bda1cbf73c6e ("tpm2-sessions: Fix tpm2_read_public range checks")
Assisted-by: Kiro:claude-opus-4.6
Reviewed-by: Justinien Bouron <jbouron@amazon.com>
Reviewed-by: Muhammad Hammad Ijaz <mhijaz@amazon.com>
Signed-off-by: Gunnar Kudrjavets <gunnarku@amazon.com>
---
 drivers/char/tpm/tpm2-sessions.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index c4da6fde748f..795cd99dc6fe 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -285,11 +285,14 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
 	    mso == TPM2_MSO_NVRAM) {
 		if (!name) {
 			ret = tpm2_read_public(chip, handle, auth->name[slot]);
-			if (ret < 0)
-				goto err;
-
-			name_size_alg = ret;
+		} else {
+			ret = name_size(name);
 		}
+
+		if (ret < 0)
+			goto err;
+
+		name_size_alg = ret;
 	} else {
 		if (name) {
 			dev_err(&chip->dev, "handle 0x%08x does not use a name\n",

base-commit: 9ec4175a30eb5adb95e446af83ddf6cb3286a82a
-- 
2.47.3

Re: [PATCH] tpm: Fix uninitialized name_size_alg in tpm_buf_append_name()

[Jarkko Sakkinen]

On Thu, May 07, 2026 at 08:18:22PM +0000, Gunnar Kudrjavets wrote:
> When tpm_buf_append_name() is called with a non-NULL name for a
> handle, the code skips the tpm2_read_public() path (which sets
> name_size_alg from the return value) and falls through to memcpy()
> with an uninitialized name_size_alg as the size argument.
> 
> The contract for tpm_buf_append_name() supports callers passing a
> non-NULL name. No current in-tree callers do so, making this a latent
> bug that would trigger if a caller ever provides a pre-computed name
> for a handle.

This is great observation but it is not a regression technically.

Thus, this really should just state the issue and don't make it
look like a bug report based on transcript that does not happen
in the wild.

I guess this is better than parameter removal since name caching
would make sense [1] in future.

[1] Already done but gathered zero interest at the time:
    https://lore.kernel.org/linux-integrity/20260125192526.782202-1-jarkko@kernel.org/
    Probably patch set should be reworked to have only relevant patches
    as my tpm_get_random() patches did not gather too much following :-)

BR, Jarkko

Re: [PATCH v2] tpm: Fix uninitialized name_size_alg in tpm_buf_append_name()

[Gunnar Kudrjavets]

On Fri, May 09, 2026 at 07:35 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> This is great observation but it is not a regression technically.
>
> Thus, this really should just state the issue and don't make it look
> like a bug report based on transcript that does not happen in the wild.

Thank you for the feedback, Jarkko. That's a fair point. Since no
in-tree caller exercises this path today, framing it as a regression
is misleading. I'm happy to rework the commit description to present
it as a proactive hardening fix rather than a bug report, and drop the
Fixes tag accordingly.

> I guess this is better than parameter removal since name caching would
> make sense [1] in future.

Agreed. Keeeping the name parameter functional makes the API ready
for name caching without further changes.

Would you prefer I resend with the adjusted description, or would you
like to suggest specific wording? Happy to go either way.

Gunnar

Re: [PATCH v2] tpm: Fix uninitialized name_size_alg in tpm_buf_append_name()

[Jarkko Sakkinen]

On Sat, May 09, 2026 at 10:34:29PM +0000, Gunnar Kudrjavets wrote:
> On Fri, May 09, 2026 at 07:35 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > This is great observation but it is not a regression technically.
> >
> > Thus, this really should just state the issue and don't make it look
> > like a bug report based on transcript that does not happen in the wild.
> 
> Thank you for the feedback, Jarkko. That's a fair point. Since no
> in-tree caller exercises this path today, framing it as a regression
> is misleading. I'm happy to rework the commit description to present
> it as a proactive hardening fix rather than a bug report, and drop the
> Fixes tag accordingly.
> 
> > I guess this is better than parameter removal since name caching would
> > make sense [1] in future.
> 
> Agreed. Keeeping the name parameter functional makes the API ready
> for name caching without further changes.
> 
> Would you prefer I resend with the adjusted description, or would you
> like to suggest specific wording? Happy to go either way.

Yeah so the point here is probably to prevent unmasking a bug in future,
with the specific example of name caching. I.e., I see the change itself
useful and important despite not being a bug fix per se.

> 
> Gunnar

BR, Jarkko

[PATCH v2] tpm: Initialize name_size_alg for non-NULL name in tpm_buf_append_name()

[Gunnar Kudrjavets]

tpm_buf_append_name() supports callers passing a pre-computed name
for handles. When name is non-NULL, the code skips the
tpm2_read_public() path but leaves name_size_alg uninitialized
before it is used as the memcpy size argument.

No current in-tree caller passes a non-NULL name, but future use
cases such as name caching would exercise this path. Initialize
name_size_alg by calling name_size() on the caller-provided name,
sharing the error check and assignment with the existing
tpm2_read_public() path. This prevents unmasking a latent bug when
the non-NULL name path is eventually used.

Assisted-by: Kiro:claude-opus-4.6
Reviewed-by: Justinien Bouron <jbouron@amazon.com>
Reviewed-by: Muhammad Hammad Ijaz <mhijaz@amazon.com>
Signed-off-by: Gunnar Kudrjavets <gunnarku@amazon.com>
---
 drivers/char/tpm/tpm2-sessions.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index c4da6fde748f..795cd99dc6fe 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -285,11 +285,14 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
 	    mso == TPM2_MSO_NVRAM) {
 		if (!name) {
 			ret = tpm2_read_public(chip, handle, auth->name[slot]);
-			if (ret < 0)
-				goto err;
-
-			name_size_alg = ret;
+		} else {
+			ret = name_size(name);
 		}
+
+		if (ret < 0)
+			goto err;
+
+		name_size_alg = ret;
 	} else {
 		if (name) {
 			dev_err(&chip->dev, "handle 0x%08x does not use a name\n",

base-commit: 44bd97559c26bb4d7abac09d29e58a4152d88567
--
2.47.3

Re: [PATCH v2] tpm: Initialize name_size_alg for non-NULL name in tpm_buf_append_name()

[Jarkko Sakkinen]

On Sun, May 10, 2026 at 05:11:27PM +0000, Gunnar Kudrjavets wrote:
> tpm_buf_append_name() supports callers passing a pre-computed name
> for handles. When name is non-NULL, the code skips the
> tpm2_read_public() path but leaves name_size_alg uninitialized
> before it is used as the memcpy size argument.
> 
> No current in-tree caller passes a non-NULL name, but future use
> cases such as name caching would exercise this path. Initialize
> name_size_alg by calling name_size() on the caller-provided name,
> sharing the error check and assignment with the existing
> tpm2_read_public() path. This prevents unmasking a latent bug when
> the non-NULL name path is eventually used.
> 
> Assisted-by: Kiro:claude-opus-4.6
> Reviewed-by: Justinien Bouron <jbouron@amazon.com>
> Reviewed-by: Muhammad Hammad Ijaz <mhijaz@amazon.com>
> Signed-off-by: Gunnar Kudrjavets <gunnarku@amazon.com>
> ---
>  drivers/char/tpm/tpm2-sessions.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> index c4da6fde748f..795cd99dc6fe 100644
> --- a/drivers/char/tpm/tpm2-sessions.c
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -285,11 +285,14 @@ int tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf,
>  	    mso == TPM2_MSO_NVRAM) {
>  		if (!name) {
>  			ret = tpm2_read_public(chip, handle, auth->name[slot]);
> -			if (ret < 0)
> -				goto err;
> -
> -			name_size_alg = ret;
> +		} else {
> +			ret = name_size(name);
>  		}
> +
> +		if (ret < 0)
> +			goto err;
> +
> +		name_size_alg = ret;
>  	} else {
>  		if (name) {
>  			dev_err(&chip->dev, "handle 0x%08x does not use a name\n",
> 
> base-commit: 44bd97559c26bb4d7abac09d29e58a4152d88567
> --
> 2.47.3
> 

Thank you. Applied.

BR, Jarkko