[PR PATCH] [RFC] Add _FORTIFY_SOURCE impl for musl.

[PR PATCH] [RFC] Add _FORTIFY_SOURCE impl for musl.

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

There is a new pull request by ericonr against master on the void-packages repository

https://github.com/ericonr/void-packages fortify
https://github.com/void-linux/void-packages/pull/24254

[RFC] Add _FORTIFY_SOURCE impl for musl.
Using the `fortify-headers` impl from https://git.2f30.org/fortify-headers/file/README.html , which was recommended in https://wiki.musl-libc.org/open-issues.html#Substitute-for-%3Ccode%3E_FORTIFY_SOURCE%3C/code%3E .

Would definitely require extensive testing, I mostly want to know whether going through this testing is worth it.

A patch file from https://github.com/void-linux/void-packages/pull/24254.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-fortify-24254.patch --]
[-- Type: text/x-diff, Size: 3161 bytes --]

From 0f1b17be07ed91f70922978b91c7cc99e7b65f63 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Wed, 12 Aug 2020 22:37:50 -0300
Subject: [PATCH 1/3] New package: fortify-headers-1.1.

---
 srcpkgs/fortify-headers/template | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 srcpkgs/fortify-headers/template

diff --git a/srcpkgs/fortify-headers/template b/srcpkgs/fortify-headers/template
new file mode 100644
index 00000000000..33c00322c4d
--- /dev/null
+++ b/srcpkgs/fortify-headers/template
@@ -0,0 +1,20 @@
+# Template file for 'fortify-headers'
+pkgname=fortify-headers
+version=1.1
+revision=1
+archs="*-musl"
+build_style=gnu-makefile
+short_desc="Standalone implementation of fortify source"
+maintainer="Érico Rolim <erico.erc@gmail.com>"
+license="ISC"
+homepage="https://git.2f30.org/fortify-headers/"
+distfiles="http://dl.2f30.org/releases/${pkgname}-${version}.tar.gz"
+checksum=6ba5d860a2d2ba4c3346924b93930c34856eafe148bdbdf271ecab8065201fb6
+
+do_build() {
+	:
+}
+
+post_install() {
+	vlicense LICENSE
+}

From c16148f3bf0704413c1cc7ee791e4d6fd5672b53 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Wed, 12 Aug 2020 22:38:10 -0300
Subject: [PATCH 2/3] musl: have musl-devel depend on fortify-headers.

---
 srcpkgs/musl/template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/srcpkgs/musl/template b/srcpkgs/musl/template
index 22e79588acd..1c3af3e23f8 100644
--- a/srcpkgs/musl/template
+++ b/srcpkgs/musl/template
@@ -2,7 +2,7 @@
 pkgname=musl
 reverts="1.2.0_1"
 version=1.1.24
-revision=3
+revision=4
 archs="*-musl"
 bootstrap=yes
 build_style=gnu-configure
