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

cmake flag USE_HARDENING is effectively ignored #1380

Open
Viech opened this issue Oct 21, 2024 · 6 comments
Open

cmake flag USE_HARDENING is effectively ignored #1380

Viech opened this issue Oct 21, 2024 · 6 comments
Labels

Comments

@Viech
Copy link
Member

Viech commented Oct 21, 2024

I believe

	if (USE_HARDENING OR NOT MINGW)
		# MinGW with _FORTIFY_SOURCE and without -fstack-protector
		# causes unsatisfied dependency on libssp.
		# https://github.com/msys2/MINGW-packages/issues/5868
		set_c_cxx_flag("-D_FORTIFY_SOURCE=2" RELEASE)
		set_c_cxx_flag("-D_FORTIFY_SOURCE=2" RELWITHDEBINFO)
		set_c_cxx_flag("-D_FORTIFY_SOURCE=2" MINSIZEREL)
		# Don't set _FORTIFY_SOURCE in debug builds.
	endif()

should rather begin with if (USE_HARDENING AND NOT MINGW) in DaemonFlags.cmake.

@Viech
Copy link
Member Author

Viech commented Oct 21, 2024

One might want to let USE_HARDENING default to ON if the above is fixed.

@Viech
Copy link
Member Author

Viech commented Oct 21, 2024

I should add that I want the USE_HARDENING flag to be retained as having it enabled clashes with the system-wide setting in /etc/makepkg.conf on Arch (causing lots of compiler warnings). So there are good reasons the user might want it disabled.

@slipher
Copy link
Member

slipher commented Oct 21, 2024

Well, it wasn't written as a logic error. The diff adding it:

-    set_c_cxx_flag("-D_FORTIFY_SOURCE=2" RELEASE)
-    set_c_cxx_flag("-D_FORTIFY_SOURCE=2" RELWITHDEBINFO)
-    set_c_cxx_flag("-D_FORTIFY_SOURCE=2" MINSIZEREL)
+    if (USE_HARDENING OR NOT MINGW)
+        # MinGW with _FORTIFY_SOURCE and without -fstack-protector causes unsatisfied dependency on libssp
+        # https://github.com/msys2/MINGW-packages/issues/5868
+        set_c_cxx_flag("-D_FORTIFY_SOURCE=2" RELEASE)
+        set_c_cxx_flag("-D_FORTIFY_SOURCE=2" RELWITHDEBINFO)
+        set_c_cxx_flag("-D_FORTIFY_SOURCE=2" MINSIZEREL)
+    endif()

It's like, by default we use a medium amount of hardening but if you set USE_HARDENING we use maximum hardening. But on MinGW medium hardening doesn't work and has to be disabled.

@DolceTriade
Copy link
Contributor

The logic is still wrong. The correct logic would be to remove USE_HARDENING, unless the idea is to allow MINGW to force enable hardening?

@slipher
Copy link
Member

slipher commented Oct 21, 2024

There are other flags enabled only with USE_HARDENING.

@DolceTriade
Copy link
Contributor

Sure, not saying remove the flag USE_HARDENING, just remove it from this condition.

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

No branches or pull requests

3 participants