tpm: Fix uninitialized name_size_alg in tpm_buf_append_name()

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.

---
 drivers/char/tpm/tpm2-sessions.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

index c4da6fde748f..795cd99dc6fe 100644
@@ -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

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/[email protected]/
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

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.

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

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.

BR, Jarkko

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.

---
 drivers/char/tpm/tpm2-sessions.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

index c4da6fde748f..795cd99dc6fe 100644
@@ -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