| Message ID | 20221111180431.1307444-1-ilias.apalodimas@linaro.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: replace a u16_strdup with alloc + memcpy | expand |
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 --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);
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(-)