* [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