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

Implement restrictions on taking addresses of members and variables. #490

Merged
merged 9 commits into from May 21, 2018

Conversation

dtarditi
Copy link
Contributor

@dtarditi dtarditi commented May 19, 2018

Checked C restricts taking the addresses of:

  1. Members with member bounds declarations.
  2. Members used in member bounds declarations.
  3. Variables with bounds declarations.
  4. Variables/variable members used in bounds declarations.

This change implements restrictions 1 through 3. This addresses issue #213 and part of issue #212.

  • Taking the address of non-array members with or used in bounds declarations is now an error.
  • Taking the address of non-array members with or used in bounds-safe interfaces is allowed in unchecked scopes. It is an error in checked scopes. In unchecked scopes, we don't warn about this, even though it could go wrong. We don't want to add warnings for existing code. We could add an optional warning if we wanted to.
  • Taking the address of non-array variables with bounds declaration is now an error.

It is OK to take the address of an array variable or member because you can't use the resulting pointer to modify the pointer that the array converts to.

We recognize variations on these restrictions, such as taking the address of a parenthesized lvalue expression or taking the address of an expression where a bounds-safe interface cast has been inserted.

This was more complex to implement than the specification describes because of the possible use of nested members. See the long comment describing the algorithm in CheckedCAlias.cpp. We handle a few different cases:

  • The simple case where a member bounds expression only uses members declared in the current structure.
  • A member bounds expression uses members from a nested structure of the current structure. In this case, we do not allow the address of the nested structure to be taken. Given
struct NestedLen {
   int len; 
};

struct S {
   struct NestedLen n;
   _Array_ptr<int> p : count(n.len);
}

we don't allow the address of n to be taken.

  • A structure with bounds is embedded in another structure, and then an address of member used in member bounds is taken.

We handle these cases by recognizing "paths" of member accesses and using that to recognize paths that may result in aliases to members used in bounds, and paths that won't result in aliases to members used in bounds.

All of the benchmarks that we converted to Checked C compile with these restrictions in place. This is a step toward removing one of the caveats in the SecDev paper. We still need to restrict taking the address of variables and variable members used in bounds declarations to complete the aliasing restrictions.

It'll be interesting to see what happens on real-world code. We could loosen some of the restrictions, for example, and allow taking the addresses involving constant-sized bounds (bounds that only involve the variable or member).

I found a fixed bug in the lexicographic comparison of declarations. We were supposed to be comparing pointers to names in declarations, and compared pointers to the declarations instead. We then tried dereferencing the pointers to names.

Testing:

  • I've added a new file containing tests of address-of expressions in the Checked C repo. This will be subject to a separate pull request.
  • Passed clang tests on Linux x64.
  • Passed LNT tests on Linux x64.

Handle the case that a member is directly used in a member bounds.
We'll handle the case where a member of a member is used later.

There are 3 situations that we handle:
1. Address-taken members used in a bounds declarations.
2. Address-taken members used in a bounds-safe interface.
- To avoid producing errors for existing code, it is not an error in
  an unchecked scope.
- It is an error if this is done in a checked scope.
- When computing paths, skip lvalue bit casts introduced for bunds-safe
  interfaces.
- Don't give an error for taking the address of an array with bounds.
Copy link
Collaborator

@awruef awruef left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants