On 12/08/2021 09:26, Marvin Häuser wrote:
The value of p is constant, so it can be placed in a constant data
section. p points to a global variable, so if the compiler does not
manage to somehow turn this into relative addressing (let's assume
it does not), it needs to generate a relocation. This means the
compiler cannot put it in __TEXT,__const, so it has to put it in
__DATA,__const (of course it could put it in other __DATA sections,
but let's assume the compiler agrees this should be read-only). The
very same issue will arise and no matter the choice of the compiler,
this will end up in .data. Do you agree? Or do we have some
guarantee that Apple Clang cannot emit __DATA,__const?
I don’t see your bigger point. The compiler is free to implement as
it sees fit. Which section some code ends up in is more of an
implementation detail for the compiler, and we can’t really depend on
that?
Your point, rightfully, was that things that we request to be
read-only (may) end up being read-write. My issue is that, if the
compiler requests this pointer to be read-only (it may not, but also
it may), our PE executable does not honour it either. __DATA,__const
is a section for constant data, and we put it into a read-write
section. The bigger point is, whenever the compile stack wants
something read-only (be that NASM or be that Apple Clang, anything
really), we should actually ensure it is read-only. I can do that for
only NASM by forcing the __TEXT,__const section name (at the cost of
prohibiting relocs), but I do not know how to do it for Apple Clang.
At worst we could take a hacked-ish solution, where all Mach-O
segments are converted to PE/COFF sections - with the exception of the
__DATA,__const section, which, if aligned on a segment alignment
boundary, can be inserted between the two other parts .data is split
into. I read in the XNU source that the ARM protection code does
something roughly like this [1], but I'm really far from well-versed
in the deep details of macOS.
Sorry for this not being "integrated" in above text, but I found two
more things while looking for citation 1.
1) Mach-O sections can be renamed, including the preceding segment
name [2]. According to the very next line, the example actually
creates a new segment. Does it allow merging into another, existing
segment? What if I did something like:
-Wl,-rename_section,__DATA,__const,__TEXT,__const2
or even
-Wl,-rename_section,__DATA,__const,__TEXT,__const
(I cut the upper part of the quote to make some room...)
Both worked as expected.
i.e. can it merge two sections together? if __DATA,__const had data
with relocs, would the renaming trigger the "no relocs" error of
__TEXT, or does that happen before section renaming? Any chance it can
be turned off?
As feared, the error triggers. However I found a seemingly
not-so-well-known flag for Apple ld64 to disable it: "-read_only_relocs
suppress". In fact, it is already used for IA32 builds, but not for X64
builds? [1]
I also noticed the extent of the XCODE5 "exceptions", e.g. duplicated
files [2]. The files seem to be out-of-sync, but interestingly the
XCODE5 version is the one that is more current and used in other
toolchain builds as well.
So, maybe we can do the following?
1) Pass "-read_only_relocs suppress" for X64 and clean up any
XCODE5-specific workarounds for text relocations.
2) Use neither "__TEXT,__const" nor "__DATA,__const" for
NASM_RODATA_SECTION_NAME, but "__DATA_CONST,__const". Give it RNX
permissions.
3) Case-distinct between "generate standalone .rodata" and "merge
.rodata into .text".
3.1) case 1: Rename "__TEXT,__const" and "__TEXT,__cstring" to
"__DATA_CONST,__text_const" and "__DATA_CONST,__cstring" respectively.
3.2) case 2: Rename "__DATA_CONST,__const" to "__TEXT,__data_const".
Rationales:
1) There is no such concept in PE/COFF, we can discard it and align with
PE/COFF and ELF toolchains. Allows ".rodata" merge into ".text".
2) There is no pre-existing segment for read-only data, so we create our
own with a naming convention taken from XNU.
3.1) Both former sections do not contain executable code, so we properly
enforce the separation introduced in 2). Choose unique names to not
reduce the object file separation.
3.2) Merge ".rodata" into ".text" in the same way ELF toolchains do.
Saves space and we get to keep read-only.
I can test both cases work fine soon.
1.1) Actually, for the standalone .rodata section, we can just rename
it to __DATA_CONST,__const, as I have seen elsewhere in XNU. No
hacked-ish solution needed. :)
2) We actually can force the compiler to put data in the constant data
segment [3]. This is of course not used in EDK II, and probably
neither portable nor necessary, but an interesting detail. I really
think we should honour it either way.
I will likely try to get my hands on some sort of Apple development
environment soon, but I cannot promise much right now. I think it
really is better if I can test through all toolchains myself. If you
release Apple Clang for Linux, I also won't complain of course. :)
I found a port of Apple ld64 for Linux. I get it's not "the real deal",
but all observed behaviour matches what I've seen so far from Apple
Xcode. Actually boots OVMF. :)
A side note, seems like even latest mtoc gives .data RWX no matter what?
Ouch...
Best regards,
Marvin
[1]
https://github.com/tianocore/edk2/blob/master/BaseTools/Conf/tools_def.template#L2980-L2982
[2]
https://github.com/tianocore/edk2/blob/0a6b303dcedb7af238ad485d545802befb797b3a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerAsm.nasm
https://github.com/tianocore/edk2/blob/0a6b303dcedb7af238ad485d545802befb797b3a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
We should double check what is happening for ELF on x86, ARM,
RiskV, etc. and do the same thing. I assume all the tools that
generate PE/COFF directly are good with .rodata?
They are not, that is the whole point of the patch in its current
shape. .rodata is valid for ELF and Mach-O, PE/COFF needs .rdata.
I think it is likely as simple as dumping the EFL object file in
objdump or gdb for the given toolchain (like my Xcode example).
TL;DR It looks to me like nasm does some SECTION translations
under the hood to make code portable, and I’d like to make sure
we capture those in the new NASM_RODATA_SECTION_NAME. If some one
is doing a security review having NASM_RODATA_SECTION_NAME is
going to imply that a .rodata section is being used by that
specific toolchain, and I think that is much worse than the
current “magic” behavior in nasm. We are much better off
explaining what is really happening, since it is not very obvious.
I feel like I'm too tired to get the point. Do you mean you want
comments whenever this section name is used? Or comments in
tools_def?
I think I’d settle for a more descriptive commit comment that
better defines what the define means like I mentioned in the other
mail.
Hmm no, we can do that too, but in that case I really want comments
in the code. tools_def is not really documented at all, maybe it is
time to introduce an example comment so at least new things get
commented? Maybe just the start of a macro list. Relying on "git
blame" to figure out simple things is rather awful.
One more thing from another thread: Yes, the new macro should refer
to object file section naming. I want this patch to get object file
sections proper and sound. From there on we can fix the linking
stage to emit proper and sound executables in a later patch.
OK then please refactor the commit message to make it clear that this
patch is to get the correct section in the object files, and work is
still need to get this into executable images.
Sure.
For Xcode you can make it __DATA__,__const since that is the closest
thing to read only data and I think that is your intent.
I would like to do that, but only if we can ensure __DATA,__const is
merged into .text, or is a separate RNX section.
GenFW is part of EDKII BaseTools and mtoc is part of the open source
CCTOOLS project and both those tools would need to be modified to
create a .rodata section in PE/COFF.
Yes, that should not be a big problem. Remaining issues for me:
1) How to merge __DATA,__const into .text, or how to emit a standalone
.rodata section, for Xcode-based toolchains? (Some ideas above, will
ping Vitaly soon as well)
2) How to submit modified mtoc? Any chance it could be maintained in
EDK II like GenFw? (Would be nice if you could provide some insight)
3) How to merge .rdata into .text for MSVC? (I will try to research
this soon-ish, but no promises)
4) How to design a toggle for the platform maintainer to choose
between .text merge and standalone .rodata?
Please note that I'm not asking you to research any of those questions
(but 2) would be nice :) ), this is merely a summary of open points
till the second stage (correct executables, not just correct object
sections) can be properly approached.
Thanks for your time and insight!
Best regards,
Marvin
[1]
https://github.com/apple/darwin-xnu/blob/a1babec6b135d1f35b2590a1990af3c5c5393479/osfmk/arm/arm_vm_init.c#L318-L324
[2]
https://github.com/apple/darwin-xnu/blob/2ff845c2e033bd0ff64b5b6aa6063a1f8f65aa32/makedefs/MakeInc.def#L578
[3]
https://github.com/apple/darwin-xnu/blob/8f02f2a044b9bb1ad951987ef5bab20ec9486310/libsa/lastkerneldataconst.c#L48
Thanks,
Andrew Fish
Best regards,
Marvin
Thanks,
Andrew Fish
Best regards,
Marvin
[1]
https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm#L14
<https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm#L14>
<https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm#L14
<https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/BaseUefiCpuLib/X64/InitializeFpu.nasm#L14>>
[2] $otool -V -s __TEXT
__constBuild/OvmfX64/DEBUG_XCODE5/X64/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib/OUTPUT/X64/InitializeFpu.obj
Build//OvmfX64/DEBUG_XCODE5/X64/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib/OUTPUT/X64/InitializeFpu.obj:
Contents of (__TEXT,__const) section
0000001d 7f 03 80 1f 00 00
$ otool -l
Build//OvmfX64/DEBUG_XCODE5/X64/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib/OUTPUT/X64/InitializeFpu.obj
Build/OvmfX64/DEBUG_XCODE5/X64/UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib/OUTPUT/X64/InitializeFpu.obj:
Load command 0
cmd LC_SEGMENT_64
cmdsize 232
segname
vmaddr 0x0000000000000000
vmsize 0x0000000000000026
fileoff 288
filesize 38
maxprot 0x00000007
initprot 0x00000007
nsects 2
flags 0x0
Section
sectname __text
segname __TEXT
addr 0x0000000000000000
size 0x000000000000001d
offset 288
align 2^0 (1)
reloff 328
nreloc 2
flags 0x80000500
reserved1 0
reserved2 0
Section
sectname __const
segname __TEXT
addr 0x000000000000001d
size 0x0000000000000006
offset 320
align 2^0 (1)
reloff 0
nreloc 0
flags 0x00000000
reserved1 0
reserved2 0
Load command 1
cmd LC_SYMTAB
cmdsize 24
symoff 344
nsyms 3
stroff 392
strsize 63
Thanks,
Andrew Fish
Thanks for your notes and insight!
Best regards,
Marvin
[1]
"For compatibility with other Unix platforms, the following
standard names are also supported:
[...]
.rodata = __DATA,__const data
[...]
If the .rodata section contains no relocations, it is instead
put into the __TEXT,__const section unless this section has
already been specified explicitly."
https://www.nasm.us/xdoc/2.13.01/html/nasmdoc7.html
<https://www.nasm.us/xdoc/2.13.01/html/nasmdoc7.html>
<https://www.nasm.us/xdoc/2.13.01/html/nasmdoc7.html
<https://www.nasm.us/xdoc/2.13.01/html/nasmdoc7.html>>
[1] otool -lh DxeCore.dll
...
Load command 1
cmd LC_SEGMENT_64
cmdsize 312
segname __DATA
vmaddr 0x000000000002b000
vmsize 0x0000000000147000
fileoff 180224
filesize 8192
maxprot 0x00000003
initprot 0x00000003
nsects 3
flags 0x0
Section
sectname __const
segname __DATA
addr 0x000000000002b000
size 0x0000000000000718
offset 180224
align 2^4 (16)
reloff 0
nreloc 0
flags 0x00000000
reserved1 0
reserved2 0
Section
sectname __data
segname __DATA
addr 0x000000000002b720
size 0x00000000000014f0
offset 182048
align 2^4 (16)
reloff 0
nreloc 0
flags 0x00000000
reserved1 0
reserved2 0
Section
sectname __bss
segname __DATA
addr 0x000000000002cc10
size 0x0000000000144e11
offset 0
align 2^4 (16)
reloff 0
nreloc 0
flags 0x00000001
reserved1 0
reserved2 0
…
[2]
https://opensource.apple.com/source/cctools/cctools-698/efitools/mtoc.c.auto.html
<https://opensource.apple.com/source/cctools/cctools-698/efitools/mtoc.c.auto.html>
<https://opensource.apple.com/source/cctools/cctools-698/efitools/mtoc.c.auto.html
<https://opensource.apple.com/source/cctools/cctools-698/efitools/mtoc.c.auto.html>>
[3] otool more output…
Load command 0
cmd LC_SEGMENT_64
cmdsize 392
segname __TEXT
vmaddr 0x0000000000000240
vmsize 0x00000000000296c0
fileoff 1184
filesize 169664
maxprot 0x00000005
initprot 0x00000005
nsects 4
flags 0x0
Section
sectname __text
segname __TEXT
addr 0x0000000000000240
size 0x000000000002489f
offset 1184
align 2^3 (8)
reloff 0
nreloc 0
flags 0x80000400
reserved1 0
reserved2 0
Section
sectname __cstring
segname __TEXT
addr 0x0000000000024ae0
size 0x000000000000496d
offset 150848
align 2^4 (16)
reloff 0
nreloc 0
flags 0x00000002
reserved1 0
reserved2 0
Section
sectname __ustring
segname __TEXT
addr 0x000000000002944e
size 0x0000000000000048
offset 169646
align 2^1 (2)
reloff 0
nreloc 0
flags 0x00000000
reserved1 0
reserved2 0
Section
sectname __const
segname __TEXT
addr 0x00000000000294a0
size 0x0000000000000448
offset 169728
align 2^4 (16)
reloff 0
nreloc 0
flags 0x00000000
reserved1 0
reserved2 0
Thanks,
Andrew Fish
DEBUG_XCODE5_IA32_CC_FLAGS = -arch i386 -c -g -Os
-Wall -Werror -include AutoGen.h -funsigned-char
-fno-stack-protector -fno-builtin -fshort-wchar -fasm-blocks
-mdynamic-no-pic -mno-implicit-float -mms-bitfields
-msoft-float -Wno-unused-parameter -Wno-missing-braces
-Wno-missing-field-initializers -Wno-tautological-compare
-Wno-sign-compare -Wno-varargs
-ftrap-function=undefined_behavior_has_been_optimized_away_by_clang
$(PLATFORM_FLAGS)
@@ -3003,7 +3003,7 @@ RELEASE_XCODE5_X64_DLINK_FLAGS =
-arch x86_64 -u _$(IMAGE_ENTRY_POINT) -e _
DEBUG_XCODE5_X64_ASM_FLAGS = -arch x86_64 -g
NOOPT_XCODE5_X64_ASM_FLAGS = -arch x86_64 -g
RELEASE_XCODE5_X64_ASM_FLAGS = -arch x86_64
- *_XCODE5_X64_NASM_FLAGS = -f macho64
+ *_XCODE5_X64_NASM_FLAGS = -f macho64
-DRODATA_SECTION_NAME=.rodata
*_XCODE5_*_PP_FLAGS = -E -x assembler-with-cpp
-include AutoGen.h
*_XCODE5_*_VFRPP_FLAGS = -x c -E -P -DVFRCOMPILE -include
$(MODULE_NAME)StrDefs.h
--
2.31.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79205): https://edk2.groups.io/g/devel/message/79205
Mute This Topic: https://groups.io/mt/84764902/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
