[V2] efi_loader: loosen buffer parameter check in efi_file_read_int

Message ID 20210428135401.22365-1-peng.fan@oss.nxp.com
State New
Headers show
Series
  • [V2] efi_loader: loosen buffer parameter check in efi_file_read_int
Related show

Commit Message

Peng Fan (OSS) April 28, 2021, 1:54 p.m.
From: Peng Fan <peng.fan@nxp.com>


This is same issue as https://bugzilla.redhat.com/show_bug.cgi?id=1733817,
but that fix was wrongly partial reverted.

When reading a directory, EFI_BUFFER_TOO_SMALL should be returned when
the supplied buffer is too small, so a use-case is to call
EFI_FILE_PROTOCOL.Read() with *buffer_size=0 and buffer=NULL to
obtain the needed size before doing the actual read.

So remove the check only for directory reading, file reading already
do the check by itself.

Fixes: db12f518edb0("efi_loader: implement non-blocking file services")
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Stefan Sørensen <stefan.sorensen@spectralink.com>
Tested-by: Peter Robinson <pbrobinson@gmail.com>

Signed-off-by: Peng Fan <peng.fan@nxp.com>

---

V2:
 Update commit log Per Heinrich's comments.
 Add T-b tag

 lib/efi_loader/efi_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.30.0

Comments

Heinrich Schuchardt April 28, 2021, 2:06 p.m. | #1
On 28.04.21 15:54, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>

>

> This is same issue as https://bugzilla.redhat.com/show_bug.cgi?id=1733817,

> but that fix was wrongly partial reverted.

>

> When reading a directory, EFI_BUFFER_TOO_SMALL should be returned when

> the supplied buffer is too small, so a use-case is to call

> EFI_FILE_PROTOCOL.Read() with *buffer_size=0 and buffer=NULL to

> obtain the needed size before doing the actual read.

>

> So remove the check only for directory reading, file reading already

> do the check by itself.

>

> Fixes: db12f518edb0("efi_loader: implement non-blocking file services")

> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>

> Cc: Stefan Sørensen <stefan.sorensen@spectralink.com>

> Tested-by: Peter Robinson <pbrobinson@gmail.com>

> Signed-off-by: Peng Fan <peng.fan@nxp.com>


Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>


> ---

>

> V2:

>  Update commit log Per Heinrich's comments.

>  Add T-b tag

>

>  lib/efi_loader/efi_file.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c

> index 204105e25a..6b3f5962be 100644

> --- a/lib/efi_loader/efi_file.c

> +++ b/lib/efi_loader/efi_file.c

> @@ -554,7 +554,7 @@ static efi_status_t efi_file_read_int(struct efi_file_handle *this,

>  	efi_status_t ret = EFI_SUCCESS;

>  	u64 bs;

>

> -	if (!this || !buffer_size || !buffer)

> +	if (!this || !buffer_size)

>  		return EFI_INVALID_PARAMETER;

>

>  	bs = *buffer_size;

>
Michal Simek April 30, 2021, 11:29 a.m. | #2
st 28. 4. 2021 v 15:22 odesílatel Peng Fan (OSS) <peng.fan@oss.nxp.com> napsal:
>

> From: Peng Fan <peng.fan@nxp.com>

>

> This is same issue as https://bugzilla.redhat.com/show_bug.cgi?id=1733817,

> but that fix was wrongly partial reverted.

>

> When reading a directory, EFI_BUFFER_TOO_SMALL should be returned when

> the supplied buffer is too small, so a use-case is to call

> EFI_FILE_PROTOCOL.Read() with *buffer_size=0 and buffer=NULL to

> obtain the needed size before doing the actual read.

>

> So remove the check only for directory reading, file reading already

> do the check by itself.

>

> Fixes: db12f518edb0("efi_loader: implement non-blocking file services")

> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>

> Cc: Stefan Sørensen <stefan.sorensen@spectralink.com>

> Tested-by: Peter Robinson <pbrobinson@gmail.com>

> Signed-off-by: Peng Fan <peng.fan@nxp.com>

> ---

>

> V2:

>  Update commit log Per Heinrich's comments.

>  Add T-b tag

>

>  lib/efi_loader/efi_file.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c

> index 204105e25a..6b3f5962be 100644

> --- a/lib/efi_loader/efi_file.c

> +++ b/lib/efi_loader/efi_file.c

> @@ -554,7 +554,7 @@ static efi_status_t efi_file_read_int(struct efi_file_handle *this,

>         efi_status_t ret = EFI_SUCCESS;

>         u64 bs;

>

> -       if (!this || !buffer_size || !buffer)

> +       if (!this || !buffer_size)

>                 return EFI_INVALID_PARAMETER;

>

>         bs = *buffer_size;

> --

> 2.30.0

>


Tested-by: Michal Simek <michal.simek@xilinx.com>


Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs

Patch

diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
index 204105e25a..6b3f5962be 100644
--- a/lib/efi_loader/efi_file.c
+++ b/lib/efi_loader/efi_file.c
@@ -554,7 +554,7 @@  static efi_status_t efi_file_read_int(struct efi_file_handle *this,
 	efi_status_t ret = EFI_SUCCESS;
 	u64 bs;
 
-	if (!this || !buffer_size || !buffer)
+	if (!this || !buffer_size)
 		return EFI_INVALID_PARAMETER;
 
 	bs = *buffer_size;