Re: [PATCH] hw/tpm/tpm_tis_common.c: Assert that locty is in range

[Philippe Mathieu-Daudé]

On Fri, May 13, 2022 at 6:43 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In tpm_tis_mmio_read(), tpm_tis_mmio_write() and
> tpm_tis_dump_state(), we calculate a locality index with
> tpm_tis_locality_from_addr() and then use it as an index into the
> s->loc[] array.  In all these cases, the array index can't overflow
> because the MemoryRegion is sized to be TPM_TIS_NUM_LOCALITIES <<
> TPM_TIS_LOCALITY_SHIFT bytes.  However, Coverity can't see that, and
> it complains (CID 1487138, 1487180, 1487188, 1487198, 1487240).
>
> Add assertions that the calculated locality index is valid, which
> will help Coverity and also catch any potential future bug where
> the MemoryRegion isn't sized exactly.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Tested with 'make check' only...
>
>  hw/tpm/tpm_tis_common.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
> index e700d821816..81edae410c8 100644
> --- a/hw/tpm/tpm_tis_common.c
> +++ b/hw/tpm/tpm_tis_common.c
> @@ -295,6 +295,8 @@ static void tpm_tis_dump_state(TPMState *s, hwaddr addr)
>      uint8_t locty = tpm_tis_locality_from_addr(addr);
>      hwaddr base = addr & ~0xfff;
>
> +    assert(TPM_TIS_IS_VALID_LOCTY(locty));
> +
>      printf("tpm_tis: active locality      : %d\n"
>             "tpm_tis: state of locality %d : %d\n"
>             "tpm_tis: register dump:\n",

This one was here ...:
https://lore.kernel.org/qemu-devel/20220330235723.68033-1-philippe.mathieu.daude@gmail.com/

> @@ -336,6 +338,8 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr 
> addr,
>      uint32_t avail;
>      uint8_t v;
>
> +    assert(TPM_TIS_IS_VALID_LOCTY(locty));
> +
>      if (tpm_backend_had_startup_error(s->be_driver)) {
>          return 0;
>      }
> @@ -458,6 +462,8 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
>      uint16_t len;
>      uint32_t mask = (size == 1) ? 0xff : ((size == 2) ? 0xffff : ~0);
>
> +    assert(TPM_TIS_IS_VALID_LOCTY(locty));
> +
>      trace_tpm_tis_mmio_write(size, addr, val);
>
>      if (locty == 4) {

... but not these, so:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>