@@ -48,7 +48,7 @@ do_install() {
 }
 
 musl-devel_package() {
-	depends="kernel-libc-headers ${sourcepkg}-${version}_${revision}"
+	depends="kernel-libc-headers fortify-headers ${sourcepkg}-${version}_${revision}"
 	short_desc+=" - development files"
 	pkg_install() {
 		vmove usr/include

From e70c843db6e4a64d4b88bf382f05ebedf7bd050c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Wed, 12 Aug 2020 22:39:11 -0300
Subject: [PATCH 3/3] hardening: enable fortify-headers for musl.

---
 common/environment/configure/hardening.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/environment/configure/hardening.sh b/common/environment/configure/hardening.sh
index 10b67c75fd8..d7658ee37be 100644
--- a/common/environment/configure/hardening.sh
+++ b/common/environment/configure/hardening.sh
@@ -17,8 +17,8 @@ if [ -z "$nopie" ]; then
 		LDFLAGS="-specs=${_GCCSPECSDIR}/hardened-ld -Wl,-z,relro -Wl,-z,now ${LDFLAGS}"
 	else
 		# Enable FORITFY_SOURCE=2
-		CFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 ${CFLAGS}"
-		CXXFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 ${CXXFLAGS}"
+		CFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 -I${XBPS_CROSS_BASE}/usr/include/fortify ${CFLAGS}"
+		CXXFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 -I${XBPS_CROSS_BASE}/usr/include/fortify ${CXXFLAGS}"
 		LDFLAGS="-Wl,-z,relro -Wl,-z,now ${LDFLAGS}"
 	fi
 else

Re: [PR PATCH] [Updated] [RFC] Add _FORTIFY_SOURCE impl for musl.

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 663 bytes --]

There is an updated pull request by ericonr against master on the void-packages repository

https://github.com/ericonr/void-packages fortify
https://github.com/void-linux/void-packages/pull/24254

[RFC] Add _FORTIFY_SOURCE impl for musl.
Using the `fortify-headers` impl from https://git.2f30.org/fortify-headers/file/README.html , which was recommended in https://wiki.musl-libc.org/open-issues.html#Substitute-for-%3Ccode%3E_FORTIFY_SOURCE%3C/code%3E .

Would definitely require extensive testing, I mostly want to know whether going through this testing is worth it.

A patch file from https://github.com/void-linux/void-packages/pull/24254.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-fortify-24254.patch --]
[-- Type: text/x-diff, Size: 3164 bytes --]

From 0f4251ec56ac471886be6f34317f8ff7588dcc22 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Wed, 12 Aug 2020 22:37:50 -0300
Subject: [PATCH 1/3] New package: fortify-headers-1.1.

---
 srcpkgs/fortify-headers/template | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 srcpkgs/fortify-headers/template

diff --git a/srcpkgs/fortify-headers/template b/srcpkgs/fortify-headers/template
new file mode 100644
index 00000000000..1a82dd1dd44
--- /dev/null
+++ b/srcpkgs/fortify-headers/template
@@ -0,0 +1,20 @@
+# Template file for 'fortify-headers'
+pkgname=fortify-headers
+version=1.1
+revision=1
+archs="*-musl"
+build_style=gnu-makefile
+short_desc="Standalone implementation of fortify source"
+maintainer="Érico Nogueira <ericonr@disroot.org>"
+license="ISC"
+homepage="https://git.2f30.org/fortify-headers/"
+distfiles="http://dl.2f30.org/releases/${pkgname}-${version}.tar.gz"
+checksum=6ba5d860a2d2ba4c3346924b93930c34856eafe148bdbdf271ecab8065201fb6
+
+do_build() {
+	:
+}
+
+post_install() {
+	vlicense LICENSE
+}

From 44d73d5ebe5a4e3bb216862faa121f55bcfc80e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Wed, 12 Aug 2020 22:38:10 -0300
Subject: [PATCH 2/3] musl: have musl-devel depend on fortify-headers.

---
 srcpkgs/musl/template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/srcpkgs/musl/template b/srcpkgs/musl/template
index 22e79588acd..1c3af3e23f8 100644
--- a/srcpkgs/musl/template
+++ b/srcpkgs/musl/template
@@ -2,7 +2,7 @@
 pkgname=musl
 reverts="1.2.0_1"
 version=1.1.24
-revision=3
+revision=4
 archs="*-musl"
 bootstrap=yes
 build_style=gnu-configure
@@ -48,7 +48,7 @@ do_install() {
 }
 
 musl-devel_package() {
-	depends="kernel-libc-headers ${sourcepkg}-${version}_${revision}"
+	depends="kernel-libc-headers fortify-headers ${sourcepkg}-${version}_${revision}"
 	short_desc+=" - development files"
 	pkg_install() {
 		vmove usr/include

From a7f51f06b8ad9a7955211e2296682a547f8fdcf9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Wed, 12 Aug 2020 22:39:11 -0300
Subject: [PATCH 3/3] hardening: enable fortify-headers for musl.

---
 common/environment/configure/hardening.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/environment/configure/hardening.sh b/common/environment/configure/hardening.sh
index 10b67c75fd8..d7658ee37be 100644
--- a/common/environment/configure/hardening.sh
+++ b/common/environment/configure/hardening.sh
@@ -17,8 +17,8 @@ if [ -z "$nopie" ]; then
 		LDFLAGS="-specs=${_GCCSPECSDIR}/hardened-ld -Wl,-z,relro -Wl,-z,now ${LDFLAGS}"
 	else
 		# Enable FORITFY_SOURCE=2
-		CFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 ${CFLAGS}"
-		CXXFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 ${CXXFLAGS}"
+		CFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 -I${XBPS_CROSS_BASE}/usr/include/fortify ${CFLAGS}"
+		CXXFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 -I${XBPS_CROSS_BASE}/usr/include/fortify ${CXXFLAGS}"
 		LDFLAGS="-Wl,-z,relro -Wl,-z,now ${LDFLAGS}"
 	fi
 else

Re: [RFC] Add _FORTIFY_SOURCE impl for musl.

[q66]

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/24254#issuecomment-673248776

Comment:
no, let's not add useless bounds checks and branches all around the stdlib in all our release software please

Re: [RFC] Add _FORTIFY_SOURCE impl for musl.

[q66]

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/24254#issuecomment-673248776

Comment:
no, let's not add useless bounds checks and branches all around the stdlib in all our release software please

i also have concerns that this will potentially break builds of stuff in unexpected ways, since it's a third party set of headers that doesn't come with libc and just forward-includes stuff and renames the APIs (as opposed to glibc which just comes with separate headers for fortified and regular) - would need to be tested though

Re: [RFC] Add _FORTIFY_SOURCE impl for musl.

[q66]

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/24254#issuecomment-673254502

Comment:
looking further at how it works, if we wanted to include this, definitely not in this manner - instead we'd do it like alpine and have the compiler implicitly inject it (with `-isystem` rather than `-I`) as including it in normal flags via `-I` is just asking for trouble (for one we want to treat them like system headers, to avoid potential warnings and so on...)

Re: [RFC] Add _FORTIFY_SOURCE impl for musl.

[q66]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/24254#issuecomment-673248776

Comment:
no, let's not add useless bounds checks and branches all around the stdlib in all our release software please (looking at the source, this does not utilize gcc's fortified builtins, even though it should, a lot of the stuff instead seems to be implemented trivially)

i also have concerns that this will potentially break builds of stuff in unexpected ways, since it's a third party set of headers that doesn't come with libc and just forward-includes stuff and renames the APIs (as opposed to glibc which just comes with separate headers for fortified and regular) - would need to be tested though

Re: [PR PATCH] [Updated] [RFC] Add _FORTIFY_SOURCE impl for musl.

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 663 bytes --]

There is an updated pull request by ericonr against master on the void-packages repository

https://github.com/ericonr/void-packages fortify
https://github.com/void-linux/void-packages/pull/24254

[RFC] Add _FORTIFY_SOURCE impl for musl.
Using the `fortify-headers` impl from https://git.2f30.org/fortify-headers/file/README.html , which was recommended in https://wiki.musl-libc.org/open-issues.html#Substitute-for-%3Ccode%3E_FORTIFY_SOURCE%3C/code%3E .

Would definitely require extensive testing, I mostly want to know whether going through this testing is worth it.

A patch file from https://github.com/void-linux/void-packages/pull/24254.patch is attached

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: github-pr-fortify-24254.patch --]
[-- Type: text/x-diff, Size: 3178 bytes --]

