11 ms·

Here's a few more we use: -Wvla Warn about any use of stack-allocated variable length arrays. There are a couple of reasons why these are bad: without care

by rwmj 4y ago

Here's a few more we use: -Wvla Warn about any use of stack-allocated variable length arrays. There are a couple of reasons why these are bad: without care they can cause exploits where a caller specifies a very large array which crashes the program. And they are a problem where stack space is limited (kernel code, threads on 32 bit machines). -Wframe-larger-than=5000 -Wstack-usage=10000 Prevent large stack frames. Same reasons as above. -Wshadow Prevent shadowed variables, eg. a local variable with the same name as a global. -Werror Turn warnings into errors. Only use this for developer builds though, because newer compilers introduce new warnings and that will cause problems for downstream packagers. -fno-omit-frame-pointer https://rwmj.wordpress.com/2023/02/14/frame-pointers-vs-dwarf-my-verdict/ https://rwmj.wordpress.com/2023/02/14/frame-pointers-vs-dwar... -fvisibility=hidden When building a library, make sure all symbols between compilation units but within the library are not exported. Basically reduces the load-time overhead and prevents random ELF symbol interposition. If you use this, then to export a symbol you have to use '__attribute__((__visibility__("default")))'. -fno-strict-overflow -Wno-strict-overflow (Controversially) unbreaks the C compiler by allowing signed overflows to do the obvious thing.

DyslexicAtheist 4y ago
> -fno-omit-frame-pointer agreed on 64bit. Also this advise is old it hasn't popped up out of nowhere. I still recall it being crucial back in the early 2000's to improve performance. And today it remains very valid because of 32 bit IoT systems.
- rwmj 4y ago
  Right, on 32 bit you do want to omit frame pointers so %ebp can be used as a general register. (Fedora is on the verge of dropping 32 bit platforms entirely.)
pajko 4y ago
Frame pointers are old-school technology for most of the platforms, even in embedded usage. The libunwind library should be used instead, with the -funwind-tables or -fasynchronous-unwind-tables compiler options and additionally -rdynamic, -fno-dwarf2-cfi-asm, -fvar-tracking-assignments.
- rwmj 4y ago
  That's a nice theory but it doesn't actually work in practice. Read the link I posted above.
- mgaunard 4y ago
  -rdynamic isn't going to help libunwind, it's for glibc's built-in backtrace library which you simply shouldn't use. var-tracking-assignments in on by default when you use any variant of -g (which you should). You can also configure the targeted dwarf version there (which by default is 5 these days) and the level of debugging info, which you should set to the max. This makes your dwarf2-related options redundant. -funwind-tables is also there by default in C++, not sure for C.
  - wahern 4y ago
    > -rdynamic isn't going to help libunwind, it's for glibc's built-in backtrace library which you simply shouldn't use. -rdynamic exports global symbols from the main binary so that they're available to satisfy dependencies of dynamically linked libraries and modules. For example, a Perl or Lua interpreter, or an Apache daemon, will use -rdynamic so that its symbols can satisfy the dependencies of binary modules, rather than requiring the engine implementation to be linked via a shared library itself, which is more brittle. It's sort of midway between static and dynamic linking in terms of ensuring everybody is using the same implementation of a shared framework.
- nemetroid 4y ago
  Frame pointers are making a resurgence for use cases where you want to unwind the stack really quickly, and Dwarf based options are too slow.
  - PhilipRoman 4y ago
    Maybe I'm missing something but isn't it redundant to have both stack and frame pointers? Just use negative offsets from the frame pointer to address local variables.
mgaunard 4y ago
I never get this whole thing of compiling with -fno-omit-frame-pointer, it's already the default unless you enable optimizations.
- bigdict 4y ago
  Often you want to profile an optimized binary.
