On 7 July 2014 22:43, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote: > On 7 July 2014 22:29, Matt Fleming <matt@xxxxxxxxxxxxxxxxx> wrote: >> On Wed, 02 Jul, at 12:10:02PM, Ard Biesheuvel wrote: >>> According to section 7.1 of the UEFI spec, Runtime Services are not fully >>> reentrant, and there are particular combinations of calls that need to be >>> serialized. >>> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> >>> --- >>> drivers/firmware/efi/runtime-wrappers.c | 109 +++++++++++++++++++++++++++++--- >>> 1 file changed, 99 insertions(+), 10 deletions(-) >> >> Ard, what's going on with this one? I didn't see it resubmitted along >> with v3 of "efi/arm64: handle missing virtual mapping for UEFI System >> Table". >> >> Note that we already have a lock to serialize access to the UEFI >> variable services in the form of __efivars->lock, see >> drivers/firmware/efi/vars.c. It's a spinlock because of the context we >> may need to create variables in from efi_pstore_write(). >> > > As the patch says, the UEFI spec is very clear on which combinations > of calls are not reentrant. > I don't think the rtc_lock and the efivars->lock quite cover that completely ... After doing a bit more research, I still think there is work needed if we aim to adhere to the UEFI spec, or at least be safe from the hazards it points out. So the current status is: - get/set time calls are serialized with respect to one another using rtc_lock at the wrapper level - get/set variable calls are serialized using the efivars->lock in the efivars module - get_next_variable() calls use the BKL The two things I am most concerned with are: - reset system while other calls are in flight; is this handled implicitly in some other way? - things like settime()/setwakeuptime() and setvariable() poking into the flash at the same time. Perhaps it would be sufficient to have a single spinlock cover all these cases? Or just let efivars grab the rtc_lock as well? -- Ard. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
