Hi Matthew, Janis, On Sat, 23 Nov 2024 at 21:57, Matthew Garrett <mj...@srcf.ucam.org> wrote: > > From: Janis Danisevskis <jdanisevs...@aurora.tech> > > 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 <jdanisevs...@aurora.tech> > Signed-off-by: Matthew Garrett <mgarr...@aurora.tech> > --- > > 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. > + > /** > * 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 > 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 > Thanks /Ilias