diff mbox series

efi_loader: replace a u16_strdup with alloc + memcpy

Message ID 20221111180431.1307444-1-ilias.apalodimas@linaro.org
State Accepted
Commit a930d69baa958d5f308b3910187c5f3c083fe171
Headers show
Series efi_loader: replace a u16_strdup with alloc + memcpy | expand

Commit Message

Ilias Apalodimas Nov. 11, 2022, 6:04 p.m. UTC
Heinrich reports that on RISC-V unaligned access is emulated by OpenSBI
which is very slow.  Performance wise it's better if we skip the calls
to u16_strdup() -- which in turn calls u16_strsize() and just allocate/copy the
memory directly.  The access to dp.length may still be unaligned, but that's
way less than what u16_strsize() would do

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 lib/efi_loader/efi_file.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Heinrich Schuchardt Nov. 14, 2022, 8:33 a.m. UTC | #1
On 11/11/22 19:04, Ilias Apalodimas wrote:
> Heinrich reports that on RISC-V unaligned access is emulated by OpenSBI
> which is very slow.  Performance wise it's better if we skip the calls
> to u16_strdup() -- which in turn calls u16_strsize() and just allocate/copy the
> memory directly.  The access to dp.length may still be unaligned, but that's
> way less than what u16_strsize() would do
> 
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   lib/efi_loader/efi_file.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
> index 750d380ad967..ff731db6cb8c 100644
> --- a/lib/efi_loader/efi_file.c
> +++ b/lib/efi_loader/efi_file.c
> @@ -1128,6 +1128,7 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp)
>   			container_of(fp, struct efi_device_path_file_path, dp);
>   		struct efi_file_handle *f2;
>   		u16 *filename;
> +		size_t filename_sz;
>   
>   		if (!EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) {
>   			printf("bad file path!\n");
> @@ -1140,9 +1141,14 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp)
>   		 * protocol member functions to be aligned.  So memcpy it
>   		 * unconditionally
>   		 */
> -		filename = u16_strdup(fdp->str);
> +		if (fdp->dp.length <= offsetof(struct efi_device_path_file_path, str))
> +			return NULL;
> +		filename_sz = fdp->dp.length -
> +			offsetof(struct efi_device_path_file_path, str);
> +		filename = calloc(1, filename_sz);

As we copy filename_sz bytes there is not need for zeroing out. We can 
use malloc() here.

Otherwise:
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

>   		if (!filename)
>   			return NULL;
> +		memcpy(filename, fdp->str, filename_sz);
>   		EFI_CALL(ret = f->open(f, &f2, filename,
>   				       EFI_FILE_MODE_READ, 0));
>   		free(filename);
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
index 750d380ad967..ff731db6cb8c 100644
--- a/lib/efi_loader/efi_file.c
+++ b/lib/efi_loader/efi_file.c
@@ -1128,6 +1128,7 @@  struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp)
 			container_of(fp, struct efi_device_path_file_path, dp);
 		struct efi_file_handle *f2;
 		u16 *filename;
+		size_t filename_sz;
 
 		if (!EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) {
 			printf("bad file path!\n");
@@ -1140,9 +1141,14 @@  struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp)
 		 * protocol member functions to be aligned.  So memcpy it
 		 * unconditionally
 		 */
-		filename = u16_strdup(fdp->str);
+		if (fdp->dp.length <= offsetof(struct efi_device_path_file_path, str))
+			return NULL;
+		filename_sz = fdp->dp.length -
+			offsetof(struct efi_device_path_file_path, str);
+		filename = calloc(1, filename_sz);
 		if (!filename)
 			return NULL;
+		memcpy(filename, fdp->str, filename_sz);
 		EFI_CALL(ret = f->open(f, &f2, filename,
 				       EFI_FILE_MODE_READ, 0));
 		free(filename);