Re: [PATCH v2 2/3] efi: Convert device_path allocation to use malloc()

Hi Simon

On Wed, 14 Aug 2024 at 15:40, Simon Glass sjg@chromium.org wrote:

Hi Ilias,

On Wed, 14 Aug 2024 at 05:03, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

...

Hi Simon,

On Thu, 1 Aug 2024 at 20:36, Simon Glass sjg@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@chromium.org

(no changes since v1)

What about https://lore.kernel.org/u-boot/CAFLszTjLAKaYK_jLXJ7Z571L-QMtoiqE-oxHCRs186dZ... ?

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