[PATCH v2 2/3] efi: Convert device_path allocation to use malloc()
Ilias Apalodimas
ilias.apalodimas at linaro.org
Wed Aug 14 15:07:19 CEST 2024
Hi Simon
On Wed, 14 Aug 2024 at 15:40, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Ilias,
>
> On Wed, 14 Aug 2024 at 05:03, Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Thu, 1 Aug 2024 at 20:36, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Currently this calls efi_alloc() which reserves a page for each
> > > allocation and this can overwrite memory that will be used for reading
> > > images.
> > >
> > > Switch the code to use malloc(), as with other parts of EFI, such as
> > > efi_add_protocol().
> > >
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > ---
> > >
> > > (no changes since v1)
> >
> > What about https://lore.kernel.org/u-boot/CAFLszTjLAKaYK_jLXJ7Z571L-QMtoiqE-oxHCRs186dZ2Qok5g@mail.gmail.com/
> > ?
>
> Yes, I reordered the patches in this series.
You don't need to reorder them. As Heinrich already pointed out some
of these functions are used in EFI protocols. e.g
duplicate_device_path() requires the memory to be allocated by EFI
memory services, you can't just replace it with malloc.
Thanks
/Ilias
>
> Regards,
> Simon
>
>
> >
> > Thanks
> > /Ilias
> > >
> > > lib/efi_loader/efi_device_path.c | 43 ++++++++++++------------
> > > lib/efi_loader/efi_device_path_to_text.c | 2 +-
> > > 2 files changed, 22 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> > > index 0f684590f22..47038e8e585 100644
> > > --- a/lib/efi_loader/efi_device_path.c
> > > +++ b/lib/efi_loader/efi_device_path.c
> > > @@ -262,7 +262,7 @@ struct efi_device_path *efi_dp_dup(const struct efi_device_path *dp)
> > > if (!dp)
> > > return NULL;
> > >
> > > - ndp = efi_alloc(sz);
> > > + ndp = malloc(sz);
> > > if (!ndp)
> > > return NULL;
> > > memcpy(ndp, dp, sz);
> > > @@ -315,7 +315,7 @@ efi_device_path *efi_dp_concat(const struct efi_device_path *dp1,
> > > end_size = 2 * sizeof(END);
> > > else
> > > end_size = sizeof(END);
> > > - p = efi_alloc(sz1 + sz2 + end_size);
> > > + p = malloc(sz1 + sz2 + end_size);
> > > if (!p)
> > > return NULL;
> > > ret = p;
> > > @@ -347,7 +347,7 @@ struct efi_device_path *efi_dp_append_node(const struct efi_device_path *dp,
> > > ret = efi_dp_dup(dp);
> > > } else if (!dp) {
> > > size_t sz = node->length;
> > > - void *p = efi_alloc(sz + sizeof(END));
> > > + void *p = malloc(sz + sizeof(END));
> > > if (!p)
> > > return NULL;
> > > memcpy(p, node, sz);
> > > @@ -356,7 +356,7 @@ struct efi_device_path *efi_dp_append_node(const struct efi_device_path *dp,
> > > } else {
> > > /* both dp and node are non-null */
> > > size_t sz = efi_dp_size(dp);
> > > - void *p = efi_alloc(sz + node->length + sizeof(END));
> > > + void *p = malloc(sz + node->length + sizeof(END));
> > > if (!p)
> > > return NULL;
> > > memcpy(p, dp, sz);
> > > @@ -377,7 +377,7 @@ struct efi_device_path *efi_dp_create_device_node(const u8 type,
> > > if (length < sizeof(struct efi_device_path))
> > > return NULL;
> > >
> > > - ret = efi_alloc(length);
> > > + ret = malloc(length);
> > > if (!ret)
> > > return ret;
> > > ret->type = type;
> > > @@ -399,7 +399,7 @@ struct efi_device_path *efi_dp_append_instance(
> > > return efi_dp_dup(dpi);
> > > sz = efi_dp_size(dp);
> > > szi = efi_dp_instance_size(dpi);
> > > - p = efi_alloc(sz + szi + 2 * sizeof(END));
> > > + p = malloc(sz + szi + 2 * sizeof(END));
> > > if (!p)
> > > return NULL;
> > > ret = p;
> > > @@ -424,7 +424,7 @@ struct efi_device_path *efi_dp_get_next_instance(struct efi_device_path **dp,
> > > if (!dp || !*dp)
> > > return NULL;
> > > sz = efi_dp_instance_size(*dp);
> > > - p = efi_alloc(sz + sizeof(END));
> > > + p = malloc(sz + sizeof(END));
> > > if (!p)
> > > return NULL;
> > > memcpy(p, *dp, sz + sizeof(END));
> > > @@ -825,11 +825,11 @@ struct efi_device_path *efi_dp_from_part(struct blk_desc *desc, int part)
> > > {
> > > void *buf, *start;
> > >
> > > - start = buf = efi_alloc(dp_part_size(desc, part) + sizeof(END));
> > > - if (!buf)
> > > + start = malloc(dp_part_size(desc, part) + sizeof(END));
> > > + if (!start)
> > > return NULL;
> > >
> > > - buf = dp_part_fill(buf, desc, part);
> > > + buf = dp_part_fill(start, desc, part);
> > >
> > > *((struct efi_device_path *)buf) = END;
> > >
> > > @@ -852,7 +852,7 @@ struct efi_device_path *efi_dp_part_node(struct blk_desc *desc, int part)
> > > dpsize = sizeof(struct efi_device_path_cdrom_path);
> > > else
> > > dpsize = sizeof(struct efi_device_path_hard_drive_path);
> > > - buf = efi_alloc(dpsize);
> > > + buf = malloc(dpsize);
> > >
> > > if (buf)
> > > dp_part_node(buf, desc, part);
> > > @@ -912,7 +912,7 @@ struct efi_device_path *efi_dp_from_file(const struct efi_device_path *dp,
> > > if (fpsize > U16_MAX)
> > > return NULL;
> > >
> > > - buf = efi_alloc(dpsize + fpsize + sizeof(END));
> > > + buf = malloc(dpsize + fpsize + sizeof(END));
> > > if (!buf)
> > > return NULL;
> > >
> > > @@ -940,7 +940,7 @@ struct efi_device_path *efi_dp_from_uart(void)
> > > struct efi_device_path_uart *uart;
> > > size_t dpsize = dp_size(dm_root()) + sizeof(*uart) + sizeof(END);
> > >
> > > - buf = efi_alloc(dpsize);
> > > + buf = malloc(dpsize);
> > > if (!buf)
> > > return NULL;
> > > pos = dp_fill(buf, dm_root());
> > > @@ -963,11 +963,11 @@ struct efi_device_path __maybe_unused *efi_dp_from_eth(void)
> > >
> > > dpsize += dp_size(eth_get_dev());
> > >
> > > - start = buf = efi_alloc(dpsize + sizeof(END));
> > > - if (!buf)
> > > + start = malloc(dpsize + sizeof(END));
> > > + if (!start)
> > > return NULL;
> > >
> > > - buf = dp_fill(buf, eth_get_dev());
> > > + buf = dp_fill(start, eth_get_dev());
> > >
> > > *((struct efi_device_path *)buf) = END;
> > >
> > > @@ -980,22 +980,21 @@ struct efi_device_path *efi_dp_from_mem(uint32_t memory_type,
> > > uint64_t end_address)
> > > {
> > > struct efi_device_path_memory *mdp;
> > > - void *buf, *start;
> > > + void *start;
> > >
> > > - start = buf = efi_alloc(sizeof(*mdp) + sizeof(END));
> > > - if (!buf)
> > > + start = malloc(sizeof(*mdp) + sizeof(END));
> > > + if (!start)
> > > return NULL;
> > >
> > > - mdp = buf;
> > > + mdp = start;
> > > mdp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
> > > mdp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MEMORY;
> > > mdp->dp.length = sizeof(*mdp);
> > > mdp->memory_type = memory_type;
> > > mdp->start_address = start_address;
> > > mdp->end_address = end_address;
> > > - buf = &mdp[1];
> > >
> > > - *((struct efi_device_path *)buf) = END;
> > > + *((struct efi_device_path *)&mdp[1]) = END;
> > >
> > > return start;
> > > }
> > > diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
> > > index 0c7b30a26e7..4192581ac45 100644
> > > --- a/lib/efi_loader/efi_device_path_to_text.c
> > > +++ b/lib/efi_loader/efi_device_path_to_text.c
> > > @@ -33,7 +33,7 @@ static u16 *efi_str_to_u16(char *str)
> > > u16 *out, *dst;
> > >
> > > len = sizeof(u16) * (utf8_utf16_strlen(str) + 1);
> > > - out = efi_alloc(len);
> > > + out = malloc(len);
> > > if (!out)
> > > return NULL;
> > > dst = out;
> > > --
> > > 2.34.1
> > >
More information about the U-Boot
mailing list