Laszlo Ersek <ler...@redhat.com> writes: > On 01/30/19 15:33, Paolo Bonzini wrote: >> On 30/01/19 15:13, Markus Armbruster wrote: >>> -global driver=cfi.pflash01,property=secure,value=on >>> >>> Affects *all* such devices, but fortunately we have at most two, and >>> the one we don't want to affect happens to ignore the property value. >> >> Is this true? I think both need secure=on, at least on x86. > > commit f71e42a5c98722d6faa5be84a34fbad90d27dc04 > Author: Paolo Bonzini <pbonz...@redhat.com> > Date: Wed Apr 8 14:09:43 2015 +0200 > > pflash_cfi01: add secure property > > When this property is set, MMIO accesses are only allowed with the > MEMTXATTRS_SECURE attribute. This is used for secure access to UEFI > variables stored in flash. > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > > If you don't add "secure=on" to unit#0, then MMIO accesses will be > possible outside of SMM. From those, I'd hazard "MMIO reads" are > generally irrelevant. "MMIO writes" could succeed to the RAM image, but: > > - they are never written back to the disk (due to readonly=on), > > - the actual contents of unit#0 stops mattering as soon as the SEC phase > decompresses the PEIFV and DXEFV firmware volumes from it, to DRAM. > The SMM infrastructure is then constructed in SMRAM from DRAM. By the > time a 3rd party UEFI application or driver, or an OS, is reached, the > sensitive bits are all locked in SMRAM. > > ... But, I wonder if S3 resume would be under threat in this case. In > that case, SEC runs again (from pflash), and it re-decompresses > PEIFV/DXEFV from pflash to DRAM. If the in-memory image of pflash > doesn't revert to the (pristine, due to readonly=on) disk image at > platform reset, then I reckon there could be a problem; the SEC code and > the compressed FVs could have been tampered with in memory. > > I guess it's best to apply secure=on to unit#0 as well, after all :)
I thought secure=on affected only writes (and so wouldn't matter with readonly=on), but I was wrong: static MemTxResult pflash_mem_read_with_attrs(void *opaque, hwaddr addr, uint64_t *value, unsigned len, MemTxAttrs attrs) { pflash_t *pfl = opaque; bool be = !!(pfl->features & (1 << PFLASH_BE)); if ((pfl->features & (1 << PFLASH_SECURE)) && !attrs.secure) { *value = pflash_data_read(opaque, addr, len, be); } else { *value = pflash_read(opaque, addr, len, be); } return MEMTX_OK; } pflash_data_read() is what pflash_read() does when pfl->cmd is 0. Hmm, why is it okay to treat all pfl->cmd values the same when secure=on?