xen/common/efi/boot.c | 46 ++++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 14 deletions(-)
Although code is compiled with -fpic option data is not position
independent. This causes data pointer to become invalid if
code is not relocated properly which is what happens for
efi_multiboot2 which is called by multiboot entry code.
Code tested adding
PrintErrMesg(L"Test message", EFI_BUFFER_TOO_SMALL);
in efi_multiboot2 before calling efi_arch_edd (this function
can potentially call PrintErrMesg).
Before the patch (XenServer installation on Qemu, xen replaced
with vanilla xen.gz):
Booting `XenServer (Serial)'Booting `XenServer (Serial)'
Test message: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!!
ExceptionData - 0000000000000000 I:0 R:0 U:0 W:0 P:0 PK:0 SS:0 SGX:0
RIP - 000000007DC29E46, CS - 0000000000000038, RFLAGS - 0000000000210246
RAX - 0000000000000000, RCX - 0000000000000050, RDX - 0000000000000000
RBX - 000000007DAB4558, RSP - 000000007EFA1200, RBP - 0000000000000000
RSI - FFFF82D040467A88, RDI - 0000000000000000
R8 - 000000007EFA1238, R9 - 000000007EFA1230, R10 - 0000000000000000
R11 - 000000007CF42665, R12 - FFFF82D040467A88, R13 - 000000007EFA1228
R14 - 000000007EFA1225, R15 - 000000007DAB45A8
DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030
GS - 0000000000000030, SS - 0000000000000030
CR0 - 0000000080010033, CR2 - FFFF82D040467A88, CR3 - 000000007EC01000
CR4 - 0000000000000668, CR8 - 0000000000000000
DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000
DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400
GDTR - 000000007E9E2000 0000000000000047, LDTR - 0000000000000000
IDTR - 000000007E4E5018 0000000000000FFF, TR - 0000000000000000
FXSAVE_STATE - 000000007EFA0E60
!!!! Find image based on IP(0x7DC29E46) (No PDB) (ImageBase=000000007DC28000, EntryPoint=000000007DC2B917) !!!!
After the patch:
Booting `XenServer (Serial)'Booting `XenServer (Serial)'
Test message: Buffer too small
BdsDxe: loading Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
BdsDxe: starting Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331)
Fixes: 00d5d5ce23e6 ("work around Clang generating .data.rel.ro section for init-only files")
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
xen/common/efi/boot.c | 46 ++++++++++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 14 deletions(-)
---
Changes since v1:
- added "Fixes:" tag;
- fixed cast style change.
Changes since v2:
- wrap long line.
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index efbec00af9..fdbe75005c 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -287,19 +287,36 @@ static bool __init match_guid(const EFI_GUID *guid1, const EFI_GUID *guid2)
/* generic routine for printing error messages */
static void __init PrintErrMesg(const CHAR16 *mesg, EFI_STATUS ErrCode)
{
- static const CHAR16* const ErrCodeToStr[] __initconstrel = {
- [~EFI_ERROR_MASK & EFI_NOT_FOUND] = L"Not found",
- [~EFI_ERROR_MASK & EFI_NO_MEDIA] = L"The device has no media",
- [~EFI_ERROR_MASK & EFI_MEDIA_CHANGED] = L"Media changed",
- [~EFI_ERROR_MASK & EFI_DEVICE_ERROR] = L"Device error",
- [~EFI_ERROR_MASK & EFI_VOLUME_CORRUPTED] = L"Volume corrupted",
- [~EFI_ERROR_MASK & EFI_ACCESS_DENIED] = L"Access denied",
- [~EFI_ERROR_MASK & EFI_OUT_OF_RESOURCES] = L"Out of resources",
- [~EFI_ERROR_MASK & EFI_VOLUME_FULL] = L"Volume is full",
- [~EFI_ERROR_MASK & EFI_SECURITY_VIOLATION] = L"Security violation",
- [~EFI_ERROR_MASK & EFI_CRC_ERROR] = L"CRC error",
- [~EFI_ERROR_MASK & EFI_COMPROMISED_DATA] = L"Compromised data",
- [~EFI_ERROR_MASK & EFI_BUFFER_TOO_SMALL] = L"Buffer too small",
+#define ERROR_MESSAGE_LIST \
+ ERROR_MESSAGE(EFI_NOT_FOUND, "Not found") \
+ ERROR_MESSAGE(EFI_NO_MEDIA, "The device has no media") \
+ ERROR_MESSAGE(EFI_MEDIA_CHANGED, "Media changed") \
+ ERROR_MESSAGE(EFI_DEVICE_ERROR, "Device error") \
+ ERROR_MESSAGE(EFI_VOLUME_CORRUPTED, "Volume corrupted") \
+ ERROR_MESSAGE(EFI_ACCESS_DENIED, "Access denied") \
+ ERROR_MESSAGE(EFI_OUT_OF_RESOURCES, "Out of resources") \
+ ERROR_MESSAGE(EFI_VOLUME_FULL, "Volume is full") \
+ ERROR_MESSAGE(EFI_SECURITY_VIOLATION, "Security violation") \
+ ERROR_MESSAGE(EFI_CRC_ERROR, "CRC error") \
+ ERROR_MESSAGE(EFI_COMPROMISED_DATA, "Compromised data") \
+ ERROR_MESSAGE(EFI_BUFFER_TOO_SMALL, "Buffer too small")
+
+ static const struct ErrorStrings {
+ CHAR16 start;
+#undef ERROR_MESSAGE
+#define ERROR_MESSAGE(code, str) CHAR16 msg_ ## code[sizeof(str)];
+ ERROR_MESSAGE_LIST
+ } ErrorStrings __initconst = {
+ 0
+#undef ERROR_MESSAGE
+#define ERROR_MESSAGE(code, str) , L ## str
+ ERROR_MESSAGE_LIST
+ };
+ static const uint16_t ErrCodeToStr[] __initconst = {
+#undef ERROR_MESSAGE
+#define ERROR_MESSAGE(code, str) \
+ [~EFI_ERROR_MASK & code] = offsetof(struct ErrorStrings, msg_ ## code),
+ ERROR_MESSAGE_LIST
};
EFI_STATUS ErrIdx = ErrCode & ~EFI_ERROR_MASK;
@@ -308,7 +325,8 @@ static void __init PrintErrMesg(const CHAR16 *mesg, EFI_STATUS ErrCode)
PrintErr(L": ");
if( (ErrIdx < ARRAY_SIZE(ErrCodeToStr)) && ErrCodeToStr[ErrIdx] )
- mesg = ErrCodeToStr[ErrIdx];
+ mesg = (const CHAR16 *)((const void *)&ErrorStrings +
+ ErrCodeToStr[ErrIdx]);
else
{
PrintErr(L"ErrCode: ");
--
2.46.0
On 19.08.2024 14:54, Frediano Ziglio wrote: > Although code is compiled with -fpic option data is not position > independent. This causes data pointer to become invalid if > code is not relocated properly which is what happens for > efi_multiboot2 which is called by multiboot entry code. > > Code tested adding > PrintErrMesg(L"Test message", EFI_BUFFER_TOO_SMALL); > in efi_multiboot2 before calling efi_arch_edd (this function > can potentially call PrintErrMesg). > > Before the patch (XenServer installation on Qemu, xen replaced > with vanilla xen.gz): > Booting `XenServer (Serial)'Booting `XenServer (Serial)' > Test message: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!! > ExceptionData - 0000000000000000 I:0 R:0 U:0 W:0 P:0 PK:0 SS:0 SGX:0 > RIP - 000000007DC29E46, CS - 0000000000000038, RFLAGS - 0000000000210246 > RAX - 0000000000000000, RCX - 0000000000000050, RDX - 0000000000000000 > RBX - 000000007DAB4558, RSP - 000000007EFA1200, RBP - 0000000000000000 > RSI - FFFF82D040467A88, RDI - 0000000000000000 > R8 - 000000007EFA1238, R9 - 000000007EFA1230, R10 - 0000000000000000 > R11 - 000000007CF42665, R12 - FFFF82D040467A88, R13 - 000000007EFA1228 > R14 - 000000007EFA1225, R15 - 000000007DAB45A8 > DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030 > GS - 0000000000000030, SS - 0000000000000030 > CR0 - 0000000080010033, CR2 - FFFF82D040467A88, CR3 - 000000007EC01000 > CR4 - 0000000000000668, CR8 - 0000000000000000 > DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000 > DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400 > GDTR - 000000007E9E2000 0000000000000047, LDTR - 0000000000000000 > IDTR - 000000007E4E5018 0000000000000FFF, TR - 0000000000000000 > FXSAVE_STATE - 000000007EFA0E60 > !!!! Find image based on IP(0x7DC29E46) (No PDB) (ImageBase=000000007DC28000, EntryPoint=000000007DC2B917) !!!! > > After the patch: > Booting `XenServer (Serial)'Booting `XenServer (Serial)' > Test message: Buffer too small > BdsDxe: loading Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331) > BdsDxe: starting Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331) > > Fixes: 00d5d5ce23e6 ("work around Clang generating .data.rel.ro section for init-only files") > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com> > --- > xen/common/efi/boot.c | 46 ++++++++++++++++++++++++++++++------------- > 1 file changed, 32 insertions(+), 14 deletions(-) > --- > Changes since v1: > - added "Fixes:" tag; > - fixed cast style change. > > Changes since v2: > - wrap long line. And what about the Fixes: tag? Jan
On Mon, Aug 19, 2024 at 2:19 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 19.08.2024 14:54, Frediano Ziglio wrote: > > Although code is compiled with -fpic option data is not position > > independent. This causes data pointer to become invalid if > > code is not relocated properly which is what happens for > > efi_multiboot2 which is called by multiboot entry code. > > > > Code tested adding > > PrintErrMesg(L"Test message", EFI_BUFFER_TOO_SMALL); > > in efi_multiboot2 before calling efi_arch_edd (this function > > can potentially call PrintErrMesg). > > > > Before the patch (XenServer installation on Qemu, xen replaced > > with vanilla xen.gz): > > Booting `XenServer (Serial)'Booting `XenServer (Serial)' > > Test message: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!! > > ExceptionData - 0000000000000000 I:0 R:0 U:0 W:0 P:0 PK:0 SS:0 SGX:0 > > RIP - 000000007DC29E46, CS - 0000000000000038, RFLAGS - 0000000000210246 > > RAX - 0000000000000000, RCX - 0000000000000050, RDX - 0000000000000000 > > RBX - 000000007DAB4558, RSP - 000000007EFA1200, RBP - 0000000000000000 > > RSI - FFFF82D040467A88, RDI - 0000000000000000 > > R8 - 000000007EFA1238, R9 - 000000007EFA1230, R10 - 0000000000000000 > > R11 - 000000007CF42665, R12 - FFFF82D040467A88, R13 - 000000007EFA1228 > > R14 - 000000007EFA1225, R15 - 000000007DAB45A8 > > DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030 > > GS - 0000000000000030, SS - 0000000000000030 > > CR0 - 0000000080010033, CR2 - FFFF82D040467A88, CR3 - 000000007EC01000 > > CR4 - 0000000000000668, CR8 - 0000000000000000 > > DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000 > > DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400 > > GDTR - 000000007E9E2000 0000000000000047, LDTR - 0000000000000000 > > IDTR - 000000007E4E5018 0000000000000FFF, TR - 0000000000000000 > > FXSAVE_STATE - 000000007EFA0E60 > > !!!! Find image based on IP(0x7DC29E46) (No PDB) (ImageBase=000000007DC28000, EntryPoint=000000007DC2B917) !!!! > > > > After the patch: > > Booting `XenServer (Serial)'Booting `XenServer (Serial)' > > Test message: Buffer too small > > BdsDxe: loading Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331) > > BdsDxe: starting Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331) > > > > Fixes: 00d5d5ce23e6 ("work around Clang generating .data.rel.ro section for init-only files") ^^^^ here ?? > > Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com> > > --- > > xen/common/efi/boot.c | 46 ++++++++++++++++++++++++++++++------------- > > 1 file changed, 32 insertions(+), 14 deletions(-) > > --- > > Changes since v1: > > - added "Fixes:" tag; > > - fixed cast style change. > > > > Changes since v2: > > - wrap long line. > > And what about the Fixes: tag? > > Jan
On 19.08.2024 16:02, Frediano Ziglio wrote: > On Mon, Aug 19, 2024 at 2:19 PM Jan Beulich <jbeulich@suse.com> wrote: >> >> On 19.08.2024 14:54, Frediano Ziglio wrote: >>> Although code is compiled with -fpic option data is not position >>> independent. This causes data pointer to become invalid if >>> code is not relocated properly which is what happens for >>> efi_multiboot2 which is called by multiboot entry code. >>> >>> Code tested adding >>> PrintErrMesg(L"Test message", EFI_BUFFER_TOO_SMALL); >>> in efi_multiboot2 before calling efi_arch_edd (this function >>> can potentially call PrintErrMesg). >>> >>> Before the patch (XenServer installation on Qemu, xen replaced >>> with vanilla xen.gz): >>> Booting `XenServer (Serial)'Booting `XenServer (Serial)' >>> Test message: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!! >>> ExceptionData - 0000000000000000 I:0 R:0 U:0 W:0 P:0 PK:0 SS:0 SGX:0 >>> RIP - 000000007DC29E46, CS - 0000000000000038, RFLAGS - 0000000000210246 >>> RAX - 0000000000000000, RCX - 0000000000000050, RDX - 0000000000000000 >>> RBX - 000000007DAB4558, RSP - 000000007EFA1200, RBP - 0000000000000000 >>> RSI - FFFF82D040467A88, RDI - 0000000000000000 >>> R8 - 000000007EFA1238, R9 - 000000007EFA1230, R10 - 0000000000000000 >>> R11 - 000000007CF42665, R12 - FFFF82D040467A88, R13 - 000000007EFA1228 >>> R14 - 000000007EFA1225, R15 - 000000007DAB45A8 >>> DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030 >>> GS - 0000000000000030, SS - 0000000000000030 >>> CR0 - 0000000080010033, CR2 - FFFF82D040467A88, CR3 - 000000007EC01000 >>> CR4 - 0000000000000668, CR8 - 0000000000000000 >>> DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000 >>> DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400 >>> GDTR - 000000007E9E2000 0000000000000047, LDTR - 0000000000000000 >>> IDTR - 000000007E4E5018 0000000000000FFF, TR - 0000000000000000 >>> FXSAVE_STATE - 000000007EFA0E60 >>> !!!! Find image based on IP(0x7DC29E46) (No PDB) (ImageBase=000000007DC28000, EntryPoint=000000007DC2B917) !!!! >>> >>> After the patch: >>> Booting `XenServer (Serial)'Booting `XenServer (Serial)' >>> Test message: Buffer too small >>> BdsDxe: loading Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331) >>> BdsDxe: starting Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331) >>> >>> Fixes: 00d5d5ce23e6 ("work around Clang generating .data.rel.ro section for init-only files") > > ^^^^ here ?? Did you not see ... >>> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com> >>> --- >>> xen/common/efi/boot.c | 46 ++++++++++++++++++++++++++++++------------- >>> 1 file changed, 32 insertions(+), 14 deletions(-) >>> --- >>> Changes since v1: >>> - added "Fixes:" tag; >>> - fixed cast style change. >>> >>> Changes since v2: >>> - wrap long line. >> >> And what about the Fixes: tag? ... my respective v2 comment then? There I said: "I don't think this is right. While this is where the array was introduced, it was correct at that time afaict. It went wrong when MB2 support was added about a year later. 9180f5365524 ("x86: add multiboot2 protocol support for EFI platforms") may be reasonable to blame, albeit I'm not sure that was the final one, after which MB2 support was considered complete." Jan
On Mon, Aug 19, 2024 at 3:14 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 19.08.2024 16:02, Frediano Ziglio wrote: > > On Mon, Aug 19, 2024 at 2:19 PM Jan Beulich <jbeulich@suse.com> wrote: > >> > >> On 19.08.2024 14:54, Frediano Ziglio wrote: > >>> Although code is compiled with -fpic option data is not position > >>> independent. This causes data pointer to become invalid if > >>> code is not relocated properly which is what happens for > >>> efi_multiboot2 which is called by multiboot entry code. > >>> > >>> Code tested adding > >>> PrintErrMesg(L"Test message", EFI_BUFFER_TOO_SMALL); > >>> in efi_multiboot2 before calling efi_arch_edd (this function > >>> can potentially call PrintErrMesg). > >>> > >>> Before the patch (XenServer installation on Qemu, xen replaced > >>> with vanilla xen.gz): > >>> Booting `XenServer (Serial)'Booting `XenServer (Serial)' > >>> Test message: !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!! > >>> ExceptionData - 0000000000000000 I:0 R:0 U:0 W:0 P:0 PK:0 SS:0 SGX:0 > >>> RIP - 000000007DC29E46, CS - 0000000000000038, RFLAGS - 0000000000210246 > >>> RAX - 0000000000000000, RCX - 0000000000000050, RDX - 0000000000000000 > >>> RBX - 000000007DAB4558, RSP - 000000007EFA1200, RBP - 0000000000000000 > >>> RSI - FFFF82D040467A88, RDI - 0000000000000000 > >>> R8 - 000000007EFA1238, R9 - 000000007EFA1230, R10 - 0000000000000000 > >>> R11 - 000000007CF42665, R12 - FFFF82D040467A88, R13 - 000000007EFA1228 > >>> R14 - 000000007EFA1225, R15 - 000000007DAB45A8 > >>> DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030 > >>> GS - 0000000000000030, SS - 0000000000000030 > >>> CR0 - 0000000080010033, CR2 - FFFF82D040467A88, CR3 - 000000007EC01000 > >>> CR4 - 0000000000000668, CR8 - 0000000000000000 > >>> DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000 > >>> DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400 > >>> GDTR - 000000007E9E2000 0000000000000047, LDTR - 0000000000000000 > >>> IDTR - 000000007E4E5018 0000000000000FFF, TR - 0000000000000000 > >>> FXSAVE_STATE - 000000007EFA0E60 > >>> !!!! Find image based on IP(0x7DC29E46) (No PDB) (ImageBase=000000007DC28000, EntryPoint=000000007DC2B917) !!!! > >>> > >>> After the patch: > >>> Booting `XenServer (Serial)'Booting `XenServer (Serial)' > >>> Test message: Buffer too small > >>> BdsDxe: loading Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331) > >>> BdsDxe: starting Boot0000 "UiApp" from Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(462CAA21-7614-4503-836E-8AB6F4662331) > >>> > >>> Fixes: 00d5d5ce23e6 ("work around Clang generating .data.rel.ro section for init-only files") > > > > ^^^^ here ?? > > Did you not see ... > > >>> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com> > >>> --- > >>> xen/common/efi/boot.c | 46 ++++++++++++++++++++++++++++++------------- > >>> 1 file changed, 32 insertions(+), 14 deletions(-) > >>> --- > >>> Changes since v1: > >>> - added "Fixes:" tag; > >>> - fixed cast style change. > >>> > >>> Changes since v2: > >>> - wrap long line. > >> > >> And what about the Fixes: tag? > > ... my respective v2 comment then? There I said: > > "I don't think this is right. While this is where the array was introduced, > it was correct at that time afaict. It went wrong when MB2 support was added > about a year later. 9180f5365524 ("x86: add multiboot2 protocol support for > EFI platforms") may be reasonable to blame, albeit I'm not sure that was the > final one, after which MB2 support was considered complete." > > Jan Yes, missed, updated Frediano
© 2016 - 2024 Red Hat, Inc.