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 <p...@fedoraproject.org> | > | > 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