4 ms·

Broadening compiler checks for buffer overflows in _FORTIFY_SOURCE (2021)

32 points by limoce 5y ago

akie 5y ago
I understand the need for constructions like this, and I understand the limitations you work with when dealing with older languages such as C or C++, but does anyone else think that this is just incredibly hacky? I mean, this is the kind of stuff that needs to be taken care of at the language level. But I guess that's impossible, so we have this instead. Still, progress! I guess.
- nyanpasu64 5y ago
  Personally if I wanted to write similar functionality in standard C++ (much like fmtlib's compile-time format string checking) rather than compiler-specific C extensions, I'd try making wmemcpy a template on the T* being passed in, then check against sizeof(T). This raises several questions: - Does this falsely raise buffer overflow errors for T with flexible array members where the actual allocation is bigger than the T? - On a related note, is it even legal to memcpy more than a T's worth of data into a T? (IMO you shouldn't be doing that, but I think it's legal in C, and not with my template trick.) - You can't take the address of a template function. What happens when you try to take &wmemcpy? Do you get a non-inlined wmemcpy with a failing branch which never knows the object size? A call to __builtin_dynamic_object_size? Or &__original_wmemcpy?
  - steerablesafe 5y ago
    > is it even legal to memcpy more than a T's worth of data into a T? (IMO you shouldn't be doing that, but I think it's legal in C, and not with my template trick.) How can you know if it's a single `T` or an array of `T` objects? You can memcpy into an array of `T` through a `T*` with no limitation on the size. I don't see anything inherently wrong with that either.
    - nyanpasu64 5y ago
      Oh, forgot about that case.
- staticassertion 5y ago
  IDK I think it's kind of cool. I don't see this as being much hackier than any other compiler pass that looks for a pattern and inserts or removes code. Is it any hackier than totally randomizing memory placements? Obviously C and C++ are dumpster fires for these sorts of bugs, but this seems like a reasonable approach.
- chasil 5y ago
  For a language developed mostly in the 70s, in which so much system software is written, any defenses are worthwhile. It might be unwise to use this in systemd (as triggering these traps would crash the system), but it would be helpful in many other places, especially in processes that have called listen() to be network servers.
  - nayuki 5y ago
    Crashing the system would be preferable to corrupting memory and crashing the system at an undetermined time in the future.
    - chasil 5y ago
      This is not necessarily true. These memory bugs in the X server were very long lived. "Some people are not happy to find that previously reliable programs suddenly start crashing. However, as one person put it, they would rather have an application crash than introduce a subtle but undetected corruption in some mission critical database that does not get detected until three months down the road. One such application is X(7), the graphics interface for nearly any UNIX based platforms, including Linux, Solaris, and the other BSD distributions. It was discovered that the X server would crash in several different ways." https://undeadly.org/cgi?action=article;sid=20051224192032 https://undeadly.org/cgi?action=article;sid=20051224192032
nyanpasu64 5y ago
Ahh, I came across this article trying and failing to rebuild glibc in debug mode on Arch Linux, and it would always error out due to _FORTIFY_SOURCE (eeg. https://bbs.archlinux.org/viewtopic.php?id=245755 https://bbs.archlinux.org/viewtopic.php?id=245755). IIRC I tried creating a chroot but ran into the same error (or couldn't make the chroot work, forgot which). In the end I gave up rebuilding glibc in debug mode (since it would've slowed down all my programs). I still don't know what I did wrong; maybe glibc is just incompatible with optimizations off. Nowadays Arch uploads package symbols to debug packages and servers accessible by debuginfod (https://wiki.archlinux.org/title/Debugging/Getting_traces https://wiki.archlinux.org/title/Debugging/Getting_traces), but I've observed debuginfod greatly slows down gdb and valgrind and strace (so I don't set the DEBUGINFOD_URLS environment variable by default, only when actually debugging).
deleted 5y ago
[deleted]
staticassertion 5y ago
> This promises to significantly widen fortification coverage to include cases where the compiler can see the non-constant expression for object size. Any stats on the coverage increase?
nayuki 5y ago
So _FORTIFY_SOURCE adds checks to functions like memcpy(), but seems to do nothing to help custom code that uses for loops and array indexing. I guess I'll keep using -fsanitize=address (ASan) in my debug builds.
- fwsgonzo 5y ago
  Some of those clever loops end up calling memcpy anyway because the compiler deems it to be faster, but yes. For better safety there needs to be bounds checking.
- dundarious 5y ago
  Clang, and I believe gcc and many other compilers, can synthesize a memcpy from simple loops. I don't say this to vouch for its abilities, and in general I don't recommend relying on such transformations, but it will help probably for a non-trivial amount of such custom code -- but of course, nowhere near all.

32 points by limoce 5y ago

akie 5y ago

I understand the need for constructions like this, and I understand the limitations you work with when dealing with older languages such as C or C++, but does anyone else think that this is just incredibly hacky? I mean, this is the kind of stuff that needs to be taken care of at the language level. But I guess that's impossible, so we have this instead. Still, progress! I guess.

nyanpasu64 5y ago
Personally if I wanted to write similar functionality in standard C++ (much like fmtlib's compile-time format string checking) rather than compiler-specific C extensions, I'd try making wmemcpy a template on the T* being passed in, then check against sizeof(T). This raises several questions: - Does this falsely raise buffer overflow errors for T with flexible array members where the actual allocation is bigger than the T? - On a related note, is it even legal to memcpy more than a T's worth of data into a T? (IMO you shouldn't be doing that, but I think it's legal in C, and not with my template trick.) - You can't take the address of a template function. What happens when you try to take &wmemcpy? Do you get a non-inlined wmemcpy with a failing branch which never knows the object size? A call to __builtin_dynamic_object_size? Or &__original_wmemcpy?
- steerablesafe 5y ago
  > is it even legal to memcpy more than a T's worth of data into a T? (IMO you shouldn't be doing that, but I think it's legal in C, and not with my template trick.) How can you know if it's a single `T` or an array of `T` objects? You can memcpy into an array of `T` through a `T*` with no limitation on the size. I don't see anything inherently wrong with that either.
  - nyanpasu64 5y ago
    Oh, forgot about that case.
staticassertion 5y ago
IDK I think it's kind of cool. I don't see this as being much hackier than any other compiler pass that looks for a pattern and inserts or removes code. Is it any hackier than totally randomizing memory placements? Obviously C and C++ are dumpster fires for these sorts of bugs, but this seems like a reasonable approach.
chasil 5y ago
For a language developed mostly in the 70s, in which so much system software is written, any defenses are worthwhile. It might be unwise to use this in systemd (as triggering these traps would crash the system), but it would be helpful in many other places, especially in processes that have called listen() to be network servers.
- nayuki 5y ago
  Crashing the system would be preferable to corrupting memory and crashing the system at an undetermined time in the future.
  - chasil 5y ago
    This is not necessarily true. These memory bugs in the X server were very long lived. "Some people are not happy to find that previously reliable programs suddenly start crashing. However, as one person put it, they would rather have an application crash than introduce a subtle but undetected corruption in some mission critical database that does not get detected until three months down the road. One such application is X(7), the graphics interface for nearly any UNIX based platforms, including Linux, Solaris, and the other BSD distributions. It was discovered that the X server would crash in several different ways." https://undeadly.org/cgi?action=article;sid=20051224192032 https://undeadly.org/cgi?action=article;sid=20051224192032

nyanpasu64 5y ago

Ahh, I came across this article trying and failing to rebuild glibc in debug mode on Arch Linux, and it would always error out due to _FORTIFY_SOURCE (eeg. https://bbs.archlinux.org/viewtopic.php?id=245755 https://bbs.archlinux.org/viewtopic.php?id=245755). IIRC I tried creating a chroot but ran into the same error (or couldn't make the chroot work, forgot which). In the end I gave up rebuilding glibc in debug mode (since it would've slowed down all my programs). I still don't know what I did wrong; maybe glibc is just incompatible with optimizations off. Nowadays Arch uploads package symbols to debug packages and servers accessible by debuginfod (https://wiki.archlinux.org/title/Debugging/Getting_traces https://wiki.archlinux.org/title/Debugging/Getting_traces), but I've observed debuginfod greatly slows down gdb and valgrind and strace (so I don't set the DEBUGINFOD_URLS environment variable by default, only when actually debugging).

deleted 5y ago

[deleted]

staticassertion 5y ago

> This promises to significantly widen fortification coverage to include cases where the compiler can see the non-constant expression for object size. Any stats on the coverage increase?

nayuki 5y ago

So _FORTIFY_SOURCE adds checks to functions like memcpy(), but seems to do nothing to help custom code that uses for loops and array indexing. I guess I'll keep using -fsanitize=address (ASan) in my debug builds.

fwsgonzo 5y ago
Some of those clever loops end up calling memcpy anyway because the compiler deems it to be faster, but yes. For better safety there needs to be bounds checking.
dundarious 5y ago
Clang, and I believe gcc and many other compilers, can synthesize a memcpy from simple loops. I don't say this to vouch for its abilities, and in general I don't recommend relying on such transformations, but it will help probably for a non-trivial amount of such custom code -- but of course, nowhere near all.