[Buildroot] [PATCH] mmc-utils: Rely on our own _FORTIFY_SOURCE

[Buildroot] [PATCH] mmc-utils: Rely on our own _FORTIFY_SOURCE

[Jan Kundrát]

Upstream always forced _FORTIFY_SOURCE=2. At first, I tried changing
their flags to undefine that macro first, but that did not work well.
This patch ensures that the package always respects our toolchain
settings, even in cases where we are not fortifying the build.

This fixes a build failure where the mmc-utils package failed to build
with, e.g., BR2_FORTIFY_SOURCE_1:

  <command-line>:0:0: error: "_FORTIFY_SOURCE" redefined [-Werror]
  <command-line>:0:0: note: this is the location of the previous definition

Signed-off-by: Jan Kundr?t <jan.kundrat@cesnet.cz>
---
 ...on-the-build-env-for-_FORTIFY_SOURCE.patch | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)
 create mode 100644 package/mmc-utils/0001-Rely-on-the-build-env-for-_FORTIFY_SOURCE.patch

diff --git a/package/mmc-utils/0001-Rely-on-the-build-env-for-_FORTIFY_SOURCE.patch b/package/mmc-utils/0001-Rely-on-the-build-env-for-_FORTIFY_SOURCE.patch
new file mode 100644
index 0000000000..b099efa6c4
--- /dev/null
+++ b/package/mmc-utils/0001-Rely-on-the-build-env-for-_FORTIFY_SOURCE.patch
@@ -0,0 +1,27 @@
+From 0c893e6f272351572548264bf423208a7b76bb16 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Jan=20Kundr=C3=A1t?= <jan.kundrat@cesnet.cz>
+Date: Tue, 7 Aug 2018 17:29:35 +0200
+Subject: [PATCH] Rely on the build env for _FORTIFY_SOURCE
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Signed-off-by: Jan Kundr?t <jan.kundrat@cesnet.cz>
+---
+ Makefile | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/Makefile b/Makefile
+index aa27ff2..a3890b2 100644
+--- a/Makefile
++++ b/Makefile
+@@ -1,5 +1,5 @@
+ CC ?= gcc
+-AM_CFLAGS = -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2
++AM_CFLAGS = -D_FILE_OFFSET_BITS=64
+ CFLAGS ?= -g -O2
+ objects = \
+ 	mmc.o \
+-- 
+2.17.1
+
-- 
2.17.1

[Buildroot] [PATCH] mmc-utils: Rely on our own _FORTIFY_SOURCE

[Jan Kundrát]

On ?ter? 7. srpna 2018 17:32:51 CEST, Jan Kundr?t wrote:
> Upstream always forced _FORTIFY_SOURCE=2. At first, I tried changing
> their flags to undefine that macro first, but that did not work well.
> This patch ensures that the package always respects our toolchain
> settings, even in cases where we are not fortifying the build.
>
> This fixes a build failure where the mmc-utils package failed to build
> with, e.g., BR2_FORTIFY_SOURCE_1:
>
>   <command-line>:0:0: error: "_FORTIFY_SOURCE" redefined [-Werror]
>   <command-line>:0:0: note: this is the location of the previous definition
>
> Signed-off-by: Jan Kundr?t <jan.kundrat@cesnet.cz>
> ---
>  ...on-the-build-env-for-_FORTIFY_SOURCE.patch | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
>  create mode 100644 
> package/mmc-utils/0001-Rely-on-the-build-env-for-_FORTIFY_SOURCE.patch
>
> diff --git 
> a/package/mmc-utils/0001-Rely-on-the-build-env-for-_FORTIFY_SOURCE.patch 
> b/package/mmc-utils/0001-Rely-on-the-build-env-for-_FORTIFY_SOURCE.patch
> new file mode 100644
> index 0000000000..b099efa6c4
> --- /dev/null
> +++ b/package/mmc-utils/0001-Rely-on-the-build-env-for-_FORTIFY_SOURCE.patch
> @@ -0,0 +1,27 @@
> +From 0c893e6f272351572548264bf423208a7b76bb16 Mon Sep 17 00:00:00 2001
> +From: =?UTF-8?q?Jan=20Kundr=C3=A1t?= <jan.kundrat@cesnet.cz>
> +Date: Tue, 7 Aug 2018 17:29:35 +0200
> +Subject: [PATCH] Rely on the build env for _FORTIFY_SOURCE
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +Signed-off-by: Jan Kundr?t <jan.kundrat@cesnet.cz>
> +---
> + Makefile | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/Makefile b/Makefile
> +index aa27ff2..a3890b2 100644
> +--- a/Makefile
> ++++ b/Makefile
> +@@ -1,5 +1,5 @@
> + CC ?= gcc
> +-AM_CFLAGS = -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2
> ++AM_CFLAGS = -D_FILE_OFFSET_BITS=64
> + CFLAGS ?= -g -O2
> + objects = \
> + 	mmc.o \
> +-- 
> +2.17.1
> +

This patch might not be needed when Matthew Weber's series ([PATCH 0/6] 
Hardening Flag Bugfix/Enhancement) lands. My recommendation is to hold it 
for a while.

With kind regards,
Jan

[Buildroot] [PATCH] mmc-utils: Rely on our own _FORTIFY_SOURCE

[Thomas Petazzoni]

Hello,

