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

[CI Check]ArmPkg: DefaultExceptionHandlerLib: Do Not Allocate Memory #4703

Closed

Conversation

os-d
Copy link

@os-d os-d commented Jul 31, 2023

If gST->ConOut is available when Arm's DefaultExceptionHandler is running, AsciiPrint will get called to attempt to print to ConOut, in addition to the serial output.

AsciiPrint calls AsciiInternalPrint in UefiLibPrint.c which in turn calls AllocatePool to allocate a buffer to convert the Ascii input string to a Unicode string to pass to ConOut->OutputString.

Per the comment on DefaultExceptionHandler, we should not be allocating memory in the exception handler, as this can cause the exception handler to fail if we had a memory exception or the system state is such that we cannot allocate memory.

It has been observed on ArmVirtQemu that exceptions generated in the memory handling code will fail to output the stack dump and CPU state that is critical to debugging because the AllocatePool will fail.

This patch fixes the Arm and AARCH64 DefaultExceptionHandlers to not allocate memory when ConOut is available and instead use stack memory to convert the Ascii string needed for SerialPortWrite to the Unicode string needed for ConOut->OutputString. Correspondingly, ArmVirtQemu can now output the stack dump and CPU state when hitting an exception in memory code.

Cc: Leif Lindholm quic_llindhol@quicinc.com
Cc: Ard Biesheuvel ardb+tianocore@kernel.org
Cc: Sami Mujawar sami.mujawar@arm.com
Cc: Gerd Hoffmann kraxel@redhat.com

If gST->ConOut is available when Arm's DefaultExceptionHandler
is running, AsciiPrint will get called to attempt to print to
ConOut, in addition to the serial output.

AsciiPrint calls AsciiInternalPrint in UefiLibPrint.c which
in turn calls AllocatePool to allocate a buffer to convert
the Ascii input string to a Unicode string to pass to
ConOut->OutputString.

Per the comment on DefaultExceptionHandler, we should not be
allocating memory in the exception handler, as this can cause
the exception handler to fail if we had a memory exception or
the system state is such that we cannot allocate memory.

It has been observed on ArmVirtQemu that exceptions generated
in the memory handling code will fail to output the stack dump
and CPU state that is critical to debugging because the
AllocatePool will fail.

This patch fixes the Arm and AARCH64 DefaultExceptionHandlers to
not allocate memory when ConOut is available and instead use
stack memory to convert the Ascii string needed for SerialPortWrite
to the Unicode string needed for ConOut->OutputString. Correspondingly,
ArmVirtQemu can now output the stack dump and CPU state when hitting
an exception in memory code.

Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
@os-d
Copy link
Author

os-d commented Aug 2, 2023

Closing for v2.

@os-d os-d closed this Aug 2, 2023
os-d pushed a commit to os-d/edk2 that referenced this pull request Aug 2, 2023
Changes since v1:

- Move OutputString write to after serial logging
- Drop sending "Recursive Exception" to ConOut

If gST->ConOut is available when Arm's DefaultExceptionHandler
is running, AsciiPrint will get called to attempt to print to
ConOut, in addition to the serial output.

AsciiPrint calls AsciiInternalPrint in UefiLibPrint.c which
in turn calls AllocatePool to allocate a buffer to convert
the Ascii input string to a Unicode string to pass to
ConOut->OutputString.

Per the comment on DefaultExceptionHandler, we should not be
allocating memory in the exception handler, as this can cause
the exception handler to fail if we had a memory exception or
the system state is such that we cannot allocate memory.

It has been observed on ArmVirtQemu that exceptions generated
in the memory handling code will fail to output the stack dump
and CPU state that is critical to debugging because the
AllocatePool will fail.

This patch fixes the Arm and AARCH64 DefaultExceptionHandlers to
not allocate memory when ConOut is available and instead use
stack memory to convert the Ascii string needed for SerialPortWrite
to the Unicode string needed for ConOut->OutputString. Correspondingly,
ArmVirtQemu can now output the stack dump and CPU state when hitting
an exception in memory code.

GitHub PR: tianocore#4703

Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
os-d pushed a commit to os-d/edk2 that referenced this pull request Aug 2, 2023
Changes since v1:

- Move OutputString write to after serial logging
- Drop sending "Recursive Exception" to ConOut

If gST->ConOut is available when Arm's DefaultExceptionHandler
is running, AsciiPrint will get called to attempt to print to
ConOut, in addition to the serial output.

AsciiPrint calls AsciiInternalPrint in UefiLibPrint.c which
in turn calls AllocatePool to allocate a buffer to convert
the Ascii input string to a Unicode string to pass to
ConOut->OutputString.

Per the comment on DefaultExceptionHandler, we should not be
allocating memory in the exception handler, as this can cause
the exception handler to fail if we had a memory exception or
the system state is such that we cannot allocate memory.

It has been observed on ArmVirtQemu that exceptions generated
in the memory handling code will fail to output the stack dump
and CPU state that is critical to debugging because the
AllocatePool will fail.

This patch fixes the Arm and AARCH64 DefaultExceptionHandlers to
not allocate memory when ConOut is available and instead use
stack memory to convert the Ascii string needed for SerialPortWrite
to the Unicode string needed for ConOut->OutputString. Correspondingly,
ArmVirtQemu can now output the stack dump and CPU state when hitting
an exception in memory code.

GitHub PR: tianocore#4703

Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
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.

None yet

1 participant