From 0f4251ec56ac471886be6f34317f8ff7588dcc22 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Wed, 12 Aug 2020 22:37:50 -0300
Subject: [PATCH 1/3] New package: fortify-headers-1.1.

---
 srcpkgs/fortify-headers/template | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 srcpkgs/fortify-headers/template

diff --git a/srcpkgs/fortify-headers/template b/srcpkgs/fortify-headers/template
new file mode 100644
index 00000000000..1a82dd1dd44
--- /dev/null
+++ b/srcpkgs/fortify-headers/template
@@ -0,0 +1,20 @@
+# Template file for 'fortify-headers'
+pkgname=fortify-headers
+version=1.1
+revision=1
+archs="*-musl"
+build_style=gnu-makefile
+short_desc="Standalone implementation of fortify source"
+maintainer="Érico Nogueira <ericonr@disroot.org>"
+license="ISC"
+homepage="https://git.2f30.org/fortify-headers/"
+distfiles="http://dl.2f30.org/releases/${pkgname}-${version}.tar.gz"
+checksum=6ba5d860a2d2ba4c3346924b93930c34856eafe148bdbdf271ecab8065201fb6
+
+do_build() {
+	:
+}
+
+post_install() {
+	vlicense LICENSE
+}

From 44d73d5ebe5a4e3bb216862faa121f55bcfc80e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Wed, 12 Aug 2020 22:38:10 -0300
Subject: [PATCH 2/3] musl: have musl-devel depend on fortify-headers.

