From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 26B1E941DBE for ; Tue, 26 Mar 2024 20:01:42 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=92k5wZUmhypYzt/xiarFxht8iArCmTdX4bfx3oR5B9o=; c=relaxed/simple; d=groups.io; h=Subject:To:From:User-Agent:MIME-Version:Date:Message-ID:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20240206; t=1711483301; v=1; b=vZU2ssyiqg4JxXJw/Ps5wSidop4ss+yivNCaM9i1f3RQ9GH63nNo0at7gvY4GIEMURSokFea j4vYmir1c1QgWvsWiawHOHisOhUwHpYnFhJUgHhLWKRxhjzq7qlo5CweWpJSraOWyjiHXREjUr2 mX55wj1Ubs8qDbePlRCP+RbRu/BCv69zrMsJ1BCarC8VaDzugJJq9p+gX9L+uRA0OXeiD1Bd4MG 1rceoQojga2TyReBpeNtXpS6WJ++QIi4NqkY8JM0UPTkLirjG4VSVCiXrCFN7RiMgPuFKF1icXx Xnnq45jyMQ3nXSSEiB53p7upXt2/n/YKYX+Wcgthuvuyw== X-Received: by 127.0.0.2 with SMTP id rkfOYY7687511xiFBFRj436Z; Tue, 26 Mar 2024 13:01:41 -0700 Subject: [edk2-devel] [PATCH 1/1] OvmfPkg: Align the SEC module within OvmfPkgX64 To: devel@edk2.groups.io From: "Glenn Griffin" X-Originating-Location: Seattle, Washington, US (104.132.166.64) X-Originating-Platform: Linux Chrome 123 User-Agent: GROUPS.IO Web Poster MIME-Version: 1.0 Date: Tue, 26 Mar 2024 09:52:43 -0700 Message-ID: Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ggriffiniii@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: yHCTwaSO1hFaophDHhgGcxgzx7686176AA= Content-Type: multipart/alternative; boundary="YxJsqEKiROGCkWqR2baq" X-Spam-Flag: yes X-Spam-Level: ************ X-GND-Spam-Score: 190 X-GND-Status: SPAM Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=vZU2ssyi; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io --YxJsqEKiROGCkWqR2baq Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Prior to this change the alignment of the SEC module would be 4 bytes. This is inconsistent with the expectations of the compiler and can lead to unexpected behavior. For example a modern version of clang with size optimizations enabled (-Oz) can break the ALIGN_POINTER macro in the SEC module. Here is a minimal example: https://godbolt.org/z/fzW67ndTY ``` unsigned char mArray[112]; #define ALIGN_VALUE(Value, Alignment)=C2=A0 ((Value) + (((Alignment) - (Val= ue)) & ((Alignment) - 1))) #define ALIGN_POINTER(Pointer, Alignment)=C2=A0 ((void *) (ALIGN_VALUE ((ui= nt64_t)(Pointer), (Alignment)))) int main() { printf("0x%llx\n", mArray); printf("0x%llx\n", ALIGN_POINTER(mArray, 64)); return 0; } ``` The above code compiles down to: ``` main:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0# @main push=C2=A0 =C2=A0 r14 push=C2=A0 =C2=A0 rbx push=C2=A0 =C2=A0 rax lea=C2=A0 =C2=A0 =C2=A0rbx, [rip + .L.str] lea=C2=A0 =C2=A0 =C2=A0r14, [rip + mArray] mov=C2=A0 =C2=A0 =C2=A0rdi, rbx mov=C2=A0 =C2=A0 =C2=A0rsi, r14 xor=C2=A0 =C2=A0 =C2=A0eax, eax call=C2=A0 =C2=A0 printf@PLT push=C2=A0 =C2=A0 64 pop=C2=A0 =C2=A0 =C2=A0rsi sub=C2=A0 =C2=A0 =C2=A0esi, r14d and=C2=A0 =C2=A0 =C2=A0esi, 48 add=C2=A0 =C2=A0 =C2=A0rsi, r14 mov=C2=A0 =C2=A0 =C2=A0rdi, rbx xor=C2=A0 =C2=A0 =C2=A0eax, eax call=C2=A0 =C2=A0 printf@PLT xor=C2=A0 =C2=A0 =C2=A0eax, eax add=C2=A0 =C2=A0 =C2=A0rsp, 8 pop=C2=A0 =C2=A0 =C2=A0rbx pop=C2=A0 =C2=A0 =C2=A0r14 ret mArray: .zero=C2=A0 =C2=A0112 .L.str: .asciz=C2=A0 "0x%llx\n" ``` Note that the `value & 63` in the ALIGN_VALUE implementation gets transformed into `and esi, 48` in the assembly. The compiler knows the array address is already aligned so it believes there is no need to clear the last 4 bits of the address. However by mapping the data section that contains mArray onto a 4-byte-aligned base address we violate the compiler's expectations. The last 4 bits of the mArray address are no longer zeroes leading to an ALIGN_POINTER macro that doesn't work. Cc: Ard Biesheuvel Signed-off-by: Glenn Griffin --- OvmfPkg/OvmfPkgX64.fdf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf index eb3fb90cb8b6..8b60355de40b 100644 --- a/OvmfPkg/OvmfPkgX64.fdf +++ b/OvmfPkg/OvmfPkgX64.fdf @@ -434,7 +434,7 @@ [FV.FVMAIN_COMPACT] [Rule.Common.SEC] FILE SEC =3D $(NAMED_GUID) { -=C2=A0 =C2=A0 PE32=C2=A0 =C2=A0 =C2=A0PE32=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0$(INF_OUTPUT)/$(MODULE_NAME).efi +=C2=A0 =C2=A0 PE32=C2=A0 =C2=A0 =C2=A0PE32=C2=A0 =C2=A0Align=3DAuto=C2=A0 = =C2=A0 $(INF_OUTPUT)/$(MODULE_NAME).efi UI=C2=A0 =C2=A0 =C2=A0 =C2=A0STRING =3D"$(MODULE_NAME)" Optional VERSION=C2=A0 STRING =3D"$(INF_VERSION)" Optional BUILD_NUM=3D$(BUILD_NUMBE= R) } -- 2.44.0.396.g6e790dbe36-goog -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#117138): https://edk2.groups.io/g/devel/message/117138 Mute This Topic: https://groups.io/mt/105165329/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --YxJsqEKiROGCkWqR2baq Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable
Prior to this change the alignment of the SEC module would be 4 bytes.=
This is inconsistent with the expectations of the compiler and can lea= d
to unexpected behavior.
 
