The TPM character devices expose a sequential command/response
interface, but their open handlers leave FMODE_PREAD and FMODE_PWRITE
enabled.

After a command leaves a response pending, pread(fd, buf, 16, 0x1400)
passes 0x1400 as *off to tpm_common_read(). The transfer length is
bounded by response_length, but the offset is used unchecked when
forming data_buffer + *off. A sufficiently large offset therefore
causes an out-of-bounds heap read through copy_to_user() and, if the
copy succeeds, an out-of-bounds zero-write through the following
memset().

Positional I/O does not provide coherent semantics for this interface.
An arbitrary pread offset cannot represent how much of a response has
been consumed sequentially. The write callback always stores a command
at the start of data_buffer, while pwrite() does not update file->f_pos
and can leave the sequential read cursor stale.

Call nonseekable_open() from both open handlers. This removes
FMODE_PREAD and FMODE_PWRITE, causing positional reads and writes to
fail with -ESPIPE before reaching the TPM callbacks, and explicitly
marks the files non-seekable. Normal read() and write() continue to use
the existing sequential f_pos cursor, leaving the response state
machine unchanged.

Tested on Linux 6.12 with KASAN and a swtpm TPM2 device:

- sequential partial reads returned the complete response;
- pread() and preadv() with offset 0x1400 returned -ESPIPE;
- pwrite() and pwritev() with offset zero returned -ESPIPE;
- the pending response remained intact after the rejected operations;
- a subsequent normal command/response cycle completed normally; and
- no KASAN report was produced.

Fixes: 9488585b21be ("tpm: add support for partial reads")
Link: https://lore.kernel.org/all/[email protected]/
Cc: [email protected]
Signed-off-by: Jaewon Yang <[email protected]>
---
Changes in v2:
- replace the response-buffer bounds check with nonseekable_open();
- reject positional read and write at open time;
- preserve the existing sequential read/write state machine.

The alternative response_length rework proposed during review was tested
and not taken: a read-until-EOF loop hangs because cleanup resets *off
without clearing response_length. It also treats an arbitrary positional
offset as the consumption cursor; for example,

    pread(fd, &c, 1, 99)

on a 100-byte response can discard bytes 0 through 98 without returning
them.

 drivers/char/tpm/tpm-dev.c   | 2 +-
 drivers/char/tpm/tpmrm-dev.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index 2779a8738..74488f0a7 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -36,7 +36,7 @@ static int tpm_open(struct inode *inode, struct file *file)
 
        tpm_common_open(file, chip, priv, NULL);
 
-       return 0;
+       return nonseekable_open(inode, file);
 
  out:
        clear_bit(0, &chip->is_open);
diff --git a/drivers/char/tpm/tpmrm-dev.c b/drivers/char/tpm/tpmrm-dev.c
index f48d4d9e1..19e8f2779 100644
--- a/drivers/char/tpm/tpmrm-dev.c
+++ b/drivers/char/tpm/tpmrm-dev.c
@@ -29,7 +29,7 @@ static int tpmrm_open(struct inode *inode, struct file *file)
 
        tpm_common_open(file, chip, &priv->priv, &priv->space);
 
-       return 0;
+       return nonseekable_open(inode, file);
 }
 
 static int tpmrm_release(struct inode *inode, struct file *file)
-- 
2.43.0


Reply via email to