From: Jarkko Sakkinen <jarkko@kernel.org>
To: Shahriyar Jalayeri <shahriyar@posteo.de>
Cc: peterhuewe@gmx.de, jgg@ziepe.ca, linux-integrity@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] tpm: infineon: add bounds check in tpm_inf_recv
Date: Fri, 10 Oct 2025 20:40:13 +0300 [thread overview]
Message-ID: <aOlE_cREuIuCw2do@kernel.org> (raw)
In-Reply-To: <20251010065252.4377-1-shahriyar@posteo.de>
On Fri, Oct 10, 2025 at 06:52:55AM +0000, Shahriyar Jalayeri wrote:
> Add two buffer size validations to prevent buffer overflows in
> tpm_inf_recv():
>
> 1. Validate that the provided buffer can hold at least the 4-byte header
> before attempting to read it.
> 2. Validate that the buffer is large enough to hold the data size reported
> by the TPM before reading the payload.
>
> Without these checks, a malicious or malfunctioning TPM could cause buffer
> overflows by reporting data sizes larger than the provided buffer, leading
> to memory corruption.
>
> Fixes: ebb81fdb3dd0 ("[PATCH] tpm: Support for Infineon TPM")
> Signed-off-by: Shahriyar Jalayeri <shahriyar@posteo.de>
> ---
> drivers/char/tpm/tpm_infineon.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> index 7638b65b8..0fe4193a3 100644
> --- a/drivers/char/tpm/tpm_infineon.c
> +++ b/drivers/char/tpm/tpm_infineon.c
> @@ -250,6 +250,10 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> number_of_wtx = 0;
>
> recv_begin:
> + /* expect at least 1-byte VL header, 1-byte ctrl-tag, 2-byte data size */
This is definitely good enough :-)
But is that comment misaligned? Does VL come from "VLAN"?
> + if (count < 4)
> + return -EIO;
> +
> /* start receiving header */
> for (i = 0; i < 4; i++) {
> ret = wait(chip, STAT_RDA);
> @@ -268,6 +272,9 @@ static int tpm_inf_recv(struct tpm_chip *chip, u8 * buf, size_t count)
> /* size of the data received */
I'd delete the above comment.
> size = ((buf[2] << 8) | buf[3]);
And use here:
https://elixir.bootlin.com/linux/v6.17.1/source/include/linux/byteorder/generic.h#L108
Not exactly in scope but it would be good convention and make
the check after it more readable.
>
> + if (size + 6 > count)
> + return -EIO;
> +
> for (i = 0; i < size; i++) {
> wait(chip, STAT_RDA);
> buf[i] = tpm_data_in(RDFIFO);
> --
> 2.43.0
>
BR, Jarkko
prev parent reply other threads:[~2025-10-10 17:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-10 6:52 [PATCH v2] tpm: infineon: add bounds check in tpm_inf_recv Shahriyar Jalayeri
2025-10-10 7:11 ` Paul Menzel
2025-10-10 17:40 ` Jarkko Sakkinen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aOlE_cREuIuCw2do@kernel.org \
--to=jarkko@kernel.org \
--cc=jgg@ziepe.ca \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=peterhuewe@gmx.de \
--cc=shahriyar@posteo.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.