For example a modern version of clang with size optimizations enabled<= /div>
(-Oz) can break the ALIGN_POINTER macro in the SEC module.
 
Here is a minimal example:
 
https://godbolt.org/z/fzW67ndTY
 
```
unsigned char mArray[112];
 
#define ALIGN_VALUE(Value, Alignment)  ((Value) + (((Alignment) -= (Value)) & ((Alignment) - 1)))
#define ALIGN_POINTER(Pointer, Alignment)  ((void *) (ALIGN_VALUE= ((uint64_t)(Pointer), (Alignment))))
 
int main() {
    printf("0x%llx\n", mArray);
    printf("0x%llx\n", ALIGN_POINTER(mArray, 64));
    return 0;
}
```
 
The above code compiles down to:
 
```
main:                  &n= bsp;                # @main
        push    r14
        push    rbx
        push    rax
        lea     rbx, [rip + .L.str]=
        lea     r14, [rip + mArray]=
        mov     rdi, rbx
        mov     rsi, r14
        xor     eax, eax
        call    printf@PLT
        push    64
        pop     rsi
        sub     esi, r14d
        and     esi, 48
        add     rsi, r14
        mov     rdi, rbx
        xor     eax, eax
        call    printf@PLT
        xor     eax, eax
        add     rsp, 8
        pop     rbx
        pop     r14
        ret
mArray:
        .zero   112
 
.L.str:
        .asciz  "0x%llx\n"
```
 
Note that the `value & 63` in the ALIGN_VALUE implementation gets<= /div>
transformed into `and esi, 48` in the assembly. The compiler knows
the array address is already aligned so it believes there is no need t= o
clear the last 4 bits of the address.
 
However by mapping the data section that contains mArray onto a
4-byte-aligned base address we violate the compiler's expectations. Th= e
last 4 bits of the mArray address are no longer zeroes leading to an
ALIGN_POINTER macro that doesn't work.
 
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
 
Signed-off-by: Glenn Griffin <ggriffiniii@gmail.com>
---
 OvmfPkg/OvmfPkgX64.fdf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index eb3fb90cb8b6..8b60355de40b 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -434,7 +434,7 @@ [FV.FVMAIN_COMPACT]
 
 [Rule.Common.SEC]
   FILE SEC =3D $(NAMED_GUID) {
-    PE32     PE32       =    $(INF_OUTPUT)/$(MODULE_NAME).efi
+    PE32     PE32   Align=3DAuto&n= bsp;   $(INF_OUTPUT)/$(MODULE_NAME).efi
     UI       STRING =3D"$(MODULE_N= AME)" Optional
     VERSION  STRING =3D"$(INF_VERSION)" Optional = BUILD_NUM=3D$(BUILD_NUMBER)
   }
-- 
2.44.0.396.g6e790dbe36-goog
 
_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#117138) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--YxJsqEKiROGCkWqR2baq--