[PATCH] efi_loader : Suppress error print message

Sun Jan 28 10:20:38 CET 2024

On 1/28/24 06:20, Bhumkar, Tejas Arvind wrote:
> [AMD Official Use Only - General]
>
> Hi Heinrich,
>
>> -----Original Message-----
>> From: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> Sent: Wednesday, January 24, 2024 2:09 AM
>> To: Bhumkar, Tejas Arvind <tejas.arvind.bhumkar at amd.com>
>> Cc: u-boot at lists.denx.de; trini at konsulko.com; sjg at chromium.org; Simek, Michal
>> <michal.simek at amd.com>; Abbarapu, Venkatesh
>> <venkatesh.abbarapu at amd.com>; git at xilinx.com; Ilias Apalodimas
>> <ilias.apalodimas at linaro.org>
>> 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 19:53, Bhumkar, Tejas Arvind wrote:
>>> [AMD Official Use Only - General]
>>>
>>> Hi Ilias & Heinrich,
>>>
>>>> -----Original Message-----
>>>> From: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>> Sent: Tuesday, January 23, 2024 3:30 PM
>>>> To: Ilias Apalodimas <ilias.apalodimas at linaro.org>; Bhumkar, Tejas
>>>> Arvind <tejas.arvind.bhumkar at amd.com>
>>>> Cc: u-boot at lists.denx.de; xypron.glpk at gmx.de; trini at konsulko.com;
>>>> sjg at chromium.org; Simek, Michal <michal.simek at amd.com>; Abbarapu,
>>>> Venkatesh <venkatesh.abbarapu at amd.com>; git at 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.bhumkar at 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-sjoerd@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.
>
> [Tejas]: I've provided two logs: one obtained from the latest u-boot origin/master, resulting in the "ERROR: reserving fdt memory region failed" error, and the other from reverting commit 06d514d77c37 ("lmb: consider EFI memory map"), which leads to the "efi_free_pool: illegal free" error.
>
> Thank You,
> Tejas.

Where can I find these logs?

Best regards

Heinrich

>
>>
>> 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.bhumkar at 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
>>>>>>
>>>
>