* [PATCH v2] tpm: Make the TPM character devices non-seekable
[not found] <CAHk-=wi8aXM9=Y8othMb1oxTv-PDw4isj-FPbXu0MXsm_EPE1g@mail.gmail.com>
@ 2026-07-12 17:11 ` Jaewon Yang
2026-07-12 19:54 ` Linus Torvalds
2026-07-18 18:13 ` Jarkko Sakkinen
0 siblings, 2 replies; 4+ messages in thread
From: Jaewon Yang @ 2026-07-12 17:11 UTC (permalink / raw)
To: Peter Huewe, Jarkko Sakkinen
Cc: Jason Gunthorpe, linux-integrity, linux-kernel, security,
linux-api
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/20260710090217.191289-1-yong010301@gmail.com/
Cc: stable@vger.kernel.org
Signed-off-by: Jaewon Yang <yong010301@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] tpm: Make the TPM character devices non-seekable
2026-07-12 17:11 ` [PATCH v2] tpm: Make the TPM character devices non-seekable Jaewon Yang
@ 2026-07-12 19:54 ` Linus Torvalds
2026-07-18 18:13 ` Jarkko Sakkinen
1 sibling, 0 replies; 4+ messages in thread
From: Linus Torvalds @ 2026-07-12 19:54 UTC (permalink / raw)
To: Jaewon Yang
Cc: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, linux-integrity,
linux-kernel, security, linux-api
On Sun, 12 Jul 2026 at 10:12, Jaewon Yang <yong010301@gmail.com> wrote:
>
> Call nonseekable_open() from both open handlers.
Ack, this seems to be the simplest approach. I'll just commit it as-is,
Linus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] tpm: Make the TPM character devices non-seekable
2026-07-12 17:11 ` [PATCH v2] tpm: Make the TPM character devices non-seekable Jaewon Yang
2026-07-12 19:54 ` Linus Torvalds
@ 2026-07-18 18:13 ` Jarkko Sakkinen
2026-07-18 18:20 ` Jarkko Sakkinen
1 sibling, 1 reply; 4+ messages in thread
From: Jarkko Sakkinen @ 2026-07-18 18:13 UTC (permalink / raw)
To: Jaewon Yang
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, linux-kernel,
security, linux-api
On Mon, Jul 13, 2026 at 02:11:47AM +0900, Jaewon Yang wrote:
> 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/20260710090217.191289-1-yong010301@gmail.com/
> Cc: stable@vger.kernel.org
> Signed-off-by: Jaewon Yang <yong010301@gmail.com>
> ---
> 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
>
LGTM
Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] tpm: Make the TPM character devices non-seekable
2026-07-18 18:13 ` Jarkko Sakkinen
@ 2026-07-18 18:20 ` Jarkko Sakkinen
0 siblings, 0 replies; 4+ messages in thread
From: Jarkko Sakkinen @ 2026-07-18 18:20 UTC (permalink / raw)
To: Jaewon Yang
Cc: Peter Huewe, Jason Gunthorpe, linux-integrity, linux-kernel,
security, linux-api
oN Sat, Jul 18, 2026 at 09:13:30PM +0300, Jarkko Sakkinen wrote:
> On Mon, Jul 13, 2026 at 02:11:47AM +0900, Jaewon Yang wrote:
> > 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/20260710090217.191289-1-yong010301@gmail.com/
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jaewon Yang <yong010301@gmail.com>
> > ---
> > 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
> >
>
> LGTM
>
>
> Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
[A slow response time due on holiday until end of this Month.]
BR, Jarkko
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-18 18:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CAHk-=wi8aXM9=Y8othMb1oxTv-PDw4isj-FPbXu0MXsm_EPE1g@mail.gmail.com>
2026-07-12 17:11 ` [PATCH v2] tpm: Make the TPM character devices non-seekable Jaewon Yang
2026-07-12 19:54 ` Linus Torvalds
2026-07-18 18:13 ` Jarkko Sakkinen
2026-07-18 18:20 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox