Skip to content

Commit abce9ac

Browse files
PeterHueweKent Yoder
authored and
Kent Yoder
committed
tpm: Propagate error from tpm_transmit to fix a timeout hang
tpm_write calls tpm_transmit without checking the return value and assigns the return value unconditionally to chip->pending_data, even if it's an error value. This causes three bugs. So if we write to /dev/tpm0 with a tpm_param_size bigger than TPM_BUFSIZE=0x1000 (e.g. 0x100a) and a bufsize also bigger than TPM_BUFSIZE (e.g. 0x100a) tpm_transmit returns -E2BIG which is assigned to chip->pending_data as -7, but tpm_write returns that TPM_BUFSIZE bytes have been successfully been written to the TPM, altough this is not true (bug #1). As we did write more than than TPM_BUFSIZE bytes but tpm_write reports that only TPM_BUFSIZE bytes have been written the vfs tries to write the remaining bytes (in this case 10 bytes) to the tpm device driver via tpm_write which then blocks at /* cannot perform a write until the read has cleared either via tpm_read or a user_read_timer timeout */ while (atomic_read(&chip->data_pending) != 0) msleep(TPM_TIMEOUT); for 60 seconds, since data_pending is -7 and nobody is able to read it (since tpm_read luckily checks if data_pending is greater than 0) (#bug 2). After that the remaining bytes are written to the TPM which are interpreted by the tpm as a normal command. (bug #3) So if the last bytes of the command stream happen to be a e.g. tpm_force_clear this gets accidentally sent to the TPM. This patch fixes all three bugs, by propagating the error code of tpm_write and returning -E2BIG if the input buffer is too big, since the response from the tpm for a truncated value is bogus anyway. Moreover it returns -EBUSY to userspace if there is a response ready to be read. Signed-off-by: Peter Huewe <[email protected]> Signed-off-by: Kent Yoder <[email protected]>
1 parent bf53083 commit abce9ac

File tree

1 file changed

+14
-7
lines changed

1 file changed

+14
-7
lines changed

drivers/char/tpm/tpm.c

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,17 +1182,20 @@ ssize_t tpm_write(struct file *file, const char __user *buf,
11821182
size_t size, loff_t *off)
11831183
{
11841184
struct tpm_chip *chip = file->private_data;
1185-
size_t in_size = size, out_size;
1185+
size_t in_size = size;
1186+
ssize_t out_size;
11861187

11871188
/* cannot perform a write until the read has cleared
1188-
either via tpm_read or a user_read_timer timeout */
1189-
while (atomic_read(&chip->data_pending) != 0)
1190-
msleep(TPM_TIMEOUT);
1191-
1192-
mutex_lock(&chip->buffer_mutex);
1189+
either via tpm_read or a user_read_timer timeout.
1190+
This also prevents splitted buffered writes from blocking here.
1191+
*/
1192+
if (atomic_read(&chip->data_pending) != 0)
1193+
return -EBUSY;
11931194

11941195
if (in_size > TPM_BUFSIZE)
1195-
in_size = TPM_BUFSIZE;
1196+
return -E2BIG;
1197+
1198+
mutex_lock(&chip->buffer_mutex);
11961199

11971200
if (copy_from_user
11981201
(chip->data_buffer, (void __user *) buf, in_size)) {
@@ -1202,6 +1205,10 @@ ssize_t tpm_write(struct file *file, const char __user *buf,
12021205

12031206
/* atomic tpm command send and result receive */
12041207
out_size = tpm_transmit(chip, chip->data_buffer, TPM_BUFSIZE);
1208+
if (out_size < 0) {
1209+
mutex_unlock(&chip->buffer_mutex);
1210+
return out_size;
1211+
}
12051212

12061213
atomic_set(&chip->data_pending, out_size);
12071214
mutex_unlock(&chip->buffer_mutex);

0 commit comments

Comments
 (0)