On Fri, 2019-04-26 at 22:31 +0200, Niels Möller wrote:
Simo Sorce simo@redhat.com writes:
I understand this is not a high bar, and I will fold the segment note in if you think that is what we should do, but I wanted to make you aware of why I did not do the same as what we do with the stack note.
I think we should aim to make all files "cet-compliant" if we do it att all. After all, the common case is to have a libnettle.so, and then any single object file missing the annotation will make the linker disable the feature, if I've understood it correctly. I agree it should be disabled by default until we're more confident in test coverage.
Yes we should have all files CET compliant, the idea of having a distinct macro was to make it easy to catch submissions of new files that lack it.
BTW, do you know how that works with late loading using dlopen? One could have a process running in CET-mode, which dynamically loads an so-file with code lacking endbr instructions and corresponding annotation.
Yes I had the same question so I asked to our glibc gurus. The only protection that is problematic is IBT as it requires insertion of new instructions, so there is a mode where you can mark specific pages so that the CPU will ignore missing endbr instruction exclusively for executable code on those pages. So you can load an entire library via dlopen() on specially marked pages and code running on those will have no protection while the rest of the code will do. It is an all-or nothing at the library level at that point. It is not clear to me if this is fully supported today in glibc yet.
I guess in theory this could be done for individual libraries brought in at execution time by the dynamic linker, but I think that is not done and instead the whole binary is either enabled or disabled currently.
If we think about it as an arch-specific thing, which I guess we should, then maybe the m4 divert should be in x86_64/machine.m4 and x86/machine.m4, not asm.m4.
Makes sense, I can move it, do you still want it to be automagically added to all assembly files?
That is funny, I actually used CET_ENDBR to make it easier to find for others grepping as binutils also uses a _CET_ENDBR macro, it sounded more consistent
I agree your name is better for readability, even if less amusing.
I'd like to understand what's missing. Maybe we can fix it more explicitly?
I do not think we can easily fix it manually, it is mostly other section notes that the gcc compiler adds when it fortifies C code. But those notes do not really apply to handcrafted assembly.
[...]
So this flag is basically just saying to the compiler that it should add whatever is appropriate (which may change depending on compiler flags) because our code is good as is.
Since the command line flag is passed with -Wa, it tells the *assembler* to add notes.
True.
Which ones? Is it based only on the command line, say, $(CFLAGS) contains -fharden-foo makes the assembler produces a "foo" note. Or is it based on what's actually in the assembly input file?
It is not based on what is in the assembly input file afaik. It generates "GNU Build Attribute" notes.
readelf show a single additional Owner attribute named GA$<version>3a1 with Data size always set to 0x10 and applied to each section of the library corresponding to assembly files that cover the memory range occupied by said library.
I have no more information than that at the moment, but I can ask if you want me to dig through it.
Is there a risk that it automatically generates a note promising something about the assembly code which we don't actually fulfill?
I was told that's not the case, I think it just sets bare notes that basically do not assert anything specific, it just makes the tools that check for those notes happy.
Is there any documentation? I see that it is mentioned in the binutils-2.31 release announcement, but I've not found it mentioned in the Gas manual.
Couldn't find anything either. Only some fedora wiki change proposal page that mentions them.
https://software.intel.com/sites/default/files/managed/4d/2a/control-flow-en... (I do no have a better link)
Looks like reasonable doc. (Closest on wikpedia seems to be https://en.wikipedia.org/wiki/Control-flow_integrity).
Yeah I found this page too on wikipedia but is is it too generic (it also referenced the link above)
See above explanation and let me know if that changes your opinion, otherwise I will do this.
+ALIGN(8) +.long 1f - 0f +.long 4f - 1f +.long 5 +0: +.string "GNU" +1: +ALIGN(8) +.long 0xc0000002 +.long 3f - 2f +2: +.long 0x03 +3: +ALIGN(8) +4:
Are there no symbolic names for this note? Since we require assembler to suport endbr instructions, can we require that it know about these notes as well? What does it look like in gcc output?
There are symbolic names to compose the 0x03 value and for the 0xc0000002 values, the rest are just label arithmetic.
I will change in next submission.
I see, I was hoping for something more similar to
.section .note.GNU-stack,"",TYPE_PROGBITS
Nope, no such thing.
I'm still curious as to what it looks like in gcc output.
Exactly like the above.
You can see it appended in any .s file generated by gcc when you compile with CFLAGS including -fcf-protection
Attached find a new patch that replaces the first one and moves the section definition to be machine specific
The second is a WIP, to also move the new instructions and make the section atumagic. It makes the configure.ac clearer but requires to add macros to each machine.m4 file. Given it is a amchine specific technology I did not think it made sense to add CET_ENDBR name in non intel architectures, and allows us to use CODEFROM :-)
The other patches are unchanged, but I changed the order to put them first so they can be applied right away if you want.