On Mon Sep 23, 2024 at 9:05 AM EEST, Herbert Xu wrote:
> Dear TPM maintainers:
There's really only just me (for past 10 years). Maybe that should be
updatred.
>
> Please have a look at the tpm hwrng driver because it appears to
> be returning a length longer than the buffer length that we gave
> it. In particular, tpm2 appears to be the culprit (though I didn't
> really check tpm1 at all so it could also be buggy).
>
> The following patch hopefully should confirm that this is indeed
> caused by TPM and not some other HWRNG driver.
>
> ---8<---
> If a buggy driver returns a length that is longer than the size
> of the buffer provided to it, then this may lead to a buffer overread
> in the caller.
>
> Stop this by adding a check for it in the hwrng core.
>
> Reported-by: Guangwu Zhang <guazhang@xxxxxxxxxx>
> Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index 57c51efa5613..018316f54621 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -181,8 +181,15 @@ static inline int rng_get_data(struct hwrng *rng, u8 *buffer, size_t size,
> int present;
>
> BUG_ON(!mutex_is_locked(&reading_mutex));
> - if (rng->read)
> - return rng->read(rng, (void *)buffer, size, wait);
> + if (rng->read) {
> + int err;
> +
> + err = rng->read(rng, buffer, size, wait);
> + if (WARN_ON_ONCE(err > 0 && err > size))
Are you sure you want to use WARN_ON_ONCE here instead of
pr_warn_once()? I.e. is it worth of taking down the whole
kernel?
> + err = size;
> +
> + return err;
> + }
>
> if (rng->data_present)
> present = rng->data_present(rng, wait);
BR, Jarkko
