This is an archive of the discontinued LLVM Phabricator instance.

[LLD] Implement /guard:[no]longjmp
ClosedPublic

Authored by rnk on Feb 12 2018, 5:52 PM.

Details

Summary

This protects calls to longjmp from transferring control to arbitrary
program points. Instead, longjmp calls are limited to the set of
registered setjmp return addresses.

This also implements /guard:nolongjmp to allow users to link in object
files that call setjmp that weren't compiled with /guard:cf. In this
case, the linker will approximate the set of address taken functions,
but it will leave longjmp unprotected.

I used the following program to test, compiling it with different -guard
flags:

$ cl -c t.c -guard:cf
$ lld-link t.obj -guard:cf

#include <setjmp.h>
#include <stdio.h>
jmp_buf buf;
void g() {
  printf("before longjmp\n");
  fflush(stdout);
  longjmp(buf, 1);
}
void f() {
  if (setjmp(buf)) {
    printf("setjmp returned non-zero\n");
    return;
  }
  g();
}
int main() {
  f();
  printf("hello world\n");
}

In particular, the program aborts when the code is compiled *without*
-guard:cf and linked with -guard:cf. That indicates that longjmps are
protected.

Diff Detail

Repository
rL LLVM

Event Timeline

rnk created this revision.Feb 12 2018, 5:52 PM
ruiu added a comment.Feb 13 2018, 10:29 AM

Overall looking pretty good. A few minor comments.

lld/COFF/Config.h
75 ↗(On Diff #133970)

Since we are using enum class above, I'd use enum class too.

lld/COFF/DriverUtils.cpp
135 ↗(On Diff #133970)

I'd write these if-else-ifs in the same order as the enum values. I.e. no, nolongjmp and full.

137–139 ↗(On Diff #133970)

I'd combine the two else ifs like Args.equals_lower() || Args.equals_lower()

lld/COFF/Writer.cpp
925 ↗(On Diff #133970)

This is the same as Config->GuardCF == Full?

rnk marked 4 inline comments as done.Feb 13 2018, 10:43 AM

Thanks, done

rnk updated this revision to Diff 134084.Feb 13 2018, 10:44 AM
  • switch enum style and comparisons
ruiu accepted this revision.Feb 13 2018, 10:49 AM

LGTM

lld/COFF/Config.h
75 ↗(On Diff #134084)

I don't think you need uint8_t anymore because you are not using these enum values as integers.

76–78 ↗(On Diff #134084)

I'd remove = 0, = 1 and = 2 because they are not compared by < nor >.

This revision is now accepted and ready to land.Feb 13 2018, 10:49 AM
rnk updated this revision to Diff 134086.Feb 13 2018, 10:53 AM
  • fixup boolean logic error
This revision was automatically updated to reflect the committed changes.