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.da...@gmail.com <mailto:philippe.mathieu.da...@gmail.com>> wrote:

    From: Philippe Mathieu-Daudé <f4...@amsat.org <mailto:f4...@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é <f4...@amsat.org
    <mailto:f4...@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.

Reply via email to