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

Compiler guide: Add note about redefining default of FORTIFY_SOURCE #273

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

gkunz
Copy link
Contributor

@gkunz gkunz commented Nov 1, 2023

In case a compiler uses a predefined default for FORTIFY_SOURCE and a different mode is set on the command line, a warning about redefining the macro is triggered. To avoid this, the default value of FORTIFY_SOURCE can be unset first before setting the desired value.

Fixes #272

@gkunz
Copy link
Contributor Author

gkunz commented Nov 1, 2023

See also the findings in #270.

@@ -124,7 +124,7 @@ Table 2: Recommended compiler options that enable run-time protection mechanisms

| Compiler Flag | Supported since | Description |
|:----------------------------------------------------------------------------------------- |:----------------------------------:|:-------------------------------------------------------------------------------------------- |
| [`-D_FORTIFY_SOURCE=3`](#-D_FORTIFY_SOURCE=3) <br/>(requires `-O1` or higher) | GCC 12.0<br/>Clang 9.0.0[^Guelton20] | Fortify sources with compile- and run-time checks for unsafe libc usage and buffer overflows. Some fortification levels can impact performance. |
| [`-D_FORTIFY_SOURCE=3`](#-D_FORTIFY_SOURCE=3) <br/>(requires `-O1` or higher, <br/> may require -U_FORTIFY_SOURCE) | GCC 12.0<br/>Clang 9.0.0[^Guelton20] | Fortify sources with compile- and run-time checks for unsafe libc usage and buffer overflows. Some fortification levels can impact performance. |
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer "may require prepending -U_FORTIFY_SOURCE" to avoid the interpretation that one should specify -D_FORTIFY_SOURCE=3 -U_FORTIFY_SOURCE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -267,6 +266,8 @@ To benefit from `_FORTIFY_SOURCE` checks following requirements must be met:

If checks added by `_FORTIFY_SOURCE` detect unsafe behavior at run-time they will print an error message and terminate the application.

A default mode for FORTIFY_SOURCE may be predefined for a given compiler, for instance gcc shipped with Ubuntu 22.04 uses FORTIFY_SOURCE=2 by default. If a mode of FORTIFY_SOURCE is set on the command line which differs from the default, the compiler warns about redefining the FORTIFY_SOURCE macro. To avoid this, the predefined mode can be unset with -U_FORTIFY_SOURCE before setting the desired value.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: We are capitalizing GCC elsewhere in the guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

gkunz added 2 commits November 4, 2023 00:01
In case a compiler uses a predefined default for FORTIFY_SOURCE and a different
mode is set on the command line, a warning about redefining the macro is
triggered. To avoid this, the default value of FORTIFY_SOURCE can be unset first
before setting the desired value.

Signed-off-by: Georg Kunz <[email protected]>
@gkunz gkunz force-pushed the undefine-fortify-source branch from f097d3b to d6d8d23 Compare November 3, 2023 23:05
@thomasnyman
Copy link
Contributor

lgtm

@thomasnyman thomasnyman merged commit 272068b into ossf:main Nov 7, 2023
2 checks passed
@gkunz gkunz deleted the undefine-fortify-source branch April 10, 2024 19:08
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.

Compiler Hardening Guide: add note about redefining default of FORTIFY_SOURCE
3 participants