Hi Ilias, On Mon, 25 Nov 2024 at 06:41, Ilias Apalodimas <[email protected]> wrote: > > Hi Matthew, Janis, > > On Sat, 23 Nov 2024 at 21:57, Matthew Garrett <[email protected]> wrote: > > > > From: Janis Danisevskis <[email protected]> > > > > efi_bind_block had two issues. > > 1. A pointer to a the stack was inserted as plat structure and thus used > > beyond its life time. > > Yep and I wonder how the device_unbind() code managed to survice since > it free that stack allocated memory... > Probably never called.
:-) > > > 2. Only the first segment of the device path was copied into the > > platfom data structure resulting in an unterminated device path. > > > > Signed-off-by: Janis Danisevskis <[email protected]> > > Signed-off-by: Matthew Garrett <[email protected]> > > --- > > > > lib/efi/efi_app_init.c | 29 +++++++++++++++++++++-------- > > 1 file changed, 21 insertions(+), 8 deletions(-) > > > > diff --git a/lib/efi/efi_app_init.c b/lib/efi/efi_app_init.c > > index 9704020b749..cc91e1d74b8 100644 > > --- a/lib/efi/efi_app_init.c > > +++ b/lib/efi/efi_app_init.c > > @@ -19,6 +19,15 @@ > > > > DECLARE_GLOBAL_DATA_PTR; > > > > +static size_t device_path_length(const struct efi_device_path *device_path) > > +{ > > + const struct efi_device_path *d; > > + > > + for (d = device_path; d->type != DEVICE_PATH_TYPE_END; d = (void > > *)d + d->length) { > > + } > > + return (void *)d - (void *)device_path + d->length; > > +} > > We already define efi_dp_size(), you can use that here. Unfortunately that is only available in EFI_LOADER. Should we try to split some of the code out to lib/efi ? > > > + > > /** > > * efi_bind_block() - bind a new block device to an EFI device > > * > > @@ -39,19 +48,23 @@ int efi_bind_block(efi_handle_t handle, struct > > efi_block_io *blkio, > > struct efi_device_path *device_path, int len, > > struct udevice **devp) > > { > > - struct efi_media_plat plat; > > + struct efi_media_plat *plat; > > struct udevice *dev; > > char name[18]; > > int ret; > > - > > - plat.handle = handle; > > - plat.blkio = blkio; > > - plat.device_path = malloc(device_path->length); > > - if (!plat.device_path) > > + size_t device_path_len = device_path_length(device_path); > > + > > + plat = malloc(sizeof(struct efi_media_plat)); > > + if (!plat) > > + return log_msg_ret("plat", -ENOMEM); > > + plat->handle = handle; > > + plat->blkio = blkio; > > + plat->device_path = malloc(device_path_len); > > + if (!plat->device_path) > > you need to free plat here now I will send a patch. > > > return log_msg_ret("path", -ENOMEM); > > - memcpy(plat.device_path, device_path, device_path->length); > > + memcpy(plat->device_path, device_path, device_path_len); > > ret = device_bind(dm_root(), DM_DRIVER_GET(efi_media), "efi_media", > > - &plat, ofnode_null(), &dev); > > + plat, ofnode_null(), &dev); > > if (ret) > > return log_msg_ret("bind", ret); > > > > -- > > 2.47.0 > > Regards, Simon
