diff mbox series

[v2] efi_loader: initialize return values in efi_uninstall_multiple_protocol_interfaces_int()

Message ID 20221110081155.209647-1-ilias.apalodimas@linaro.org
State New
Headers show
Series [v2] efi_loader: initialize return values in efi_uninstall_multiple_protocol_interfaces_int() | expand

Commit Message

Ilias Apalodimas Nov. 10, 2022, 8:11 a.m. UTC
If the va_list we got handed over contains no protocols we must return
EFI_SUCCESS.  However in that case the current code just returns
an uninitialized value.
Fix that by setting the return value in the variable definition

Addresses-Coverity: CID 376195:  ("Uninitialized variables  (UNINIT)")
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
changes since v1:
- return EFI_SUCCESS instead of EFI_INVALID_PARAMETER

 lib/efi_loader/efi_boottime.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Heinrich Schuchardt Nov. 10, 2022, 8:15 a.m. UTC | #1
On 11/10/22 09:11, Ilias Apalodimas wrote:
> If the va_list we got handed over contains no protocols we must return
> EFI_SUCCESS.  However in that case the current code just returns
> an uninitialized value.
> Fix that by setting the return value in the variable definition
>
> Addresses-Coverity: CID 376195:  ("Uninitialized variables  (UNINIT)")
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> changes since v1:
> - return EFI_SUCCESS instead of EFI_INVALID_PARAMETER
>
>   lib/efi_loader/efi_boottime.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index a56021559bbf..f52d086d17bf 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -2754,7 +2754,7 @@ efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
>   {
>   	const efi_guid_t *protocol;
>   	void *protocol_interface;
> -	efi_status_t ret;
> +	efi_status_t ret = EFI_SUCCESS;

OK

>   	size_t i = 0;
>   	efi_va_list argptr_copy;
>
> @@ -2765,7 +2765,7 @@ efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
>   	for (;;) {
>   		protocol = efi_va_arg(argptr, efi_guid_t*);
>   		if (!protocol)
> -			break;
> +			goto out;

You may reach this point after actually uninstalling protocols. You have
to check if the last protocol is uninstalled. Please, drop this change.

Best regards

Heinrich

>   		protocol_interface = efi_va_arg(argptr, void*);
>   		ret = efi_uninstall_protocol(handle, protocol,
>   					     protocol_interface);
Ilias Apalodimas Nov. 10, 2022, 8:18 a.m. UTC | #2
Hi Heinrich

On Thu, 10 Nov 2022 at 10:15, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 11/10/22 09:11, Ilias Apalodimas wrote:
> > If the va_list we got handed over contains no protocols we must return
> > EFI_SUCCESS.  However in that case the current code just returns
> > an uninitialized value.
> > Fix that by setting the return value in the variable definition
> >
> > Addresses-Coverity: CID 376195:  ("Uninitialized variables  (UNINIT)")
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> > changes since v1:
> > - return EFI_SUCCESS instead of EFI_INVALID_PARAMETER
> >
> >   lib/efi_loader/efi_boottime.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index a56021559bbf..f52d086d17bf 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -2754,7 +2754,7 @@ efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
> >   {
> >       const efi_guid_t *protocol;
> >       void *protocol_interface;
> > -     efi_status_t ret;
> > +     efi_status_t ret = EFI_SUCCESS;
>
> OK
>
> >       size_t i = 0;
> >       efi_va_list argptr_copy;
> >
> > @@ -2765,7 +2765,7 @@ efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
> >       for (;;) {
> >               protocol = efi_va_arg(argptr, efi_guid_t*);
> >               if (!protocol)
> > -                     break;
> > +                     goto out;
>
> You may reach this point after actually uninstalling protocols. You have
> to check if the last protocol is uninstalled. Please, drop this change.

Yes good catch.  Let me send a v3

Thanks
/Ilias
>
> Best regards
>
> Heinrich
>
> >               protocol_interface = efi_va_arg(argptr, void*);
> >               ret = efi_uninstall_protocol(handle, protocol,
> >                                            protocol_interface);
>
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index a56021559bbf..f52d086d17bf 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2754,7 +2754,7 @@  efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
 {
 	const efi_guid_t *protocol;
 	void *protocol_interface;
-	efi_status_t ret;
+	efi_status_t ret = EFI_SUCCESS;
 	size_t i = 0;
 	efi_va_list argptr_copy;
 
@@ -2765,7 +2765,7 @@  efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
 	for (;;) {
 		protocol = efi_va_arg(argptr, efi_guid_t*);
 		if (!protocol)
-			break;
+			goto out;
 		protocol_interface = efi_va_arg(argptr, void*);
 		ret = efi_uninstall_protocol(handle, protocol,
 					     protocol_interface);