On Mon, 2020-03-09 at 14:59 -0700, H.J. Lu wrote: > On Mon, Mar 9, 2020 at 2:42 PM Simo Sorce <[email protected]> wrote: > > On Mon, 2020-03-09 at 14:31 -0700, H.J. Lu wrote: > > > On Mon, Mar 9, 2020 at 2:15 PM Simo Sorce <[email protected]> wrote: > > > > On Mon, 2020-03-09 at 12:46 -0700, H.J. Lu wrote: > > > > > On Mon, Mar 9, 2020 at 12:22 PM Simo Sorce <[email protected]> wrote: > > > > > > On Mon, 2020-03-09 at 15:19 -0400, Simo Sorce wrote: > > > > > > > On Mon, 2020-03-09 at 11:56 -0700, H.J. Lu wrote: > > > > > > > > On Mon, Mar 9, 2020 at 11:19 AM Simo Sorce <[email protected]> > > > > > > > > wrote: > > > > > > > > > On Mon, 2020-03-09 at 19:03 +0100, Niels Möller wrote: > > > > > > > > > > Simo Sorce <[email protected]> writes: > > > > > > > > > > > > > > > > > > > > > The patchset i solder than I did remember, April 2019 > > > > > > > > > > > But I recall running at least one version of it on our > > > > > > > > > > > CET emulator @ > > > > > > > > > > > Red Hat. > > > > > > > > > > > > > > > > > > > > Sorry I forgot to followup on that. It seems only the first > > > > > > > > > > easy cleanup > > > > > > > > > > patch, "Add missing EPILOGUEs in assembly files", was > > > > > > > > > > applied back then. > > > > > > > > > > > > > > > > > > > > Do you remember why you used GNU_CET_SECTION() explicitly > > > > > > > > > > in .asm files, > > > > > > > > > > rather than using an m4 divert? > > > > > > > > > > > > > > > > > > Not really I do not recall anymore, but I think there was a > > > > > > > > > reason, as > > > > > > > > > I recall you made that comment back then and it "didn't work > > > > > > > > > out" when > > > > > > > > > I tried is the memory I have of it. > > > > > > > > > Might have to do with differences in how it lays out the code > > > > > > > > > when done > > > > > > > > > via m4 divert, but not 100% sure. > > > > > > > > > > > > > > > > > > > > > > > > > m4 divert requires much less changes. Here is the updated > > > > > > > > patch with > > > > > > > > ASM_X86_ENDBR, ASM_X86_MARK_CET_ALIGN and ASM_X86_MARK_CET. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Two comments on your patch. > > > > > > > > > > > > > > 1. It is an error to align based on architecture. All GNU Notes > > > > > > > MUST be > > > > > > > aligned 8 bytes. Since 2018 GNU Libc ignores misaligned notes. > > > > > > > > > > > > Ah nevermind this point, misunderstanding with my libc expert, the 4 > > > > > > bytes alignment is ok on 32 bit code. > > > > > > > > > > > > > 2. It is better to use .pushsection .popsection pairs around the > > > > > > > note > > > > > > > instead of .section because of the side effects of using .section > > > > > > > > > > Done. > > > > > > > > > > > > The m4 divert looks smaller impact, feel free to lift the Gnu Note > > > > > > > section in my patch #3 and place it into your patch if you want. > > > > > > > My > > > > > > > code also made it more explicit what all the sections values > > > > > > > actually > > > > > > > mean which will help in long term maintenance if someone else > > > > > > > need to > > > > > > > change anything (like for example changing to enable only > > > > > > > ShadowStack > > > > > > > vs IBT). > > > > > > > > > > > > > > > > > Since CET support requires all objects are marked for CET, CET > > > > > marker on > > > > > assembly sources is controlled by compiler options, not by configure > > > > > option. > > > > > Also linker can merge multiple .note.gnu.property sections in a single > > > > > input file: > > > > > > > > > > [hjl@gnu-cfl-1 tmp]$ cat p.s > > > > > .pushsection ".note.gnu.property", "a" > > > > > .p2align 3 > > > > > .long 1f - 0f > > > > > .long 4f - 1f > > > > > .long 5 > > > > > 0: > > > > > .asciz "GNU" > > > > > 1: > > > > > .p2align 3 > > > > > .long 0xc0000002 > > > > > .long 3f - 2f > > > > > 2: > > > > > .long 1 > > > > > 3: > > > > > .p2align 3 > > > > > 4: > > > > > .popsection > > > > > .pushsection ".note.gnu.property", "a" > > > > > .p2align 3 > > > > > .long 1f - 0f > > > > > .long 4f - 1f > > > > > .long 5 > > > > > 0: > > > > > .asciz "GNU" > > > > > 1: > > > > > .p2align 3 > > > > > .long 0xc0000002 > > > > > .long 3f - 2f > > > > > 2: > > > > > .long 2 > > > > > 3: > > > > > .p2align 3 > > > > > 4: > > > > > .popsection > > > > > [hjl@gnu-cfl-1 tmp]$ as -o p.o p.s -mx86-used-note=no > > > > > [hjl@gnu-cfl-1 tmp]$ readelf -n p.o > > > > > > > > > > Displaying notes found in: .note.gnu.property > > > > > Owner Data size Description > > > > > GNU 0x00000010 NT_GNU_PROPERTY_TYPE_0 > > > > > Properties: x86 feature: IBT > > > > > GNU 0x00000010 NT_GNU_PROPERTY_TYPE_0 > > > > > Properties: x86 feature: SHSTK > > > > > [hjl@gnu-cfl-1 tmp]$ ld -r p.o > > > > > [hjl@gnu-cfl-1 tmp]$ readelf -n a.out > > > > > > > > > > Displaying notes found in: .note.gnu.property > > > > > Owner Data size Description > > > > > GNU 0x00000010 NT_GNU_PROPERTY_TYPE_0 > > > > > Properties: x86 feature: IBT, SHSTK > > > > > [hjl@gnu-cfl-1 tmp]$ > > > > > > > > > > New properties can be added without changing CET marker. > > > > > > > > > > Here is the updated patch. > > > > > > > > This patch looks good to me. > > > > Unfortunately I never received the original email creating the thred, > > > > did you send other patches too ? > > > > > > This is the only patch needed to enable CET. > > > > > > > Or is the prologue stuff sufficient to pass test suite in CET emulator? > > > > > > > > > > It is sufficient to pass all tests on real CET processors with > > > > > > $ CC="gcc -Wl,-z,cet-report=error -fcf-protection" CXX="g++ > > > -Wl,-z,cet-report=error -fcf-protection" > > > /home/hjl/work/git/gitlab/nettle/configure > > > > Excellent! > > > > I wonder if we should create a negative test with an actual incorrect > > jmp instruction to verify that CET is enabled, given it is very easy to > > get the linker to disable CET if any dependency is not CET enabled. > > > > When I ran the code in the emulator I manually added a jump to a non > > endbr instruction to make sure it would fail, and it did :-) > > > > I have some CET smoke tests in violations directory at > > https://gitlab.com/cet-software/cet-smoke-test
I meant as part of Nettle, so hopefully we do not regress (once CET enabled machines become available in CI systems). We can have a simple binary that links to libnettle and libhogweed and has inline assembly that jumps to a non ENDBR instruction. The test will SUCCEED if the binary crashes... The test should run only if the HW supports CET of course, I assume this is something that can be tested via /proc/cpuinfo or similar methods ? Simo. -- Simo Sorce RHEL Crypto Team Red Hat, Inc _______________________________________________ nettle-bugs mailing list [email protected] http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
