Leif, Heinrich,
Thank you for your comments. I should have been more careful in reading UEFI specification :)
On Thu, Aug 09, 2018 at 02:08:32PM +0100, Leif Lindholm wrote:
On Thu, Aug 09, 2018 at 03:15:38PM +0900, AKASHI Takahiro wrote:
The commit 21b3edfc964 ("efi_loader: check parameters of CreateEvent") enforces a strict parameter check at CreateEvent(). Unfortunately, however, EDK2's Shell.efi calls this function with notify_tpl == 0.
I find this done in CreatePopulateInstallShellProtocol() in Application/Shell/ShellProtocol.c, is that the one you see?
Right.
The patch above does right thing and we'd better fix the issue on EDK2 side, and yet we might want a workaround allowing for running un-modified version of EDK2 in short-term solution.
Where we find non-spec-compliant code in EDK2, we want to fix EDK2. That doesn't mean that we don't perhaps want to work around it in U-Boot anyway. But if we do, I would prefer if we could spam the console a bit as well, to warn people of badly behaving apps.
However...
The patch provides a minimum mitigation of parameter check.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
lib/efi_loader/efi_boottime.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 2281703f261..e7a19c35415 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -627,7 +627,8 @@ efi_status_t efi_create_event(uint32_t type, efi_uintn_t notify_tpl, return EFI_INVALID_PARAMETER; }
- if (is_valid_tpl(notify_tpl) != EFI_SUCCESS)
- /* notify_tpl == 0: workaround for EDK2's Shell.efi */
- if (notify_tpl && (is_valid_tpl(notify_tpl) != EFI_SUCCESS))
From the UEFI spec (2.7) description of CreateEvent() boot service:
The EVT_NOTIFY_WAIT and EVT_NOTIFY_SIGNAL flags are exclusive. If neither flag is specified, the caller does not require any notification concerning the event and the NotifyTpl, NotifyFunction, and NotifyContext parameters are ignored.
So it's not a workaround for Shell specifically. However, based on that text, something like
if (type & (EVT_NOTIFY_WAIT | EVT_NOTIFY_SIGNAL)) if ((is_valid_tpl(notify_tpl) != EFI_SUCCESS))may resolve this in a more compliant way.
OK. I will respin my patch, also addressing Heinrich's comments.
-Takahiro AKASHI
Of course, this may require additional changes to the remainder of the function.
/ Leif
return EFI_INVALID_PARAMETER;evt = calloc(1, sizeof(struct efi_event));
2.18.0