---
 srcpkgs/musl/template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/srcpkgs/musl/template b/srcpkgs/musl/template
index 22e79588acd..1c3af3e23f8 100644
--- a/srcpkgs/musl/template
+++ b/srcpkgs/musl/template
@@ -2,7 +2,7 @@
 pkgname=musl
 reverts="1.2.0_1"
 version=1.1.24
-revision=3
+revision=4
 archs="*-musl"
 bootstrap=yes
 build_style=gnu-configure
@@ -48,7 +48,7 @@ do_install() {
 }
 
 musl-devel_package() {
-	depends="kernel-libc-headers ${sourcepkg}-${version}_${revision}"
+	depends="kernel-libc-headers fortify-headers ${sourcepkg}-${version}_${revision}"
 	short_desc+=" - development files"
 	pkg_install() {
 		vmove usr/include

From e7d19004a5b0aab8bab89b085f934e6e4992169a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=89rico=20Rolim?= <erico.erc@gmail.com>
Date: Wed, 12 Aug 2020 22:39:11 -0300
Subject: [PATCH 3/3] hardening: enable fortify-headers for musl.

---
 common/environment/configure/hardening.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/environment/configure/hardening.sh b/common/environment/configure/hardening.sh
index 10b67c75fd8..a70b52254e2 100644
--- a/common/environment/configure/hardening.sh
+++ b/common/environment/configure/hardening.sh
@@ -17,8 +17,8 @@ if [ -z "$nopie" ]; then
 		LDFLAGS="-specs=${_GCCSPECSDIR}/hardened-ld -Wl,-z,relro -Wl,-z,now ${LDFLAGS}"
 	else
 		# Enable FORITFY_SOURCE=2
-		CFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 ${CFLAGS}"
-		CXXFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 ${CXXFLAGS}"
+		CFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 -isystem ${XBPS_CROSS_BASE}/usr/include/fortify ${CFLAGS}"
+		CXXFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2 -isystem ${XBPS_CROSS_BASE}/usr/include/fortify ${CXXFLAGS}"
 		LDFLAGS="-Wl,-z,relro -Wl,-z,now ${LDFLAGS}"
 	fi
 else

Re: [RFC] Add _FORTIFY_SOURCE impl for musl.

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 601 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/24254#issuecomment-673276440

Comment:
> with -isystem rather than -I

So like I just changed it?

---

I considered making this PR mostly because dalias said he likes the design, so I didn't expect any huge issues with the impl. Might not be the most optimal, though.

Are your issues more with the idea of having `_FORTIFY_SOURCE` functional on musl, but requiring more bounds checks and binary size than it used to, or with the fact that this lib isn't as well implemented as it could be?

Re: [RFC] Add _FORTIFY_SOURCE impl for musl.

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/24254#issuecomment-673276440

Comment:
> with -isystem rather than -I

So like I just changed it?

---

I considered making this PR mostly because dalias said he agrees the design, so I didn't expect any huge issues with the impl. Might not be the most optimal, though.

Are your issues more with the idea of having `_FORTIFY_SOURCE` functional on musl, but requiring more bounds checks and binary size than it used to, or with the fact that this lib isn't as well implemented as it could be?

Re: [RFC] Add _FORTIFY_SOURCE impl for musl.

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/24254#issuecomment-673276440

Comment:
> with -isystem rather than -I

So like I just changed it?

---

I considered making this PR mostly because dalias said he agrees with the design, so I didn't expect any huge issues with the impl. Might not be the most optimal, though.

Are your issues more with the idea of having `_FORTIFY_SOURCE` functional on musl, but requiring more bounds checks and binary size than it used to, or with the fact that this lib isn't as well implemented as it could be?

Re: [RFC] Add _FORTIFY_SOURCE impl for musl.

[q66]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/24254#issuecomment-673508244

Comment:
`XBPS_CROSS_BASE` will not work since the libc is not installed for crossbuilds (it's shipped by crosstoolchains)

I'm not fundamentally opposed to using fortify, the library could use cleaning up (it'd simplify it too, e.g. use `__builtin___memcpy_chk` and so on) and the way you implemented it in the PR is questionable

if we're to add this, the best opportunity is when doing the gcc10 transition

Re: [RFC] Add _FORTIFY_SOURCE impl for musl.

[sgn]

[-- Attachment #1: Type: text/plain, Size: 337 bytes --]

New comment by sgn on void-packages repository

https://github.com/void-linux/void-packages/pull/24254#issuecomment-673509816

Comment:
Even if we want this merged in, as of it's now, we can't bootstrap from foreign distro.
At least add `bootstrap=yes` to `fortify-headers`.
**And** some code to append those header to bootstrap step.

Re: [RFC] Add _FORTIFY_SOURCE impl for musl.

[ericonr]

[-- Attachment #1: Type: text/plain, Size: 744 bytes --]

New comment by ericonr on void-packages repository

https://github.com/void-linux/void-packages/pull/24254#issuecomment-673515683

Comment:
> XBPS_CROSS_BASE will not work since the libc is not installed for crossbuilds (it's shipped by crosstoolchains)

I know, I have to add them there too. I just didn't want to go through the trouble before I had an ok.

> and the way you implemented it in the PR is questionable

Which part specifically?

> the library could use cleaning up

If you could give me some tips, I would gladly make some changes. Would just have to find out whether upstream is active or if we'd need to fork it under void-linux (?).

> And some code to append those header to bootstrap step.

Any tips on this?

Re: [RFC] Add _FORTIFY_SOURCE impl for musl.

[q66]

[-- Attachment #1: Type: text/plain, Size: 292 bytes --]

New comment by q66 on void-packages repository

https://github.com/void-linux/void-packages/pull/24254#issuecomment-673521875

Comment:
as i said, this should be patched into gcc itself (i.e. like alpine does it)

i'll close this and you can bring it up when the gcc10 update is being done

Re: [PR PATCH] [Closed]: [RFC] Add _FORTIFY_SOURCE impl for musl.

[q66]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

There's a closed pull request on the void-packages repository

[RFC] Add _FORTIFY_SOURCE impl for musl.
https://github.com/void-linux/void-packages/pull/24254

Description:
Using the `fortify-headers` impl from https://git.2f30.org/fortify-headers/file/README.html , which was recommended in https://wiki.musl-libc.org/open-issues.html#Substitute-for-%3Ccode%3E_FORTIFY_SOURCE%3C/code%3E .

Would definitely require extensive testing, I mostly want to know whether going through this testing is worth it.