Re: [PATCH-for-7.1] hw/tpm/tpm_tis: Avoid eventual read overrun

[Philippe Mathieu-Daudé]

On 31/3/22 09:50, Marc-André Lureau wrote:
> Hi
>
> On Thu, Mar 31, 2022 at 4:02 AM Philippe Mathieu-Daudé 
> <philippe.mathieu.daude@gmail.com 
> <mailto:philippe.mathieu.daude@gmail.com>> wrote:
>
>     From: Philippe Mathieu-Daudé <f4bug@amsat.org <mailto:f4bug@amsat.org>>
>
>     The TPMState structure hold an array of TPM_TIS_NUM_LOCALITIES
>     TPMLocality loc[], having TPM_TIS_NUM_LOCALITIES defined as '5'.
>
>     tpm_tis_locality_from_addr() returns up to 3 bits, so 7.
>
>     While unlikely, Coverity is right to report an overrun. Assert
>     we are in range to fix:
>
>        *** CID 1487240:  Memory - illegal accesses  (OVERRUN)
>        hw/tpm/tpm_tis_common.c: 298 in tpm_tis_dump_state()
>        294         int idx;
>        295         uint8_t locty = tpm_tis_locality_from_addr(addr);
>        296         hwaddr base = addr & ~0xfff;
>        297
>        >>>     CID 1487240:  Memory - illegal accesses  (OVERRUN)
>        >>>     Overrunning array "s->loc" of 5 24-byte elements at
>     element index 7 (byte offset 191) using index "locty" (which
>     evaluates to 7).
>        298         printf("tpm_tis: active locality      : %d\n"
>        299                "tpm_tis: state of locality %d : %d\n"
>        300                "tpm_tis: register dump:\n",
>        301                s->active_locty,
>        302                locty, s->loc[locty].state);
>
>     Fixes: Coverity CID 1487240
>     Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org
>     <mailto:f4bug@amsat.org>>
>
>
> Maybe that assert should be in tpm_tis_locality_from_addr(), as various 
> callers could produce the same report.

OK I see, tpm_tis_memory_ops handlers are safe because mapped as:

    memory_region_init_io(&s->mmio, obj, &tpm_tis_memory_ops,
                          s, "tpm-tis-mmio",
                          TPM_TIS_NUM_LOCALITIES <<
                          TPM_TIS_LOCALITY_SHIFT);

So invalid addresses are impossible from guest.