-
Notifications
You must be signed in to change notification settings - Fork 299
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
realloc
copies incorrect amount of memory, fails running under Mu firmware
#538
Comments
Yes. Because |
#543 (comment) suggests importing fixes from EDK2, which sounds good to me. |
There is one long-standing problem in CRT realloc wrapper, which will cause the obvious buffer overflow issue when re-allocating one bigger memory block: void *realloc (void *ptr, size_t size) { // // BUG: hardcode OldSize == size! We have no any knowledge about // memory size of original pointer ptr. // return ReallocatePool ((UINTN) size, (UINTN) size, ptr); } This patch introduces one extra header to record the memory buffer size information when allocating memory block from malloc routine, and re-wrap the realloc() and free() routines to remove this BUG. Cc: Laszlo Ersek <[email protected]> Cc: Ting Ye <[email protected]> Cc: Jian J Wang <[email protected]> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Qin Long <[email protected]> Reviewed-by: Jian J Wang <[email protected]> Validated-by: Jian J Wang <[email protected]> Cherry picked from https://github.com/tianocore/edk2.git, commit cf8197a39d07179027455421a182598bd6989999. Changes: * `SIGNATURE_32` -> `EFI_SIGNATURE_32` * Added definition of `MIN` Fixes rhboot#538 Signed-off-by: Nicholas Bishop <[email protected]>
There is one long-standing problem in CRT realloc wrapper, which will cause the obvious buffer overflow issue when re-allocating one bigger memory block: void *realloc (void *ptr, size_t size) { // // BUG: hardcode OldSize == size! We have no any knowledge about // memory size of original pointer ptr. // return ReallocatePool ((UINTN) size, (UINTN) size, ptr); } This patch introduces one extra header to record the memory buffer size information when allocating memory block from malloc routine, and re-wrap the realloc() and free() routines to remove this BUG. Cc: Laszlo Ersek <[email protected]> Cc: Ting Ye <[email protected]> Cc: Jian J Wang <[email protected]> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Qin Long <[email protected]> Reviewed-by: Jian J Wang <[email protected]> Validated-by: Jian J Wang <[email protected]> Cherry picked from https://github.com/tianocore/edk2.git, commit cf8197a39d07179027455421a182598bd6989999. Changes: * `SIGNATURE_32` -> `EFI_SIGNATURE_32` * Added definition of `MIN` Fixes #538 Signed-off-by: Nicholas Bishop <[email protected]>
There is one long-standing problem in CRT realloc wrapper, which will cause the obvious buffer overflow issue when re-allocating one bigger memory block: void *realloc (void *ptr, size_t size) { // // BUG: hardcode OldSize == size! We have no any knowledge about // memory size of original pointer ptr. // return ReallocatePool ((UINTN) size, (UINTN) size, ptr); } This patch introduces one extra header to record the memory buffer size information when allocating memory block from malloc routine, and re-wrap the realloc() and free() routines to remove this BUG. Cc: Laszlo Ersek <[email protected]> Cc: Ting Ye <[email protected]> Cc: Jian J Wang <[email protected]> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Qin Long <[email protected]> Reviewed-by: Jian J Wang <[email protected]> Validated-by: Jian J Wang <[email protected]> Cherry picked from https://github.com/tianocore/edk2.git, commit cf8197a39d07179027455421a182598bd6989999. Changes: * `SIGNATURE_32` -> `EFI_SIGNATURE_32` * Added definition of `MIN` Fixes rhboot#538 Signed-off-by: Nicholas Bishop <[email protected]>
realloc
is implemented here: https://github.com/rhboot/shim/blob/main/Cryptlib/SysCall/BaseMemAllocation.c#L29It has a comment already pointing out the problem:
As currently implemented, it passes the desired size to
ReallocatePool
for both the old and new size. This happens to work fine usually, but I found when trying to test NX protection using https://github.com/microsoft/mu_tiano_platforms it reliably faults there.I tried a very quick & hacky fix by statically keeping track of allocations in a fixed-size buffer so that
realloc
knows the allocation size, and that seemed to do the trick. As far as I can tell openssl requires a functioningrealloc
, so I think some variation of manually tracking allocations will be needed since UEFI doesn't provide such an interface. I don't have a PR for this yet, but figured I'd go ahead and file an issue for awareness.The text was updated successfully, but these errors were encountered: