On Tue, 13 Aug 2019 at 22:14, Matthew Garrett <mjg59@xxxxxxxxxx> wrote: > > On Tue, Aug 13, 2019 at 4:28 AM Laszlo Ersek <lersek@xxxxxxxxxx> wrote: > > (I verified yesterday, using the edk2 source code, that there is no > > varstore reclaim after ExitBootServices(), indeed.) > > Some implementations do reclaim at runtime, in which case the > create/delete dance will permit variable creation. > > > (a) Attempting to delete the dummy variable in efi_enter_virtual_mode(). > > To be clear, the dummy variable should never actually come into > existence - we explicitly attempt to create a variable that's bigger > than the available space, so the expectation is that it will always > fail. However, should it somehow end up being created, there's a race > between the creation and the deletion and so there's a (small) risk > that the variable actually ends up there. The cleanup in > enter_virtual_mode() is just there to ensure that anything that did > end up being created on a previous boot is deleted - the expectation > is that it'll be a noop. > > > (b) The following part, in efi_query_variable_store(): > > > > + /* > > + * The runtime code may now have triggered a garbage collection > > + * run, so check the variable info again > > + */ > > > > Let me start with (b). That code is essentially dead, I would say, based > > on the information that had already been captured in the commit message > > of [1]. Reclaim would never happen after ExitBootServices(). (I assume > > efi_query_variable_store() is only invoked after ExitBootServices(), > > i.e., from kernel space proper -- sorry if that's a wrong assumption.) > > It's dead code on Tiano, but not on at least one vendor implementation. > > > Considering (a): what justified the attempt to delete the dummy variable > > in efi_enter_virtual_mode(), in commit [4]? Was that meant as a > > fail-safe just so we don't leave a dummy variable lying around? > > Yes. > > > So even if we consider the "clean DUMMY object" hunk from [4] a > > justified fail-safe for the normal boot path, it doesn't apply to the > > kexec path -- the cold-booted primary kernel will have gone through > > those motions already, will it not? > > > > Therefore, we should do two things: > > > > - Remove the cleanup from the kexec path -- the cleanup logic from [4], > > even if justified for the cold boot path, should have never modified > > the kexec path. > > I agree that there's no benefit in it being called in the kexec path. Can I take that as an ack? _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec
