| Message ID | 20240528144252.179247-2-heinrich.schuchardt@canonical.com |
|---|---|
| State | Superseded |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: improve device-tree loading | expand |
On Tue, 28 May 2024 at 17:43, Heinrich Schuchardt <heinrich.schuchardt@canonical.com> wrote: > > Allow appending a device-path to a device-path that contains an end node > as separator. We need this feature for creating boot options specifying > kernel, initrd, and dtb. > > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > --- > v2: > update efi_dp_concat() instead of new function efi_dp_merge() > --- > cmd/eficonfig.c | 6 +++--- > cmd/efidebug.c | 4 ++-- > include/efi_loader.h | 2 +- > lib/efi_loader/efi_bootbin.c | 2 +- > lib/efi_loader/efi_bootmgr.c | 2 +- > lib/efi_loader/efi_boottime.c | 2 +- > lib/efi_loader/efi_device_path.c | 20 +++++++++++++------- > lib/efi_loader/efi_device_path_utilities.c | 2 +- > 8 files changed, 23 insertions(+), 17 deletions(-) > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c > index 0ba92c60e03..b13d9a3d2d9 100644 > --- a/cmd/eficonfig.c > +++ b/cmd/eficonfig.c > @@ -531,7 +531,7 @@ struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_ > dp = efi_dp_shorten(dp_volume); > if (!dp) > dp = dp_volume; > - dp = efi_dp_concat(dp, &fp->dp, false); > + dp = efi_dp_concat(dp, &fp->dp, 0); > free(buf); > > return dp; > @@ -1485,7 +1485,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo > goto out; > } > initrd_dp = efi_dp_concat((const struct efi_device_path *)&id_dp, > - dp, false); > + dp, 0); > efi_free_pool(dp); > } > > @@ -1496,7 +1496,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo > } > final_dp_size = efi_dp_size(dp) + sizeof(END); > if (initrd_dp) { > - final_dp = efi_dp_concat(dp, initrd_dp, true); > + final_dp = efi_dp_concat(dp, initrd_dp, 1); > final_dp_size += efi_dp_size(initrd_dp) + sizeof(END); > } else { > final_dp = efi_dp_dup(dp); > diff --git a/cmd/efidebug.c b/cmd/efidebug.c > index c2c525f2351..762027daf8a 100644 > --- a/cmd/efidebug.c > +++ b/cmd/efidebug.c > @@ -697,7 +697,7 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part, > short_fp = tmp_fp; > > initrd_dp = efi_dp_concat((const struct efi_device_path *)&id_dp, > - short_fp, false); > + short_fp, 0); > > out: > efi_free_pool(tmp_dp); > @@ -917,7 +917,7 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, > goto out; > } > > - final_fp = efi_dp_concat(file_path, initrd_dp, true); > + final_fp = efi_dp_concat(file_path, initrd_dp, 1); > if (!final_fp) { > printf("Cannot create final device path\n"); > r = CMD_RET_FAILURE; > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 9600941aa32..ddf2e41a95c 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -946,7 +946,7 @@ struct efi_device_path *efi_dp_from_lo(struct efi_load_option *lo, > const efi_guid_t *guid); > struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1, > const struct efi_device_path *dp2, > - bool split_end_node); > + size_t split_end_node); > struct efi_device_path *search_gpt_dp_node(struct efi_device_path *device_path); > efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8 *data, > efi_uintn_t *size); > diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c > index b7910f78fb6..a87006b3c0e 100644 > --- a/lib/efi_loader/efi_bootbin.c > +++ b/lib/efi_loader/efi_bootbin.c > @@ -150,7 +150,7 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) > msg_path = file_path; > } else { > file_path = efi_dp_concat(bootefi_device_path, > - bootefi_image_path, false); > + bootefi_image_path, 0); > msg_path = bootefi_image_path; > log_debug("Loaded from disk\n"); > } > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c > index 7da3139f917..b0bf21cf841 100644 > --- a/lib/efi_loader/efi_bootmgr.c > +++ b/lib/efi_loader/efi_bootmgr.c > @@ -130,7 +130,7 @@ static efi_status_t try_load_from_file_path(efi_handle_t *fs_handles, > if (!dp) > continue; > > - dp = efi_dp_concat(dp, fp, false); > + dp = efi_dp_concat(dp, fp, 0); > if (!dp) > continue; > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index 1951291747c..630c5f52c4f 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -1816,7 +1816,7 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > if (device_path) { > info->device_handle = efi_dp_find_obj(device_path, NULL, NULL); > > - dp = efi_dp_concat(device_path, file_path, false); > + dp = efi_dp_concat(device_path, file_path, 0); > if (!dp) { > ret = EFI_OUT_OF_RESOURCES; > goto failure; > diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c > index aec224d8466..c8c8d54f733 100644 > --- a/lib/efi_loader/efi_device_path.c > +++ b/lib/efi_loader/efi_device_path.c > @@ -276,10 +276,11 @@ struct efi_device_path *efi_dp_dup(const struct efi_device_path *dp) > * > * @dp1: First device path > * @dp2: Second device path > - * @split_end_node: If true the two device paths will be concatenated and > - * separated by an end node (DEVICE_PATH_SUB_TYPE_END). > - * If false the second device path will be concatenated to the > - * first one as-is. > + * @split_end_node: > + * * 0 to concatenate > + * * 1 to concatenate with end node added as separator > + * * size of dp1 excluding last end node to concatenate with end node as > + * separator in case dp1 contains an end node > * > * Return: > * concatenated device path or NULL. Caller must free the returned value > @@ -287,7 +288,7 @@ struct efi_device_path *efi_dp_dup(const struct efi_device_path *dp) > struct > efi_device_path *efi_dp_concat(const struct efi_device_path *dp1, > const struct efi_device_path *dp2, > - bool split_end_node) > + size_t split_end_node) > { > struct efi_device_path *ret; > size_t end_size; > @@ -301,10 +302,15 @@ efi_device_path *efi_dp_concat(const struct efi_device_path *dp1, > ret = efi_dp_dup(dp1); > } else { > /* both dp1 and dp2 are non-null */ > - unsigned sz1 = efi_dp_size(dp1); > - unsigned sz2 = efi_dp_size(dp2); > + size_t sz1; > + size_t sz2 = efi_dp_size(dp2); > void *p; > > + if (split_end_node < sizeof(struct efi_device_path)) Can we be more explicit here pls? Someone might misuse this in the future split_end_node < =1 ? And we can document we can use values up to sizeof(struct efi_device_path) if we ever need extra functionality > + sz1 = efi_dp_size(dp1); > + else > + sz1 = split_end_node; > + > if (split_end_node) > end_size = 2 * sizeof(END); > else > diff --git a/lib/efi_loader/efi_device_path_utilities.c b/lib/efi_loader/efi_device_path_utilities.c > index c95dbfa9b5f..ac250bbfcc9 100644 > --- a/lib/efi_loader/efi_device_path_utilities.c > +++ b/lib/efi_loader/efi_device_path_utilities.c > @@ -76,7 +76,7 @@ static struct efi_device_path * EFIAPI append_device_path( > const struct efi_device_path *src2) > { > EFI_ENTRY("%pD, %pD", src1, src2); > - return EFI_EXIT(efi_dp_concat(src1, src2, false)); > + return EFI_EXIT(efi_dp_concat(src1, src2, 0)); > } > > /* > -- > 2.43.0 > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
On 28.05.24 18:02, Ilias Apalodimas wrote: > On Tue, 28 May 2024 at 17:43, Heinrich Schuchardt > <heinrich.schuchardt@canonical.com> wrote: >> >> Allow appending a device-path to a device-path that contains an end node >> as separator. We need this feature for creating boot options specifying >> kernel, initrd, and dtb. >> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> >> --- >> v2: >> update efi_dp_concat() instead of new function efi_dp_merge() >> --- >> cmd/eficonfig.c | 6 +++--- >> cmd/efidebug.c | 4 ++-- >> include/efi_loader.h | 2 +- >> lib/efi_loader/efi_bootbin.c | 2 +- >> lib/efi_loader/efi_bootmgr.c | 2 +- >> lib/efi_loader/efi_boottime.c | 2 +- >> lib/efi_loader/efi_device_path.c | 20 +++++++++++++------- >> lib/efi_loader/efi_device_path_utilities.c | 2 +- >> 8 files changed, 23 insertions(+), 17 deletions(-) >> >> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c >> index 0ba92c60e03..b13d9a3d2d9 100644 >> --- a/cmd/eficonfig.c >> +++ b/cmd/eficonfig.c >> @@ -531,7 +531,7 @@ struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_ >> dp = efi_dp_shorten(dp_volume); >> if (!dp) >> dp = dp_volume; >> - dp = efi_dp_concat(dp, &fp->dp, false); >> + dp = efi_dp_concat(dp, &fp->dp, 0); >> free(buf); >> >> return dp; >> @@ -1485,7 +1485,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo >> goto out; >> } >> initrd_dp = efi_dp_concat((const struct efi_device_path *)&id_dp, >> - dp, false); >> + dp, 0); >> efi_free_pool(dp); >> } >> >> @@ -1496,7 +1496,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo >> } >> final_dp_size = efi_dp_size(dp) + sizeof(END); >> if (initrd_dp) { >> - final_dp = efi_dp_concat(dp, initrd_dp, true); >> + final_dp = efi_dp_concat(dp, initrd_dp, 1); >> final_dp_size += efi_dp_size(initrd_dp) + sizeof(END); >> } else { >> final_dp = efi_dp_dup(dp); >> diff --git a/cmd/efidebug.c b/cmd/efidebug.c >> index c2c525f2351..762027daf8a 100644 >> --- a/cmd/efidebug.c >> +++ b/cmd/efidebug.c >> @@ -697,7 +697,7 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part, >> short_fp = tmp_fp; >> >> initrd_dp = efi_dp_concat((const struct efi_device_path *)&id_dp, >> - short_fp, false); >> + short_fp, 0); >> >> out: >> efi_free_pool(tmp_dp); >> @@ -917,7 +917,7 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, >> goto out; >> } >> >> - final_fp = efi_dp_concat(file_path, initrd_dp, true); >> + final_fp = efi_dp_concat(file_path, initrd_dp, 1); >> if (!final_fp) { >> printf("Cannot create final device path\n"); >> r = CMD_RET_FAILURE; >> diff --git a/include/efi_loader.h b/include/efi_loader.h >> index 9600941aa32..ddf2e41a95c 100644 >> --- a/include/efi_loader.h >> +++ b/include/efi_loader.h >> @@ -946,7 +946,7 @@ struct efi_device_path *efi_dp_from_lo(struct efi_load_option *lo, >> const efi_guid_t *guid); >> struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1, >> const struct efi_device_path *dp2, >> - bool split_end_node); >> + size_t split_end_node); >> struct efi_device_path *search_gpt_dp_node(struct efi_device_path *device_path); >> efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8 *data, >> efi_uintn_t *size); >> diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c >> index b7910f78fb6..a87006b3c0e 100644 >> --- a/lib/efi_loader/efi_bootbin.c >> +++ b/lib/efi_loader/efi_bootbin.c >> @@ -150,7 +150,7 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) >> msg_path = file_path; >> } else { >> file_path = efi_dp_concat(bootefi_device_path, >> - bootefi_image_path, false); >> + bootefi_image_path, 0); >> msg_path = bootefi_image_path; >> log_debug("Loaded from disk\n"); >> } >> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c >> index 7da3139f917..b0bf21cf841 100644 >> --- a/lib/efi_loader/efi_bootmgr.c >> +++ b/lib/efi_loader/efi_bootmgr.c >> @@ -130,7 +130,7 @@ static efi_status_t try_load_from_file_path(efi_handle_t *fs_handles, >> if (!dp) >> continue; >> >> - dp = efi_dp_concat(dp, fp, false); >> + dp = efi_dp_concat(dp, fp, 0); >> if (!dp) >> continue; >> >> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >> index 1951291747c..630c5f52c4f 100644 >> --- a/lib/efi_loader/efi_boottime.c >> +++ b/lib/efi_loader/efi_boottime.c >> @@ -1816,7 +1816,7 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, >> if (device_path) { >> info->device_handle = efi_dp_find_obj(device_path, NULL, NULL); >> >> - dp = efi_dp_concat(device_path, file_path, false); >> + dp = efi_dp_concat(device_path, file_path, 0); >> if (!dp) { >> ret = EFI_OUT_OF_RESOURCES; >> goto failure; >> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c >> index aec224d8466..c8c8d54f733 100644 >> --- a/lib/efi_loader/efi_device_path.c >> +++ b/lib/efi_loader/efi_device_path.c >> @@ -276,10 +276,11 @@ struct efi_device_path *efi_dp_dup(const struct efi_device_path *dp) >> * >> * @dp1: First device path >> * @dp2: Second device path >> - * @split_end_node: If true the two device paths will be concatenated and >> - * separated by an end node (DEVICE_PATH_SUB_TYPE_END). >> - * If false the second device path will be concatenated to the >> - * first one as-is. >> + * @split_end_node: >> + * * 0 to concatenate >> + * * 1 to concatenate with end node added as separator >> + * * size of dp1 excluding last end node to concatenate with end node as >> + * separator in case dp1 contains an end node >> * >> * Return: >> * concatenated device path or NULL. Caller must free the returned value >> @@ -287,7 +288,7 @@ struct efi_device_path *efi_dp_dup(const struct efi_device_path *dp) >> struct >> efi_device_path *efi_dp_concat(const struct efi_device_path *dp1, >> const struct efi_device_path *dp2, >> - bool split_end_node) >> + size_t split_end_node) >> { >> struct efi_device_path *ret; >> size_t end_size; >> @@ -301,10 +302,15 @@ efi_device_path *efi_dp_concat(const struct efi_device_path *dp1, >> ret = efi_dp_dup(dp1); >> } else { >> /* both dp1 and dp2 are non-null */ >> - unsigned sz1 = efi_dp_size(dp1); >> - unsigned sz2 = efi_dp_size(dp2); >> + size_t sz1; >> + size_t sz2 = efi_dp_size(dp2); >> void *p; >> >> + if (split_end_node < sizeof(struct efi_device_path)) > > Can we be more explicit here pls? Someone might misuse this in the future > split_end_node < =1 ? And we can document we can use values up to > sizeof(struct efi_device_path) if we ever need extra functionality size_t split_end_node cannot be negative. The case split_end_node == 0 is handled below. What are you missing? Best regards Heinrich > >> + sz1 = efi_dp_size(dp1); >> + else >> + sz1 = split_end_node; >> + >> if (split_end_node) >> end_size = 2 * sizeof(END); >> else >> diff --git a/lib/efi_loader/efi_device_path_utilities.c b/lib/efi_loader/efi_device_path_utilities.c >> index c95dbfa9b5f..ac250bbfcc9 100644 >> --- a/lib/efi_loader/efi_device_path_utilities.c >> +++ b/lib/efi_loader/efi_device_path_utilities.c >> @@ -76,7 +76,7 @@ static struct efi_device_path * EFIAPI append_device_path( >> const struct efi_device_path *src2) >> { >> EFI_ENTRY("%pD, %pD", src1, src2); >> - return EFI_EXIT(efi_dp_concat(src1, src2, false)); >> + return EFI_EXIT(efi_dp_concat(src1, src2, 0)); >> } >> >> /* >> -- >> 2.43.0 >> > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
[...] > >> - unsigned sz2 = efi_dp_size(dp2); > >> + size_t sz1; > >> + size_t sz2 = efi_dp_size(dp2); > >> void *p; > >> > >> + if (split_end_node < sizeof(struct efi_device_path)) > > > > Can we be more explicit here pls? Someone might misuse this in the future > > split_end_node < =1 ? And we can document we can use values up to > > sizeof(struct efi_device_path) if we ever need extra functionality > > size_t split_end_node cannot be negative. > > The case split_end_node == 0 is handled below. What are you missing? someone misusing it and passing a value of '3' for example and print an error if the value is 1 < value < sizeof(struct efi_device_path) Thanks /Ilias > > Best regards > > Heinrich > > > > >> + sz1 = efi_dp_size(dp1); > >> + else > >> + sz1 = split_end_node; > >> + > >> if (split_end_node) > >> end_size = 2 * sizeof(END); > >> else > >> diff --git a/lib/efi_loader/efi_device_path_utilities.c b/lib/efi_loader/efi_device_path_utilities.c > >> index c95dbfa9b5f..ac250bbfcc9 100644 > >> --- a/lib/efi_loader/efi_device_path_utilities.c > >> +++ b/lib/efi_loader/efi_device_path_utilities.c > >> @@ -76,7 +76,7 @@ static struct efi_device_path * EFIAPI append_device_path( > >> const struct efi_device_path *src2) > >> { > >> EFI_ENTRY("%pD, %pD", src1, src2); > >> - return EFI_EXIT(efi_dp_concat(src1, src2, false)); > >> + return EFI_EXIT(efi_dp_concat(src1, src2, 0)); > >> } > >> > >> /* > >> -- > >> 2.43.0 > >> > > > > Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> >
On 28.05.24 18:18, Ilias Apalodimas wrote: > [...] > >>>> - unsigned sz2 = efi_dp_size(dp2); >>>> + size_t sz1; >>>> + size_t sz2 = efi_dp_size(dp2); >>>> void *p; >>>> >>>> + if (split_end_node < sizeof(struct efi_device_path)) >>> >>> Can we be more explicit here pls? Someone might misuse this in the future >>> split_end_node < =1 ? And we can document we can use values up to >>> sizeof(struct efi_device_path) if we ever need extra functionality >> >> size_t split_end_node cannot be negative. >> >> The case split_end_node == 0 is handled below. What are you missing? > > someone misusing it and passing a value of '3' for example and print > an error if the value is > 1 < value < sizeof(struct efi_device_path) I would like to avoid over-engineering. How about - if (split_end_node < sizeof(struct efi_device_path)) + if (split_end_node < 2) and changing the function description to refer to >= 2? Best regards Heinrich > > Thanks > /Ilias >> >> Best regards >> >> Heinrich >> >>> >>>> + sz1 = efi_dp_size(dp1); >>>> + else >>>> + sz1 = split_end_node; >>>> + >>>> if (split_end_node) >>>> end_size = 2 * sizeof(END); >>>> else >>>> diff --git a/lib/efi_loader/efi_device_path_utilities.c b/lib/efi_loader/efi_device_path_utilities.c >>>> index c95dbfa9b5f..ac250bbfcc9 100644 >>>> --- a/lib/efi_loader/efi_device_path_utilities.c >>>> +++ b/lib/efi_loader/efi_device_path_utilities.c >>>> @@ -76,7 +76,7 @@ static struct efi_device_path * EFIAPI append_device_path( >>>> const struct efi_device_path *src2) >>>> { >>>> EFI_ENTRY("%pD, %pD", src1, src2); >>>> - return EFI_EXIT(efi_dp_concat(src1, src2, false)); >>>> + return EFI_EXIT(efi_dp_concat(src1, src2, 0)); >>>> } >>>> >>>> /* >>>> -- >>>> 2.43.0 >>>> >>> >>> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> >>
On Tue, 28 May 2024 at 19:59, Heinrich Schuchardt <heinrich.schuchardt@canonical.com> wrote: > > On 28.05.24 18:18, Ilias Apalodimas wrote: > > [...] > > > >>>> - unsigned sz2 = efi_dp_size(dp2); > >>>> + size_t sz1; > >>>> + size_t sz2 = efi_dp_size(dp2); > >>>> void *p; > >>>> > >>>> + if (split_end_node < sizeof(struct efi_device_path)) > >>> > >>> Can we be more explicit here pls? Someone might misuse this in the future > >>> split_end_node < =1 ? And we can document we can use values up to > >>> sizeof(struct efi_device_path) if we ever need extra functionality > >> > >> size_t split_end_node cannot be negative. > >> > >> The case split_end_node == 0 is handled below. What are you missing? > > > > someone misusing it and passing a value of '3' for example and print > > an error if the value is > > 1 < value < sizeof(struct efi_device_path) > > I would like to avoid over-engineering. How about > > - if (split_end_node < sizeof(struct efi_device_path)) > + if (split_end_node < 2) > > and changing the function description to refer to >= 2? Nop, in that case, I prefer what's already there, simply because values between 0 < sizeof(struct efi_device_path) will be used for functionality similar to the one we have for 0,1 in the future Thanks /Ilias > > Best regards > > Heinrich > > > > > Thanks > > /Ilias > >> > >> Best regards > >> > >> Heinrich > >> > >>> > >>>> + sz1 = efi_dp_size(dp1); > >>>> + else > >>>> + sz1 = split_end_node; > >>>> + > >>>> if (split_end_node) > >>>> end_size = 2 * sizeof(END); > >>>> else > >>>> diff --git a/lib/efi_loader/efi_device_path_utilities.c b/lib/efi_loader/efi_device_path_utilities.c > >>>> index c95dbfa9b5f..ac250bbfcc9 100644 > >>>> --- a/lib/efi_loader/efi_device_path_utilities.c > >>>> +++ b/lib/efi_loader/efi_device_path_utilities.c > >>>> @@ -76,7 +76,7 @@ static struct efi_device_path * EFIAPI append_device_path( > >>>> const struct efi_device_path *src2) > >>>> { > >>>> EFI_ENTRY("%pD, %pD", src1, src2); > >>>> - return EFI_EXIT(efi_dp_concat(src1, src2, false)); > >>>> + return EFI_EXIT(efi_dp_concat(src1, src2, 0)); > >>>> } > >>>> > >>>> /* > >>>> -- > >>>> 2.43.0 > >>>> > >>> > >>> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > >> >
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c index 0ba92c60e03..b13d9a3d2d9 100644 --- a/cmd/eficonfig.c +++ b/cmd/eficonfig.c @@ -531,7 +531,7 @@ struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_ dp = efi_dp_shorten(dp_volume); if (!dp) dp = dp_volume; - dp = efi_dp_concat(dp, &fp->dp, false); + dp = efi_dp_concat(dp, &fp->dp, 0); free(buf); return dp; @@ -1485,7 +1485,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo goto out; } initrd_dp = efi_dp_concat((const struct efi_device_path *)&id_dp, - dp, false); + dp, 0); efi_free_pool(dp); } @@ -1496,7 +1496,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo } final_dp_size = efi_dp_size(dp) + sizeof(END); if (initrd_dp) { - final_dp = efi_dp_concat(dp, initrd_dp, true); + final_dp = efi_dp_concat(dp, initrd_dp, 1); final_dp_size += efi_dp_size(initrd_dp) + sizeof(END); } else { final_dp = efi_dp_dup(dp); diff --git a/cmd/efidebug.c b/cmd/efidebug.c index c2c525f2351..762027daf8a 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -697,7 +697,7 @@ struct efi_device_path *create_initrd_dp(const char *dev, const char *part, short_fp = tmp_fp; initrd_dp = efi_dp_concat((const struct efi_device_path *)&id_dp, - short_fp, false); + short_fp, 0); out: efi_free_pool(tmp_dp); @@ -917,7 +917,7 @@ static int do_efi_boot_add(struct cmd_tbl *cmdtp, int flag, goto out; } - final_fp = efi_dp_concat(file_path, initrd_dp, true); + final_fp = efi_dp_concat(file_path, initrd_dp, 1); if (!final_fp) { printf("Cannot create final device path\n"); r = CMD_RET_FAILURE; diff --git a/include/efi_loader.h b/include/efi_loader.h index 9600941aa32..ddf2e41a95c 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -946,7 +946,7 @@ struct efi_device_path *efi_dp_from_lo(struct efi_load_option *lo, const efi_guid_t *guid); struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1, const struct efi_device_path *dp2, - bool split_end_node); + size_t split_end_node); struct efi_device_path *search_gpt_dp_node(struct efi_device_path *device_path); efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8 *data, efi_uintn_t *size); diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index b7910f78fb6..a87006b3c0e 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -150,7 +150,7 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) msg_path = file_path; } else { file_path = efi_dp_concat(bootefi_device_path, - bootefi_image_path, false); + bootefi_image_path, 0); msg_path = bootefi_image_path; log_debug("Loaded from disk\n"); } diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 7da3139f917..b0bf21cf841 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -130,7 +130,7 @@ static efi_status_t try_load_from_file_path(efi_handle_t *fs_handles, if (!dp) continue; - dp = efi_dp_concat(dp, fp, false); + dp = efi_dp_concat(dp, fp, 0); if (!dp) continue; diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 1951291747c..630c5f52c4f 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1816,7 +1816,7 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, if (device_path) { info->device_handle = efi_dp_find_obj(device_path, NULL, NULL); - dp = efi_dp_concat(device_path, file_path, false); + dp = efi_dp_concat(device_path, file_path, 0); if (!dp) { ret = EFI_OUT_OF_RESOURCES; goto failure; diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index aec224d8466..c8c8d54f733 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -276,10 +276,11 @@ struct efi_device_path *efi_dp_dup(const struct efi_device_path *dp) * * @dp1: First device path * @dp2: Second device path - * @split_end_node: If true the two device paths will be concatenated and - * separated by an end node (DEVICE_PATH_SUB_TYPE_END). - * If false the second device path will be concatenated to the - * first one as-is. + * @split_end_node: + * * 0 to concatenate + * * 1 to concatenate with end node added as separator + * * size of dp1 excluding last end node to concatenate with end node as + * separator in case dp1 contains an end node * * Return: * concatenated device path or NULL. Caller must free the returned value @@ -287,7 +288,7 @@ struct efi_device_path *efi_dp_dup(const struct efi_device_path *dp) struct efi_device_path *efi_dp_concat(const struct efi_device_path *dp1, const struct efi_device_path *dp2, - bool split_end_node) + size_t split_end_node) { struct efi_device_path *ret; size_t end_size; @@ -301,10 +302,15 @@ efi_device_path *efi_dp_concat(const struct efi_device_path *dp1, ret = efi_dp_dup(dp1); } else { /* both dp1 and dp2 are non-null */ - unsigned sz1 = efi_dp_size(dp1); - unsigned sz2 = efi_dp_size(dp2); + size_t sz1; + size_t sz2 = efi_dp_size(dp2); void *p; + if (split_end_node < sizeof(struct efi_device_path)) + sz1 = efi_dp_size(dp1); + else + sz1 = split_end_node; + if (split_end_node) end_size = 2 * sizeof(END); else diff --git a/lib/efi_loader/efi_device_path_utilities.c b/lib/efi_loader/efi_device_path_utilities.c index c95dbfa9b5f..ac250bbfcc9 100644 --- a/lib/efi_loader/efi_device_path_utilities.c +++ b/lib/efi_loader/efi_device_path_utilities.c @@ -76,7 +76,7 @@ static struct efi_device_path * EFIAPI append_device_path( const struct efi_device_path *src2) { EFI_ENTRY("%pD, %pD", src1, src2); - return EFI_EXIT(efi_dp_concat(src1, src2, false)); + return EFI_EXIT(efi_dp_concat(src1, src2, 0)); } /*
Allow appending a device-path to a device-path that contains an end node as separator. We need this feature for creating boot options specifying kernel, initrd, and dtb. Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> --- v2: update efi_dp_concat() instead of new function efi_dp_merge() --- cmd/eficonfig.c | 6 +++--- cmd/efidebug.c | 4 ++-- include/efi_loader.h | 2 +- lib/efi_loader/efi_bootbin.c | 2 +- lib/efi_loader/efi_bootmgr.c | 2 +- lib/efi_loader/efi_boottime.c | 2 +- lib/efi_loader/efi_device_path.c | 20 +++++++++++++------- lib/efi_loader/efi_device_path_utilities.c | 2 +- 8 files changed, 23 insertions(+), 17 deletions(-)