On Mon, 2020-03-09 at 12:46 -0700, H.J. Lu wrote:
> On Mon, Mar 9, 2020 at 12:22 PM Simo Sorce <simo(a)redhat.com> 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 <simo(a)redhat.com> wrote:
> > > > > On Mon, 2020-03-09 at 19:03 +0100, Niels Möller wrote:
> > > > > > Simo Sorce <simo(a)redhat.com> 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 ?
Or is the prologue stuff sufficient to pass test suite in CET emulator?
--
Simo Sorce
RHEL Crypto Team
Red Hat, Inc