search

cornelisnetworks / opa-psm2

Other
36 stars 29 forks source link

hfi_dwordcpy: Add CET markup to the x86 assembler implementations #32

Closed fweimer closed 4 years ago

fweimer commented 5 years ago

Note that the _CET_ENDBR instruction is only needed if the function is ever called from a different shared object. I did not see any .hidden or visibility attribute, so I assumed that this might be the case, and added the instruction just in case.

rwmcguir commented 5 years ago

All of PSM2's internal symbols are .hidden with very few exceptions via the control linker script provided in the main directory,

See: https://github.com/intel/opa-psm2/blob/master/psm2_linker_script.map .... / Make all other symbols local / local: *; ....

Makefile inclusion and use of said file:: ... LINKER_SCRIPT_FILE = ${OUTDIR}/psm2_linker_script.map ... $(OUTDIR)/${TARGLIB}.so.${MAJOR}.${MINOR}: ${${TARGLIB}-objs} $(LINKER_SCRIPT_FILE) ... $(CC) $(LINKER_SCRIPT) $(LDFLAGS) -o $@ -Wl,-soname=${TARGLIB}.so.${MAJOR} -shared \ ${${TARGLIB}-objs} $(OUTDIR)/_revision.o -Lopa $(LDLIBS) ..

fweimer commented 5 years ago

Still this requires that there is never an indirect call. Presumably all binutils versions which support GNU Property note merging will always perform relocation relaxation for hidden symbols, and no one will compile the code with the large code model, so the PLT indirection is removed by the linker and there will not be an indirect call in the background. I can update the pull request to drop the _CET_ENDBR if you prefer that.

fweimer commented 5 years ago

@rwmcguir Any further comments? Could we please move this forward? Thanks.

mwheinz commented 4 years ago

Reviewing all pull requests. This looks like one that was never acted on. @fweimer, if you're still available - what tools provide cnet.h? It doesn't seem to be provided by gcc as of 8.2.1.

fweimer commented 4 years ago

@mwheinz GCC with -mcet support provides <cet.h>, which is why there is an #ifdef guard for __CET__.

mwheinz commented 4 years ago

@mwheinz GCC with -mcet support provides <cet.h>, which is why there is an #ifdef guard for __CET__.

Okay - found it. But then doesn't the patch require a change to buildflags.mak?

fweimer commented 4 years ago

But then doesn't the patch require a change to buildflags.mak?

My expectation is that distributions who need CET markup with build with -fcf-protection, so this should come from the environment CFLAGS setting, not from the project build system.

mwheinz commented 4 years ago

But then doesn't the patch require a change to buildflags.mak?

My expectation is that distributions who need CET markup with build with -fcf-protection, so this should come from the environment CFLAGS setting, not from the project build system.

Okay. We're evaluating this now. Thanks for being patient.

mwheinz commented 4 years ago

Will be included in the next PSM2 release.