On Tue, 7 Aug 2018 17:32:51 +0200, Jan Kundr?t wrote:
> Upstream always forced _FORTIFY_SOURCE=2. At first, I tried changing
> their flags to undefine that macro first, but that did not work well.
> This patch ensures that the package always respects our toolchain
> settings, even in cases where we are not fortifying the build.
> 
> This fixes a build failure where the mmc-utils package failed to build
> with, e.g., BR2_FORTIFY_SOURCE_1:
> 
>   <command-line>:0:0: error: "_FORTIFY_SOURCE" redefined [-Werror]
>   <command-line>:0:0: note: this is the location of the previous definition
> 
> Signed-off-by: Jan Kundr?t <jan.kundrat@cesnet.cz>

Is this fixing a build issue reported by the autobuilders ? Or only
with hardening options ? Your commit log is not very verbose in details
about the configuration/condition under which the build failure happens.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH] mmc-utils: Rely on our own _FORTIFY_SOURCE

[Matthew Weber]

All,
On Wed, Aug 8, 2018 at 8:17 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> On Tue, 7 Aug 2018 17:32:51 +0200, Jan Kundr?t wrote:
> > Upstream always forced _FORTIFY_SOURCE=2. At first, I tried changing
> > their flags to undefine that macro first, but that did not work well.
> > This patch ensures that the package always respects our toolchain
> > settings, even in cases where we are not fortifying the build.
> >
> > This fixes a build failure where the mmc-utils package failed to build
> > with, e.g., BR2_FORTIFY_SOURCE_1:
> >
> >   <command-line>:0:0: error: "_FORTIFY_SOURCE" redefined [-Werror]
> >   <command-line>:0:0: note: this is the location of the previous definition
> >
> > Signed-off-by: Jan Kundr?t <jan.kundrat@cesnet.cz>
>
> Is this fixing a build issue reported by the autobuilders ? Or only
> with hardening options ? Your commit log is not very verbose in details
> about the configuration/condition under which the build failure happens.

(I haven't directly talked with Jan on this) It looks very similar to
other pkgs him and I looked at where flags were messy in the pkg and
now exposed when a build is done using hardening options, not a
autobuilder.

This isn't related to my hardening fixes patchset (ie those patches
won't fix this issue).  Instead, I'd refactor this patch to move the
AM_CFLAGS "-D_FORTIFY_SOURCE=2" into the CFLAGS ?= .  That should be
an upstream-able change and preserve their intent.

Matt

[Buildroot] [PATCH] mmc-utils: Rely on our own _FORTIFY_SOURCE

[Jan Kundrát]

On p?tek 10. srpna 2018 4:56:39 CEST, Matthew Weber wrote:
> This isn't related to my hardening fixes patchset (ie those patches
> won't fix this issue).  Instead, I'd refactor this patch to move the
> AM_CFLAGS "-D_FORTIFY_SOURCE=2" into the CFLAGS ?= .  That should be
> an upstream-able change and preserve their intent.

Hi Matthew,
doing just this change, as you suggested:

-AM_CFLAGS = -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2
-CFLAGS ?= -g -O2
+AM_CFLAGS = -D_FILE_OFFSET_BITS=64
+CFLAGS ?= -g -O2 -D_FORTIFY_SOURCE=2

...stil results in an error when building this manually out-of-box, outside 
of Buildroot, on a Gentoo Linux with hardened GCC which enforces 
_FORTIFY_SOURCE via compiler spec files. The good news is that it is now 
possible to override the CFLAGS via an env var when invoking make. I guess 
I'll send this upstream, then.

It still doesn't work out-of-box everyhere, but it is no longer a Buildroot 
bug (and it will work in Buildroot).

With kind regards,
Jan

[Buildroot] [PATCH] mmc-utils: Rely on our own _FORTIFY_SOURCE

[Matthew Weber]

Jan,

On Thu, Oct 25, 2018 at 1:03 PM Jan Kundr?t <jan.kundrat@cesnet.cz> wrote:
>
> On p?tek 10. srpna 2018 4:56:39 CEST, Matthew Weber wrote:
> > This isn't related to my hardening fixes patchset (ie those patches
> > won't fix this issue).  Instead, I'd refactor this patch to move the
> > AM_CFLAGS "-D_FORTIFY_SOURCE=2" into the CFLAGS ?= .  That should be
> > an upstream-able change and preserve their intent.
>
> Hi Matthew,
> doing just this change, as you suggested:
>
> -AM_CFLAGS = -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2
> -CFLAGS ?= -g -O2
> +AM_CFLAGS = -D_FILE_OFFSET_BITS=64
> +CFLAGS ?= -g -O2 -D_FORTIFY_SOURCE=2
>
> ...stil results in an error when building this manually out-of-box, outside
> of Buildroot, on a Gentoo Linux with hardened GCC which enforces
> _FORTIFY_SOURCE via compiler spec files. The good news is that it is now
> possible to override the CFLAGS via an env var when invoking make. I guess
> I'll send this upstream, then.
>
> It still doesn't work out-of-box everyhere, but it is no longer a Buildroot
> bug (and it will work in Buildroot).

Good to note.

Wasn't sure if you saw the merge of the updated hardening feature set.
It is now handled in the toolchain wrapper and more packages (without
changes directly in the package to allow flags to be set) should build
with the hardening flags.  The wrapper appends them to the compiler
args transparently and doesn't depend on ?=.

Matt