Re: [Qemu-devel] [PATCH v1] tpm: check localities index

[P J P]

  Hello Stefan,

+-- On Tue, 20 Nov 2018, Stefan Berger wrote --+
| On 11/20/18 2:22 AM, P J P wrote:
| > From: Prasad J Pandit <address@hidden>
| >
| > While performing mmio device r/w operations, guest could set 'addr'
| > parameter such that 'locty' index exceeds TPM_TIS_NUM_LOCALITIES=5
| > after setting new 'locty' via 'tpm_tis_new_active_locality'.
| > Add check to avoid OOB access.
| 
| Do you have test code that can set the memory to such a locality or is this
| purely hypothetical at the moment?

It's not hypothetical, reporter used tpm-emulator below

  -> https://github.com/PeterHuewe/tpm-emulator

with an old version of QEMU. I shared the details with Marc-Andre earlier 
today.

| We are registering this MMIO area:
| 
|     memory_region_init_io(&s->mmio, OBJECT(s), &tpm_tis_memory_ops,
|                           s, "tpm-tis-mmio",
|                           TPM_TIS_NUM_LOCALITIES << 
| TPM_TIS_LOCALITY_SHIFT);
| 
| #define TPM_TIS_NUM_LOCALITIES      5
| 
| #define TPM_TIS_LOCALITY_SHIFT      12
| 
| --> 5 << 12 = 0x5000, thus we get memory locations [0 .. 0x4fff]
| 
| 
| We have the following code to get from an address to the locality:
| 
| static uint8_t tpm_tis_locality_from_addr(hwaddr addr)
| {
|     return (uint8_t)((addr >> TPM_TIS_LOCALITY_SHIFT) & 0x7);
| }
| 
| With this we would get localities of [0x0 .. 0x4] following the memory
| locations [0 .. 0x4fff] above.

  Right. IIUC mmio r/w routines called with 'addr' value like 0x07000 may set 
'locty' to > TPM_TIS_NUM_LOCALITIES. Also in mmio_write at one point, 
'active_locty' is set to TPM_TIS_NO_LOCALITY(=0xff), which may be used to set 
'new_locty'.

tpm_tis_mmio_write
   ...
   active_locty = TPM_TIS_NO_LOCALITY(=0xff);
   ...
   tpm_tis_new_active_locality(s, active_locty);


tpm_tis_new_active_locality(TPMState *s, uint8_t new_active_locty)
   ...
   s->active_locty = new_active_locty;


Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F