uecker 4y ago
In my opinion the improved code quality and bounds checking you can get with VLAs are worth it. Yes, the an attacker should not control the size and one should use -fstack-clash-protection and when the stack size is limited you should think twice (but vla can also reduce stack size if the alternative is a fixed size array on stack with larger fixed upper bound).
- fanf2 4y ago
  VLAs do not have extra bounds checking. Looking at it from an attacker’s point of view, a VLA is a primitive for adding an arbitrary offset to the stack pointer: extremely powerful and dangerous. You can’t use VLAs for large allocations because the stack has a limited amount of space and you don’t know what that limit is. So you must check the size is reasonable before declaring a VLA. So you might as well declare a fixed-size array that is large enough for all reasonable uses.
  - uecker 4y ago
    VLAs can enable precise bounds checking with the undefined behavior sanitizer. This is useful and you do not have precise bounds checking anymore when you replace it with a larger fixed size array. Also you increase stack size when you replace with a larger fixed size array. Yes, one should not let an attacker control the size of the VLA. But in any case, one should use -fstack-clash-protection . With stack clash protection if an attacker can control the size of VLA and it becomes to large, you can a trap with is likely DoS. With a fixed size array which you overflow instead, it is more likely a RCE. If you check the size, it does not matter. That you can't use the stack for large allocations has nothing to with VLAs and also depends on what you call large and how large your stack is.
    - pjmlp 4y ago
      They were made optional in C11, while Google paid for the development effort to clean the Linux kernel of them, because they are only yet another source of possible CVEs.
Conscat 4y ago
Changing signed integer overflow is extremely stupid considering unsigned integer overflow is what does the wrong thing in C. Just use ubsan.
- Jorengarenar 4y ago
  > unsigned integer overflow is what does the wrong thing in C You may want to rethink this statement
  - Conscat 4y ago
    No, I am correct. Unsigned integer overflow has wrapping overflow semantics. Absolutely idiotic.
    - Jorengarenar 4y ago
      So... unsigned overflow is defined, while signed is straight up UB... and for you it's the former that's wrong?
      - account42 4y ago
        Yes. Most calculations won't do the right thing with wrapping overflow and you'd be better of by them being undefined and tested via the relevant tooling (e.g. ubsan). Those that do should be written in a way to make that explicit.
      - Conscat 4y ago
        Compilers can make commutative optimizations with signed integer overflow that is impossible with wrapping semantics. This is why Zig and Carbon both make unsigned integer overflow undefined as well. There is absolutely no advantage in most cases to wrapping semantics unless it is actually required for the algorithm (like some hashing or PRNG functions, for example).
