On Thu, Jun 13, 2019 at 10:55 AM Arnd Bergmann <arnd@xxxxxxxx> wrote: > > - efi.tpm_final_log is a physical address that gets passed into > memremap() to return a pointer > - tpm2_calc_event_log_size() takes a pointer argument and > dereferences it. Where does it? It's passed with some added offset to __calc_tpm2_event_size, which does the remapping part. That's why physical address is used here. > My best guess is that we should pass the output of memremap() > here rather than the input: > > --- a/drivers/firmware/efi/tpm.c > +++ b/drivers/firmware/efi/tpm.c > @@ -75,7 +75,7 @@ int __init efi_tpm_eventlog_init(void) > goto out; > } > > - tbl_size = tpm2_calc_event_log_size(efi.tpm_final_log > + tbl_size = tpm2_calc_event_log_size(final_tbl > + sizeof(final_tbl->version) > + sizeof(final_tbl->nr_events), > final_tbl->nr_events, > > No idea if that is actually what was intended here, but it makes > the code look more plausible. Passing final_tbl will lead to failure, as it will be remapped for second time. Cast is needed here. Changing the type from void* to unsigned long or phys_addr_t (and casting later) would also do the job. I'm fine with both options.