linux-kernel - [RFC] [PATCH] tpm: Clean up TPM space after command failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZrzY_LWIXABkqd-S@earth.li>
Date: Wed, 14 Aug 2024 17:19:08 +0100
From: Jonathan McDowell <noodles@...th.li>
To: linux-integrity@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Jarkko Sakkinen <jarkko@...nel.org>,
	Peter Huewe <peterhuewe@....de>, Jason Gunthorpe <jgg@...pe.ca>
Subject: [RFC] [PATCH] tpm: Clean up TPM space after command failure

We've been seeing a problem where TPM commands time out, which if
they're the last command before the TPM device is closed causes a leak
of transient handles. They can be seen and cleaned up (with a flush
context) if the /dev/tpm0 device is used instead of /dev/tpmrm0, but it
seems like we should be doing this automatically on the transmit error
path. Patch below adds a tpm2_flush_space on error to avoid this.

Does this seem reasonable? The other query is whether tpm2_del_space
should cleanup the contexts as well, rather than just the sessions.

(Obviously in an ideal world we wouldn't see the timeouts at all, and
I'm still trying to work on getting to the bottom of these, which are
generally infrequent, but happening enough across our fleet that we were
able to observe this handle leak.)

From: Jonathan McDowell <noodles@...a.com>

tpm_dev_transmit prepares the TPM space before attempting command
transmission. However if the command fails no rollback of this
preparation is done. This can result in transient handles being leaked
if the device is subsequently closed with no further commands performed.

Fix this by flushing the space in the event of command transmission
failure.

Signed-off-by: Jonathan McDowell <noodles@...a.com>

---
 drivers/char/tpm/tpm-dev-common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index 30b4c288c1bb..c3fbbf4d3db7 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -47,6 +47,8 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,

 	if (!ret)
 		ret = tpm2_commit_space(chip, space, buf, &len);
+	else
+		tpm2_flush_space(chip);

 out_rc:
 	return ret ? ret : len;
-- 
2.46.0