Jorengarenar 4y ago
> Warn about any use of stack-allocated variable length arrays. Unfortunately, it warns about all VLA, not only stack allocated ones. What you might want is rather: -Wvla-larger-than=0 This flag will indeed report only automatic VLA (but for some reason only if optimizations are turned on [1]). [1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109395 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109395

by rwmj 4y ago

DyslexicAtheist 4y ago

> -fno-omit-frame-pointer agreed on 64bit. Also this advise is old it hasn't popped up out of nowhere. I still recall it being crucial back in the early 2000's to improve performance. And today it remains very valid because of 32 bit IoT systems.

rwmj 4y ago
Right, on 32 bit you do want to omit frame pointers so %ebp can be used as a general register. (Fedora is on the verge of dropping 32 bit platforms entirely.)

pajko 4y ago

Frame pointers are old-school technology for most of the platforms, even in embedded usage. The libunwind library should be used instead, with the -funwind-tables or -fasynchronous-unwind-tables compiler options and additionally -rdynamic, -fno-dwarf2-cfi-asm, -fvar-tracking-assignments.

rwmj 4y ago
That's a nice theory but it doesn't actually work in practice. Read the link I posted above.
mgaunard 4y ago
-rdynamic isn't going to help libunwind, it's for glibc's built-in backtrace library which you simply shouldn't use. var-tracking-assignments in on by default when you use any variant of -g (which you should). You can also configure the targeted dwarf version there (which by default is 5 these days) and the level of debugging info, which you should set to the max. This makes your dwarf2-related options redundant. -funwind-tables is also there by default in C++, not sure for C.
- wahern 4y ago
  > -rdynamic isn't going to help libunwind, it's for glibc's built-in backtrace library which you simply shouldn't use. -rdynamic exports global symbols from the main binary so that they're available to satisfy dependencies of dynamically linked libraries and modules. For example, a Perl or Lua interpreter, or an Apache daemon, will use -rdynamic so that its symbols can satisfy the dependencies of binary modules, rather than requiring the engine implementation to be linked via a shared library itself, which is more brittle. It's sort of midway between static and dynamic linking in terms of ensuring everybody is using the same implementation of a shared framework.
nemetroid 4y ago
Frame pointers are making a resurgence for use cases where you want to unwind the stack really quickly, and Dwarf based options are too slow.
- PhilipRoman 4y ago
  Maybe I'm missing something but isn't it redundant to have both stack and frame pointers? Just use negative offsets from the frame pointer to address local variables.

mgaunard 4y ago

I never get this whole thing of compiling with -fno-omit-frame-pointer, it's already the default unless you enable optimizations.

bigdict 4y ago
Often you want to profile an optimized binary.

uecker 4y ago

In my opinion the improved code quality and bounds checking you can get with VLAs are worth it. Yes, the an attacker should not control the size and one should use -fstack-clash-protection and when the stack size is limited you should think twice (but vla can also reduce stack size if the alternative is a fixed size array on stack with larger fixed upper bound).

fanf2 4y ago
VLAs do not have extra bounds checking. Looking at it from an attacker’s point of view, a VLA is a primitive for adding an arbitrary offset to the stack pointer: extremely powerful and dangerous. You can’t use VLAs for large allocations because the stack has a limited amount of space and you don’t know what that limit is. So you must check the size is reasonable before declaring a VLA. So you might as well declare a fixed-size array that is large enough for all reasonable uses.
- uecker 4y ago
  VLAs can enable precise bounds checking with the undefined behavior sanitizer. This is useful and you do not have precise bounds checking anymore when you replace it with a larger fixed size array. Also you increase stack size when you replace with a larger fixed size array. Yes, one should not let an attacker control the size of the VLA. But in any case, one should use -fstack-clash-protection . With stack clash protection if an attacker can control the size of VLA and it becomes to large, you can a trap with is likely DoS. With a fixed size array which you overflow instead, it is more likely a RCE. If you check the size, it does not matter. That you can't use the stack for large allocations has nothing to with VLAs and also depends on what you call large and how large your stack is.
  - pjmlp 4y ago
    They were made optional in C11, while Google paid for the development effort to clean the Linux kernel of them, because they are only yet another source of possible CVEs.

Conscat 4y ago

Changing signed integer overflow is extremely stupid considering unsigned integer overflow is what does the wrong thing in C. Just use ubsan.

Jorengarenar 4y ago
> unsigned integer overflow is what does the wrong thing in C You may want to rethink this statement
- Conscat 4y ago
  No, I am correct. Unsigned integer overflow has wrapping overflow semantics. Absolutely idiotic.
  - Jorengarenar 4y ago
    So... unsigned overflow is defined, while signed is straight up UB... and for you it's the former that's wrong?
    - account42 4y ago
      Yes. Most calculations won't do the right thing with wrapping overflow and you'd be better of by them being undefined and tested via the relevant tooling (e.g. ubsan). Those that do should be written in a way to make that explicit.
    - Conscat 4y ago
      Compilers can make commutative optimizations with signed integer overflow that is impossible with wrapping semantics. This is why Zig and Carbon both make unsigned integer overflow undefined as well. There is absolutely no advantage in most cases to wrapping semantics unless it is actually required for the algorithm (like some hashing or PRNG functions, for example).

Jorengarenar 4y ago

> Warn about any use of stack-allocated variable length arrays. Unfortunately, it warns about all VLA, not only stack allocated ones. What you might want is rather: -Wvla-larger-than=0 This flag will indeed report only automatic VLA (but for some reason only if optimizations are turned on [1]). [1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109395 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109395