Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix free null ptr in uefi_file.c #5

Closed
wants to merge 1 commit into from
Closed

Conversation

chnzzh
Copy link

@chnzzh chnzzh commented May 30, 2023

Hi @pbatard

I am using the Linux platform to generate OVMF image with this ntfs-3g driver.

I have successfully built the OVMF.fd image, but when I try to enter the EFI Shell, it gets stuck at the loading screen, only display a white cursor.

2023-05-30 203458

I have printed the debug information to the stdio, and it appears as follows:

ASSERT_EFI_ERROR (Status = Invalid Parameter)
ASSERT [ntfs] /home/zzh/uefi_pj/edk3/edk2/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c(821): !(((INTN)(RETURN_STATUS)(Status)) < 0)

After debugging, I found that the issue may be caused by freeing a null pointer in uefi_file.c at line 195.

Therefore, I added a condition to check for this, and now it has been fixed. The EFI Shell can be accessed normally.

@pbatard
Copy link
Owner

pbatard commented May 30, 2023

Thanks for the report.

This is not a problem that will occur on real hardware, since in release builds asserts are disabled and I did design the code for NULL to potentially be passed to FreePool() (based on the expectation from regular memory freeing calls that passing NULL as an argument is acceptable).

So, while I will integrate this fix, I am not planning to create a new release of the NTFS driver specifically for it or treat it as an urgent issue to fix.

What will most likely happen is that I will change all the FreePool() used by the driver to a SafeFreePool() version that both accepts NULL without triggering an assert and that nulls the pointer on exit, to also take care of potential double-free errors.

@pbatard pbatard self-assigned this May 30, 2023
@pbatard pbatard closed this in dc64886 Jun 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants