[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH-for-7.1] hw/tpm/tpm_tis: Avoid eventual read overrun
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH-for-7.1] hw/tpm/tpm_tis: Avoid eventual read overrun |
Date: |
Thu, 31 Mar 2022 12:42:14 +0200 |
User-agent: |
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 |
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.