Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsoundness for LLVM functions that return non-fresh pointers #641

Closed
brianhuffman opened this issue Jan 28, 2020 · 3 comments · Fixed by #826
Closed

Unsoundness for LLVM functions that return non-fresh pointers #641

brianhuffman opened this issue Jan 28, 2020 · 3 comments · Fixed by #826
Labels
subsystem: crucible-llvm Issues related to LLVM bitcode verification with crucible-llvm type: bug Issues reporting bugs or unexpected/unwanted behavior unsoundness Issues that can lead to unsoundness or false verification
Milestone

Comments

@brianhuffman
Copy link
Contributor

brianhuffman commented Jan 28, 2020

This is a similar bug to #640, but without using NULL.

Here's the C code:

#include <stdint.h>
#include <stdlib.h>

uint64_t *foo (uint64_t *x) {
	return x;
}

int bar (uint64_t *x) {
	return (x == foo(x));
}

And here's the saw-script, which can prove both that bar always returns 0 and that bar always returns 1:

bc <- llvm_load_module "return_same.bc";

let i64 = llvm_int 64;

foo_ov <-
  crucible_llvm_verify bc "foo" [] false
    do {
      x <- crucible_alloc i64;
      crucible_execute_func [x];
      y <- crucible_alloc i64;
      crucible_return y;
    }
    z3;

bar_ov0 <-
  crucible_llvm_verify bc "bar" [foo_ov] false
    do {
      x <- crucible_alloc i64;
      crucible_execute_func [x];
      crucible_return (crucible_term {{ 0 : [32] }});
    }
    z3;

bar_ov1 <-
  crucible_llvm_verify bc "bar" [] false
    do {
      x <- crucible_alloc i64;
      crucible_execute_func [x];
      crucible_return (crucible_term {{ 1 : [32] }});
    }
    z3;

The problem is that the verification of foo should fail, as the spec says that the return value of foo should be a freshly-created pointer, while it has actually copied the pointer from an input argument. To be sound, we need to check that supposedly-fresh pointers are actually disjoint from all other pointers in scope.

@brianhuffman brianhuffman added type: bug Issues reporting bugs or unexpected/unwanted behavior subsystem: crucible-llvm Issues related to LLVM bitcode verification with crucible-llvm unsoundness Issues that can lead to unsoundness or false verification labels Jan 28, 2020
@brianhuffman
Copy link
Contributor Author

Here's another variation, using addresses of globals instead of pointers from input arguments.

Here's the C code:

#include <stdint.h>
#include <stdlib.h>

uint64_t glob;

uint64_t *foo () {
	return &glob;
}

int bar () {
	return (&glob == foo());
}

and the saw-script, which again proves that bar returns 0 and returns 1:

bc <- llvm_load_module "return_global.bc";

let i64 = llvm_int 64;

foo_ov <-
  crucible_llvm_verify bc "foo" [] false
    do {
      crucible_alloc_global "glob";
      crucible_execute_func [];
      x <- crucible_alloc i64;
      crucible_return x;
    }
    z3;

bar_ov0 <-
  crucible_llvm_verify bc "bar" [foo_ov] false
    do {
      crucible_alloc_global "glob";
      crucible_execute_func [];
      crucible_return (crucible_term {{ 0 : [32] }});
    }
    z3;

bar_ov1 <-
  crucible_llvm_verify bc "bar" [] false
    do {
      crucible_alloc_global "glob";
      crucible_execute_func [];
      crucible_return (crucible_term {{ 1 : [32] }});
    }
    z3;

@brianhuffman
Copy link
Contributor Author

The second example here, involving globals, was fixed by PR #752 (4912fdd), which also fixed issue #642. We should still add a test for it, though.

The unsound example in the original post is still accepted by saw, however.

@brianhuffman
Copy link
Contributor Author

The situation is a bit different if the global is declared const. In this case, saw will happily verify a spec saying that the return pointer is a fresh immutable pointer instead of a reference to the global:

const uint64_t glob;

const uint64_t *foo () {
	return &glob;
}
foo_ov <-
  crucible_llvm_verify bc "foo" [] false
    do {
      crucible_execute_func [];
      x <- crucible_alloc_readonly i64;
      crucible_return x;
    }
    z3;

But in this case, verifying such a spec is not actually a problem, as our LLVM memory model explicitly allows immutable regions to alias each other and forbids comparing const pointers. So you can't actually do anything unsound with an override like this one.

brianhuffman pushed a commit that referenced this issue Aug 27, 2020
This test succeeds, but it should actually fail. When issue
#641 is fixed, the test will need to be modified to indicate
an expected failure.
brianhuffman pushed a commit that referenced this issue Aug 28, 2020
This test succeeds, but it should actually fail. When issue
#641 is fixed, the test will need to be modified to indicate
an expected failure.
brianhuffman pushed a commit that referenced this issue Aug 31, 2020
Fix soundness bug related to disjointness checking (#641)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
subsystem: crucible-llvm Issues related to LLVM bitcode verification with crucible-llvm type: bug Issues reporting bugs or unexpected/unwanted behavior unsoundness Issues that can lead to unsoundness or false verification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants