From mboxrd@z Thu Jan  1 00:00:00 1970
From: Heinrich Schuchardt <xypron.glpk@gmx.de>
Date: Thu, 9 Aug 2018 14:21:11 +0200
Subject: [U-Boot] [RFC] efi_loader: workaround for EDK2's shell.efi
In-Reply-To: <20180809061538.6624-1-takahiro.akashi@linaro.org>
References: <20180809061538.6624-1-takahiro.akashi@linaro.org>
Message-ID: <d942b81b-eec7-8662-31fe-d43fcf2248b8@gmx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 08/09/2018 08:15 AM, AKASHI Takahiro wrote:
> The commit 21b3edfc964 ("efi_loader: check parameters of CreateEvent")

scripts/checkpatch.pl wants 12 digits for the commit reference.

ERROR: Please use git commit description style 'commit <12+ chars of
sha1> ("<title line>")'

Please, check patches before submitting.

The commit 21b3edfc9644 ("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.
> 
> 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.
> 
> The patch provides a minimum mitigation of parameter check.
> 

This patch relates to test number 5.1.1.1.7 in Self Certification Test
(SCT) II Case Specification June 2017.

Fixes: 21b3edfc9644 ("efi_loader: check parameters of CreateEvent")

> 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))

Thanks for catching this.

The UEFI 2.7 spec has the following parameter description:

NotifyTpl: The task priority level of event notifications, if needed.

CreateEvent is implemented in EDK2 CoreCreateEvent() which calls
CoreCreateEventEx(). The latter has the following test:

  if ((Type & (EVT_NOTIFY_WAIT | EVT_NOTIFY_SIGNAL)) != 0) {
    if (NotifyTpl != TPL_APPLICATION &&
        NotifyTpl != TPL_CALLBACK &&
        NotifyTpl != TPL_NOTIFY) {
      return EFI_INVALID_PARAMETER;
    }
  }

In my patch I missed to check parameter Type first.

Please, change your patch so that it matches what test case 5.1.1.1.7
checks (i.e. the EDK 2 logic).

Best regards

Heinrich

>  		return EFI_INVALID_PARAMETER;
>  
>  	evt = calloc(1, sizeof(struct efi_event));
>