On 1/23/24 19:53, Bhumkar, Tejas Arvind wrote:
[AMD Official Use Only - General]

Hi Ilias & Heinrich,

-----Original Message-----
From: Heinrich Schuchardt <xypron.g...@gmx.de>
Sent: Tuesday, January 23, 2024 3:30 PM
To: Ilias Apalodimas <ilias.apalodi...@linaro.org>; Bhumkar, Tejas Arvind
<tejas.arvind.bhum...@amd.com>
Cc: u-boot@lists.denx.de; xypron.g...@gmx.de; tr...@konsulko.com;
s...@chromium.org; Simek, Michal <michal.si...@amd.com>; Abbarapu,
Venkatesh <venkatesh.abbar...@amd.com>; g...@xilinx.com
Subject: Re: [PATCH] efi_loader : Suppress error print message

Caution: This message originated from an External Source. Use proper caution
when opening attachments, clicking links, or responding.


On 1/23/24 09:33, Ilias Apalodimas wrote:
Hi Tejas,

On Mon, 22 Jan 2024 at 21:12, Tejas Bhumkar
<tejas.arvind.bhum...@amd.com> wrote:

Currently, on certain Xilinx platforms, an issue has been identified,
manifesting as follows:

Hello Tejas,

thank you for bringing forward this issue.

Which defconfig are you relating to?

[Tejas]:
Checking with xilinx_zynqmp_virt_defconfig



Starting kernel ...

efi_free_pool: illegal free 0x0000000077830040
efi_free_pool: illegal free 0x000000007782d040
efi_free_pool: illegal free 0x000000007782c040

The issue arises when the ramdisk image is relocated, placing it
within the previously allocated EFI memory region( as EFI is
established quite early in U-Boot).

Which version of U-Boot are you on? Commit 06d514d77c37 ("lmb: consider EFI
memory map") was meant to avoid such a situation.

[Tejas] :
I'm verifying against the latest changes in the master branch. The introduction of commit 
06d514d77c37 ("lmb: consider EFI memory map") has resolved the occurrence of the 
"efi_free_pool: illegal free" error. but, it leads to a new error, as detailed in the 
following patch: https://lore.kernel.org/all/20230212150706.2967007-1-sjo...@collabora.com/

Could you, please, try to reproduce your issues with origin/master as of
today and provide the full boot log. Please, also provide the output of
the bdinfo and the printenv command as well as the sizes of the kernel
and the RAM disk.

Best regards

Heinrich




I don't mind suppressing the print for some time, but out of
curiosity, how is the ramdisk relocated? LMB should be aware of the
EFI regions by then, so I assume the relocation code doesn't check
those?


[Tejas] :  I observe that the relocation of the RAM disk is taking place in the 
line below.
https://github.com/u-boot/u-boot/blob/master/lib/lmb.c#L480-L491
Yes, the relocated code specifically examines the LMB region and does not 
consider the EFI region.


The indicated situation is serious. If the EFI sub-system is using the same 
memory
area as the ramdisk, this may lead to corruption of the ramdisk. Suppressing the
messages is by no means adequate to solve the problem.

[Tejas] :
The challenge arises when relocating the ramdisk image, inserting it into the 
previously assigned EFI memory region,
established early in U-Boot. Consequently, when attempting to release memory in 
the EFI region during the handover
process to the kernel in the efi_free_pool function, we first verify if the 
memory was allocated by efi_allocate_pool().
The issue originates from a checksum mismatch because, during the ramdisk 
relocation, we overwrite memory allocated by EFI.
This leads to the appearance of the error message: efi_free_pool: illegal free 
0x0000000077830040.
Crucially, there is no corruption of the ramdisk seen since we do not actually 
releasing memory due to the checksum mismatch.
In our testing, this issue was observed only when the ramdisk size exceeded 
approximately 24 MB.


Best regards

Heinrich



Thanks
/Ilias

Consequently, when
attempting to release memory in the EFI memory region during the
handover process to the kernel,we encounter memory violations.

Highlighting that EFI remains active primarily during the booting of
an EFI application, and the lmb persists while configuring images for
the boot process. Since we aren't utilizing the EFI memory region
during the boot process, there is no adverse impact even in the event
of a violation.

Currently, there is an ongoing discussion regarding the handling
strategies of three memory allocators: malloc, lmb, and EFI. This
discussion is documented in the email chain titled "Proposal: U-Boot
memory management."

Therefore, it is advisable to suppress the print message during the
boot process for now.

Signed-off-by: Tejas Bhumkar <tejas.arvind.bhum...@amd.com>
---
   lib/efi_loader/efi_memory.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_memory.c
b/lib/efi_loader/efi_memory.c index edfad2d95a..821fe7616e 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -713,7 +713,7 @@ efi_status_t efi_free_pool(void *buffer)
          /* Check that this memory was allocated by efi_allocate_pool() */
          if (((uintptr_t)alloc & EFI_PAGE_MASK) ||
              alloc->checksum != checksum(alloc)) {
-               printf("%s: illegal free 0x%p\n", __func__, buffer);
+               debug("%s: illegal free 0x%p\n", __func__, buffer);
                  return EFI_INVALID_PARAMETER;
          }
          /* Avoid double free */
--
2.27.0



Reply via email to