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

add GCC option for checking virtual table pointers #485

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Flo4152
Copy link

@Flo4152 Flo4152 commented May 8, 2024

add GCC option for checking virtual table pointers

add GCC option for checking virtual table pointers

Signed-off-by: Flo4152 <151801067+Flo4152@users.noreply.github.com>
@Flo4152 Flo4152 changed the title Update Compiler-Options-Hardening-Guide-for-C-and-C++.md add GCC option for checking virtual table pointers May 8, 2024
@Flo4152
Copy link
Author

Flo4152 commented May 8, 2024

New pull request to add a section about virtual table checking section after synchronizing my repository with the base branch.

@@ -211,6 +211,7 @@ Table 2: Recommended compiler options that enable run-time protection mechanisms
| [`-fno-strict-aliasing`](#-fno-strict-aliasing) | GCC 2.95.3<br/>Clang 18.0.0 | Do not assume strict aliasing |
| [`-ftrivial-auto-var-init`](#-ftrivial-auto-var-init) | GCC 12<br/>Clang 8.0 | Perform trivial auto variable initialization |
| [`-fexceptions`](#-fexceptions) | GCC 2.95.3<br/>Clang 2.6 | Enable exception propagation to harden multi-threaded C code |
| [`-fvtable-verify=std`](#-fvtable-verify) | GCC 4.9.4 | (C++) Enable run-time checks for virtual function pointers corruption. Can impact performance.|
Copy link
Contributor

@thesamesam thesamesam May 8, 2024

Choose a reason for hiding this comment

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

IIRC, this option breaks ABI (at least if libstdc++ is built with it, which is needed for it to be more useful).

I don't know if anyone has ever e.g. tried to build a distro with it, either. I wouldn't say this is in a state to be recommended without further investigation, at the least.

Also, I think GCC supports -fsanitize=vptr nowadays.

Copy link
Author

Choose a reason for hiding this comment

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

This risk of ABI breakage is described in the Gentoo manual (USE flag - https://packages.gentoo.org/useflags/vtv). This warning seems to be related to maintaining full package compatibility and stability requirements throughout the lifetime of a system (library dependencies and link editing issues). This information provides a realistic argument for answering @david-a-wheeler's question about the use of -fvtable-verify in a real environment (#341 (comment)).

@thesamesam is right, the use of -fvtable-verify may not be recommended in many real-life situations. Adding this flag to the Guide may require, at a minimum, additional research and I can't provide any arguments to justify adding this flag to the Guide. I can only provide a few examples of C++ compiled applications and benchmarks (7zip - https://www.7-zip.org, GROMACS - https://www.gromacs.org/, ...) but nothing relevant enough to fully argue the proposal of this option.

In addition, the vptr sanitazer seems to be a more recent alternative, but I'm not familiar with it.

Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm really bored, I may consider trying it on a system-wide build of Gentoo for fun, but I don't plan on trying it. Someone else is welcome to though. I'd be interested in how it goes ;)

@Flo4152
Copy link
Author

Flo4152 commented May 8, 2024

@thomasnyman @david-a-wheeler This pull request is a duplication of #440. I close my first pull request by mistake during the synchronization of my repository.

Sorry about this mistake.

Regards,

Flo4152 and others added 5 commits May 8, 2024 21:20
reword "Can impact performance" to "Significantly impacts performance"

Signed-off-by: Flo4152 <151801067+Flo4152@users.noreply.github.com>
Use consistent wording in all -fvtable-verify option table entries

Signed-off-by: Thomas Nyman <thomas.nyman@ericsson.com>
- Adjust labels to ^Tice2012 and ^Tice2014 to be consistent with citation
  style elsewhere in the guide
- Change URL for Usenix Security '14 material to point to article as
  suggested by the text
- Adjust references to [^Tice2012] and [^Tice2014] to avoid linter
  errors

Signed-off-by: Thomas Nyman <thomas.nyman@ericsson.com>
Signed-off-by: Thomas Nyman <thomas.nyman@ericsson.com>
Signed-off-by: Thomas Nyman <thomas.nyman@ericsson.com>
@thomasnyman
Copy link
Contributor

@Flo4152 I've opened a pull request against your branch to address the linter errors as well as some consistency issues with the rest of the guide: Flo4152#1

Given the observation by @thesamesam in his #485 (comment) regarding ABI breakage I think that is something that really warrants as When to not use? section I was asking for in the original PR

I am also wondering whether -fsanitize=vptr combined with the -fsanitize-minimal-runtime we were pointed to look at in #326 would be a better options for production code.

That said, I do think the material on the usage of -fvtable-verify in this PR is good and given that there are examples of users of this, even though it may not be fit for a general recommendation, I do think this would be good material for the guide as long as the caveats with the option are clearly communicated.

Add information on the risk of ABI rupture when using the -fvable-verify flag

Signed-off-by: Flo4152 <151801067+Flo4152@users.noreply.github.com>
@thesamesam
Copy link
Contributor

thesamesam commented May 18, 2024

My reservation is that I've genuinely never heard of anyone using this outside of those initial talks when it got merged into GCC. I'm sure users have existed, or do exist somewhere, but I've never heard from them and I don't ever see it mentioned in GCC bug reports or development discussions.

I'd feel a lot better about advertising this if:

  • someone could ask around the GCC community (perhaps on the ML) what the consensus on the option is, and if it's suitable for the guide;
  • someone could try build a bunch of software with it and see what happens (and make sure e.g. the compiler doesn't just ICE immediately)

I also don't think Google is using this anymore, given they're very much using the LLVM toolchain these days, but I could be wrong.

@@ -68,7 +68,7 @@ Most programming languages prevent such defects by default. A few languages allo

Run-time attacks differ from conventional malware, which carries out its malicious program actions through a dedicated program executable, in that run-time attacks influence benign programs to behave maliciously. A run-time attack that exploits unmitigated memory vulnerabilities can be leveraged by threat actors as the initial attack vectors that allow them to gain a presence on a system, e.g., by injecting malicious code into running programs.

Modern, security-aware C and C++ software development practices, e.g., secure coding standards such as SEI CERT C[^CMU2016C] and C++[^CMU2016CPP], and program analysis aim to proactively avoid introducing memory errors (and other software defects) to applications. However, in practice completely eradicating memory errors in production C and C++ software has turned out to be near-impossible.
Modern, security-aware C and C++ software development pracs, e.g., secure coding standards such as SEI CERT C[^CMU2016C] and C++[^CMU2016CPP], and program analysis aim to proactively avoid introducing memory errors (and other software defects) to applications. However, in prac completely eradicating memory errors in production C and C++ software has turned out to be near-impossible.
Copy link
Contributor

Choose a reason for hiding this comment

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

The practicespracs changes seems accidental. Please revert this change.


### Performance implications

Caroline Tice played an important role in integrating the vtable verification (VTV) by submitting the commit of this feature in GCC 4.9[^gccvtvcommit]. She involved on articles that give information about performance penalty. She is describe performance penalty following 3 points:
Copy link
Contributor

Choose a reason for hiding this comment

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

There was feedback in the C/C++ Compiler BP Guide Call in 2024-05-16 that the "performance implications section reads very differently than text elsewhere in the guide, should be reworded to be more consistent with other sections"

This is perhaps partially because, unlikely the rest of the guide, which aims to acknowledge the original author's through appropriate references. This section seems to feature the author rather prominently. Maybe this paragraph could be shortened while still making sure the author's original work is included as a reference.


- Call verification: Performance penalty between 5% and 10%.
- Object permission changes: Between 400% and 700% slowdown for permission changes per object file, while a performance loss of 320 ms is noticeable for permission changes per binary.
- Virtual function hashtable size: Storage of virtual function hashtable imply a big waste of space.
Copy link
Contributor

Choose a reason for hiding this comment

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

Another piece of feedback from the C/C++ Compiler BP Guide Call in 2024-05-16 was that "performance numbers should be attributed to specific benchmarks, 5% impact in Chrome may be acceptable, but 5% impact on SPEC benchmarks would generally be undesirable"

Copy link
Author

@Flo4152 Flo4152 Jun 3, 2024

Choose a reason for hiding this comment

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

@thomasnyman @siddhesh The performance impact of the -fvtable-verify option is discussed in several articles.

In "Enforcing Forward-Edge Control-Flow Integrity in GCC & LLVM", a section is devoted to the performance impact of virtual table verification (7.1 VTV Performance - https://www.usenix.org/system/files/conference/usenixsecurity14/sec14-paper-tice.pdf#page=12). This article reports a performance impact on the "SPEC CPU2006 C++ benchmarks" between 2.4% and 19.6% on omnetpp, astar and xalancbmk. Four other benchmarks (povray, namd, soplex and dealII) showed no significant effect on performance. This article also focuses on reducing the performance penalty of the three affected benchmarks. They found that the performance penalty of xalancbmk fell from 19.2% to 10.8% using devirtualization and to 8.7% using static linking and devirtualization. They also noted that PGO optimization (flag -ripa) can reduce the number of virtual calls to be checked.

This information is presented in graphs in the "Improving Function Pointer Security for Virtual Method Dispatches" slides on the GCC website (https://gcc.gnu.org/wiki/cauldron2012?action=AttachFile&do=get&target=cmtice.pdf) and in the slides relating to the article "Enforcing Forward-Edge Control-Flow Integrity in GCC & LLVM" (https://www.usenix.org/sites/default/files/conference/protected-files/sec14_slides_tice.pdf).

These numbers can add consistence to the section about performance impact. I also think that optimization that improving performance of code compiled with vtv also has a place in the section about performance impact (PGO, static linking and devirtualization) .

I will integrate these numbers and optimizations into my pull request but I have to be concise.

Regards,


#### When not to use?

Using the `-fvtable-verify` flag can lead to ABI breaks.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a note here about the implications: "Software that needs to interoperate with libraries built without -fvtable-verify should not enable this option.

Flo4152 and others added 3 commits June 10, 2024 23:33
Signed-off-by: Flo4152 <151801067+Flo4152@users.noreply.github.com>
- Add benchmark information to the "Performance implications" sub-section of the -fvtable-verify section.
- Add information about interoperate with libraries built without -fvtable-verify in subsection "When not to use?" of the -fvtable-verify section.

Signed-off-by: Flo4152 <florian.berbar@gmail.com>
@david-a-wheeler
Copy link
Contributor

These are really significant performance impacts, on a relatively narrowly focused countermeasure.

We definitely should document these options, including the performance impacts noted above. I don't think we should recommend using these options by default.

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

Successfully merging this pull request may close these issues.

None yet

4 participants