* [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error
@ 2018-12-13 21:32 bill.c.roberts
2018-12-13 21:32 ` [PATCH 2/2] Makefile: add -Wstrict-overflow=5 to CFLAGS bill.c.roberts
2018-12-14 13:43 ` [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error Stephen Smalley
0 siblings, 2 replies; 12+ messages in thread
From: bill.c.roberts @ 2018-12-13 21:32 UTC (permalink / raw)
To: selinux; +Cc: sds, William Roberts
From: William Roberts <william.c.roberts@intel.com>
Certain builds of gcc enable _FORTIFY_SOURCE which results in the error:
<command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined
<command-line>:0:0: note: this is the location of the previous definition
Correct this by undefining it first and redefining it. Also, the previous
command line option was using -Wp which passing the value *AS IS* to the
pre-processor rather than to the compiler driver. The C pre-processor has
an undocumented interface subject to change per man 1 gcc. Just use the
-D option as is.
Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
libselinux/src/Makefile | 2 +-
libselinux/utils/Makefile | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
index 977b5c8cfcca..ee55bd0dbff7 100644
--- a/libselinux/src/Makefile
+++ b/libselinux/src/Makefile
@@ -64,7 +64,7 @@ ifeq ($(COMPILER), gcc)
EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op -Wpacked-bitfield-compat -Wsync-nand \
-Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc -Wsuggest-attribute=const \
-Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wtrampolines -Wjump-misses-init \
- -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -Wp,-D_FORTIFY_SOURCE
+ -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
else
EXTRA_CFLAGS = -Wunused-command-line-argument
endif
diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
index d06ffd66893b..64ab877015c6 100644
--- a/libselinux/utils/Makefile
+++ b/libselinux/utils/Makefile
@@ -30,7 +30,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi
-Wformat-extra-args -Wformat-zero-length -Wformat=2 -Wmultichar \
-Woverflow -Wpointer-to-int-cast -Wpragmas \
-Wno-missing-field-initializers -Wno-sign-compare \
- -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \
+ -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 \
-fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \
-fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time \
-Werror -Wno-aggregate-return -Wno-redundant-decls \
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] Makefile: add -Wstrict-overflow=5 to CFLAGS
2018-12-13 21:32 [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error bill.c.roberts
@ 2018-12-13 21:32 ` bill.c.roberts
2018-12-14 13:43 ` [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error Stephen Smalley
1 sibling, 0 replies; 12+ messages in thread
From: bill.c.roberts @ 2018-12-13 21:32 UTC (permalink / raw)
To: selinux; +Cc: sds, William Roberts
From: William Roberts <william.c.roberts@intel.com>
Build with strict overflow checking enabled. If the compiler optimizes
code that could be removed due to undefined signed overflow, then the
compiler will issue a warning.
Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
libselinux/src/Makefile | 3 ++-
libselinux/utils/Makefile | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
index ee55bd0dbff7..a283ac9d90c2 100644
--- a/libselinux/src/Makefile
+++ b/libselinux/src/Makefile
@@ -64,7 +64,8 @@ ifeq ($(COMPILER), gcc)
EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op -Wpacked-bitfield-compat -Wsync-nand \
-Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc -Wsuggest-attribute=const \
-Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wtrampolines -Wjump-misses-init \
- -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
+ -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 \
+ -Wstrict-overflow=5
else
EXTRA_CFLAGS = -Wunused-command-line-argument
endif
diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
index 64ab877015c6..c2f59ea947cc 100644
--- a/libselinux/utils/Makefile
+++ b/libselinux/utils/Makefile
@@ -33,7 +33,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi
-Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 \
-fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \
-fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time \
- -Werror -Wno-aggregate-return -Wno-redundant-decls \
+ -Werror -Wno-aggregate-return -Wno-redundant-decls -Wstrict-overflow=5 \
$(EXTRA_CFLAGS)
LD_SONAME_FLAGS=-soname,$(LIBSO),-z,defs,-z,relro
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error
2018-12-13 21:32 [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error bill.c.roberts
2018-12-13 21:32 ` [PATCH 2/2] Makefile: add -Wstrict-overflow=5 to CFLAGS bill.c.roberts
@ 2018-12-14 13:43 ` Stephen Smalley
2018-12-14 14:34 ` Stephen Smalley
1 sibling, 1 reply; 12+ messages in thread
From: Stephen Smalley @ 2018-12-14 13:43 UTC (permalink / raw)
To: bill.c.roberts, selinux; +Cc: William Roberts, James Carter
On 12/13/18 4:32 PM, bill.c.roberts@gmail.com wrote:
> From: William Roberts <william.c.roberts@intel.com>
>
> Certain builds of gcc enable _FORTIFY_SOURCE which results in the error:
> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined
> <command-line>:0:0: note: this is the location of the previous definition
>
> Correct this by undefining it first and redefining it. Also, the previous
> command line option was using -Wp which passing the value *AS IS* to the
> pre-processor rather than to the compiler driver. The C pre-processor has
> an undocumented interface subject to change per man 1 gcc. Just use the
> -D option as is.
See commit ca07a2ad46be141dad90d885dd33a2ac31c6559a ("libselinux: avoid
redefining _FORTIFY_SOURCE") for why we don't specify a value for
_FORTIFY_SOURCE here. Not sure about the -Wp,-D vs -D rationale.
>
> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> ---
> libselinux/src/Makefile | 2 +-
> libselinux/utils/Makefile | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> index 977b5c8cfcca..ee55bd0dbff7 100644
> --- a/libselinux/src/Makefile
> +++ b/libselinux/src/Makefile
> @@ -64,7 +64,7 @@ ifeq ($(COMPILER), gcc)
> EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op -Wpacked-bitfield-compat -Wsync-nand \
> -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc -Wsuggest-attribute=const \
> -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure -Wtrampolines -Wjump-misses-init \
> - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -Wp,-D_FORTIFY_SOURCE
> + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
> else
> EXTRA_CFLAGS = -Wunused-command-line-argument
> endif
> diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
> index d06ffd66893b..64ab877015c6 100644
> --- a/libselinux/utils/Makefile
> +++ b/libselinux/utils/Makefile
> @@ -30,7 +30,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k -Wformat-security -Winit-self -Wmissi
> -Wformat-extra-args -Wformat-zero-length -Wformat=2 -Wmultichar \
> -Woverflow -Wpointer-to-int-cast -Wpragmas \
> -Wno-missing-field-initializers -Wno-sign-compare \
> - -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \
> + -Wno-format-nonliteral -Wframe-larger-than=$(MAX_STACK_SIZE) -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 \
> -fstack-protector-all --param=ssp-buffer-size=4 -fexceptions \
> -fasynchronous-unwind-tables -fdiagnostics-show-option -funit-at-a-time \
> -Werror -Wno-aggregate-return -Wno-redundant-decls \
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error
2018-12-14 13:43 ` [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error Stephen Smalley
@ 2018-12-14 14:34 ` Stephen Smalley
2018-12-14 16:02 ` William Roberts
0 siblings, 1 reply; 12+ messages in thread
From: Stephen Smalley @ 2018-12-14 14:34 UTC (permalink / raw)
To: bill.c.roberts, selinux; +Cc: William Roberts, James Carter
On 12/14/18 8:43 AM, Stephen Smalley wrote:
> On 12/13/18 4:32 PM, bill.c.roberts@gmail.com wrote:
>> From: William Roberts <william.c.roberts@intel.com>
>>
>> Certain builds of gcc enable _FORTIFY_SOURCE which results in the error:
>> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined
>> <command-line>:0:0: note: this is the location of the previous definition
>>
>> Correct this by undefining it first and redefining it. Also, the previous
>> command line option was using -Wp which passing the value *AS IS* to the
>> pre-processor rather than to the compiler driver. The C pre-processor has
>> an undocumented interface subject to change per man 1 gcc. Just use the
>> -D option as is.
>
> See commit ca07a2ad46be141dad90d885dd33a2ac31c6559a ("libselinux: avoid
> redefining _FORTIFY_SOURCE") for why we don't specify a value for
> _FORTIFY_SOURCE here. Not sure about the -Wp,-D vs -D rationale.
I guess the issue here is that we want to provide sane defaults for
building without breaking the build when others specify their own
definitions and without weakening those definitions. By undefining and
re-defining, it seems like we might weaken existing builds that were
specifying 2.
>
>>
>> Signed-off-by: William Roberts <william.c.roberts@intel.com>
>> ---
>> libselinux/src/Makefile | 2 +-
>> libselinux/utils/Makefile | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
>> index 977b5c8cfcca..ee55bd0dbff7 100644
>> --- a/libselinux/src/Makefile
>> +++ b/libselinux/src/Makefile
>> @@ -64,7 +64,7 @@ ifeq ($(COMPILER), gcc)
>> EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op
>> -Wpacked-bitfield-compat -Wsync-nand \
>> -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc
>> -Wsuggest-attribute=const \
>> -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure
>> -Wtrampolines -Wjump-misses-init \
>> - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
>> -Wp,-D_FORTIFY_SOURCE
>> + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
>> -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
>> else
>> EXTRA_CFLAGS = -Wunused-command-line-argument
>> endif
>> diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
>> index d06ffd66893b..64ab877015c6 100644
>> --- a/libselinux/utils/Makefile
>> +++ b/libselinux/utils/Makefile
>> @@ -30,7 +30,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k
>> -Wformat-security -Winit-self -Wmissi
>> -Wformat-extra-args -Wformat-zero-length -Wformat=2
>> -Wmultichar \
>> -Woverflow -Wpointer-to-int-cast -Wpragmas \
>> -Wno-missing-field-initializers -Wno-sign-compare \
>> - -Wno-format-nonliteral
>> -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \
>> + -Wno-format-nonliteral
>> -Wframe-larger-than=$(MAX_STACK_SIZE) -U_FORTIFY_SOURCE
>> -D_FORTIFY_SOURCE=1 \
>> -fstack-protector-all --param=ssp-buffer-size=4
>> -fexceptions \
>> -fasynchronous-unwind-tables -fdiagnostics-show-option
>> -funit-at-a-time \
>> -Werror -Wno-aggregate-return -Wno-redundant-decls \
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error
2018-12-14 14:34 ` Stephen Smalley
@ 2018-12-14 16:02 ` William Roberts
2018-12-18 16:03 ` William Roberts
0 siblings, 1 reply; 12+ messages in thread
From: William Roberts @ 2018-12-14 16:02 UTC (permalink / raw)
To: Stephen Smalley; +Cc: selinux, William Roberts, James Carter
On Fri, Dec 14, 2018 at 6:32 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> On 12/14/18 8:43 AM, Stephen Smalley wrote:
> > On 12/13/18 4:32 PM, bill.c.roberts@gmail.com wrote:
> >> From: William Roberts <william.c.roberts@intel.com>
> >>
> >> Certain builds of gcc enable _FORTIFY_SOURCE which results in the error:
> >> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined
> >> <command-line>:0:0: note: this is the location of the previous definition
> >>
> >> Correct this by undefining it first and redefining it. Also, the previous
> >> command line option was using -Wp which passing the value *AS IS* to the
> >> pre-processor rather than to the compiler driver. The C pre-processor has
> >> an undocumented interface subject to change per man 1 gcc. Just use the
> >> -D option as is.
> >
> > See commit ca07a2ad46be141dad90d885dd33a2ac31c6559a ("libselinux: avoid
> > redefining _FORTIFY_SOURCE") for why we don't specify a value for
> > _FORTIFY_SOURCE here. Not sure about the -Wp,-D vs -D rationale.
I'm not 100% convinced that the patch is the best solution or the commit message
is describing the problem correctly. I could also be understanding it
wrong here.
The man page is saying not to bypass the compiler driver via -Wp, and I don't
see a good reason for it either.
See my comments below, they feed back into this.
>
> I guess the issue here is that we want to provide sane defaults for
> building without breaking the build when others specify their own
> definitions and without weakening those definitions. By undefining and
> re-defining, it seems like we might weaken existing builds that were
> specifying 2.
We conditionally assign to CFLAGS via ?= operator. Thus, CFLAGS and the
corresponding addition of EXTRA_CFLAGS which contains the undef/def
is not appended. CFLAGS specified via the environment or as an argument to
make will cause this assignment not to occur (via ?= semantics) and whatever
they specify for CFLAGS is sent to CC.
Here is some sample output:
make CFLAGS='-D_FORTIFY_SOURCE=2'
cc -D_FORTIFY_SOURCE=2 -I../include -D_GNU_SOURCE -DNO_ANDROID_BACKEND
-fPIC -DSHARED -c -o stringrep.lo stringrep.c
With that said, *i think its safe* to bump it back to '-D_FORTIFY_SOURCE=2'
>
> >
> >>
> >> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> >> ---
> >> libselinux/src/Makefile | 2 +-
> >> libselinux/utils/Makefile | 2 +-
> >> 2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> >> index 977b5c8cfcca..ee55bd0dbff7 100644
> >> --- a/libselinux/src/Makefile
> >> +++ b/libselinux/src/Makefile
> >> @@ -64,7 +64,7 @@ ifeq ($(COMPILER), gcc)
> >> EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op
> >> -Wpacked-bitfield-compat -Wsync-nand \
> >> -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc
> >> -Wsuggest-attribute=const \
> >> -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure
> >> -Wtrampolines -Wjump-misses-init \
> >> - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> >> -Wp,-D_FORTIFY_SOURCE
> >> + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> >> -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
> >> else
> >> EXTRA_CFLAGS = -Wunused-command-line-argument
> >> endif
> >> diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
> >> index d06ffd66893b..64ab877015c6 100644
> >> --- a/libselinux/utils/Makefile
> >> +++ b/libselinux/utils/Makefile
> >> @@ -30,7 +30,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k
> >> -Wformat-security -Winit-self -Wmissi
> >> -Wformat-extra-args -Wformat-zero-length -Wformat=2
> >> -Wmultichar \
> >> -Woverflow -Wpointer-to-int-cast -Wpragmas \
> >> -Wno-missing-field-initializers -Wno-sign-compare \
> >> - -Wno-format-nonliteral
> >> -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \
> >> + -Wno-format-nonliteral
> >> -Wframe-larger-than=$(MAX_STACK_SIZE) -U_FORTIFY_SOURCE
> >> -D_FORTIFY_SOURCE=1 \
> >> -fstack-protector-all --param=ssp-buffer-size=4
> >> -fexceptions \
> >> -fasynchronous-unwind-tables -fdiagnostics-show-option
> >> -funit-at-a-time \
> >> -Werror -Wno-aggregate-return -Wno-redundant-decls \
> >>
> >
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error
2018-12-14 16:02 ` William Roberts
@ 2018-12-18 16:03 ` William Roberts
2018-12-18 19:02 ` William Roberts
2018-12-19 9:12 ` Patrick Steinhardt
0 siblings, 2 replies; 12+ messages in thread
From: William Roberts @ 2018-12-18 16:03 UTC (permalink / raw)
To: Stephen Smalley, ps; +Cc: selinux, William Roberts, James Carter
Patrick,
Hoping you could maybe weigh in on your choice for bypassing the
compiler driver with -Wp and not setting _FORTIFY_SOURCE to something
like 1 or 2?
I'm seeing this issue on Ubuntu 16.04.5:
<command-line>:0:0: error: "_FORTIFY_SOURCE" redefined [-Werror]
gcc version:
gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
My thought is to undef/redef _FORTIFY_SOURCE in CFLAGS and set the
level to 2. Setting CFLAGS via the env/make arg will override this
behavior
and use CFLAGS as is.
On Fri, Dec 14, 2018 at 8:02 AM William Roberts
<bill.c.roberts@gmail.com> wrote:
>
> On Fri, Dec 14, 2018 at 6:32 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> >
> > On 12/14/18 8:43 AM, Stephen Smalley wrote:
> > > On 12/13/18 4:32 PM, bill.c.roberts@gmail.com wrote:
> > >> From: William Roberts <william.c.roberts@intel.com>
> > >>
> > >> Certain builds of gcc enable _FORTIFY_SOURCE which results in the error:
> > >> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined
> > >> <command-line>:0:0: note: this is the location of the previous definition
> > >>
> > >> Correct this by undefining it first and redefining it. Also, the previous
> > >> command line option was using -Wp which passing the value *AS IS* to the
> > >> pre-processor rather than to the compiler driver. The C pre-processor has
> > >> an undocumented interface subject to change per man 1 gcc. Just use the
> > >> -D option as is.
> > >
> > > See commit ca07a2ad46be141dad90d885dd33a2ac31c6559a ("libselinux: avoid
> > > redefining _FORTIFY_SOURCE") for why we don't specify a value for
> > > _FORTIFY_SOURCE here. Not sure about the -Wp,-D vs -D rationale.
>
> I'm not 100% convinced that the patch is the best solution or the commit message
> is describing the problem correctly. I could also be understanding it
> wrong here.
> The man page is saying not to bypass the compiler driver via -Wp, and I don't
> see a good reason for it either.
>
> See my comments below, they feed back into this.
>
> >
> > I guess the issue here is that we want to provide sane defaults for
> > building without breaking the build when others specify their own
> > definitions and without weakening those definitions. By undefining and
> > re-defining, it seems like we might weaken existing builds that were
> > specifying 2.
>
> We conditionally assign to CFLAGS via ?= operator. Thus, CFLAGS and the
> corresponding addition of EXTRA_CFLAGS which contains the undef/def
> is not appended. CFLAGS specified via the environment or as an argument to
> make will cause this assignment not to occur (via ?= semantics) and whatever
> they specify for CFLAGS is sent to CC.
>
> Here is some sample output:
> make CFLAGS='-D_FORTIFY_SOURCE=2'
> cc -D_FORTIFY_SOURCE=2 -I../include -D_GNU_SOURCE -DNO_ANDROID_BACKEND
> -fPIC -DSHARED -c -o stringrep.lo stringrep.c
>
> With that said, *i think its safe* to bump it back to '-D_FORTIFY_SOURCE=2'
>
> >
> > >
> > >>
> > >> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> > >> ---
> > >> libselinux/src/Makefile | 2 +-
> > >> libselinux/utils/Makefile | 2 +-
> > >> 2 files changed, 2 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> > >> index 977b5c8cfcca..ee55bd0dbff7 100644
> > >> --- a/libselinux/src/Makefile
> > >> +++ b/libselinux/src/Makefile
> > >> @@ -64,7 +64,7 @@ ifeq ($(COMPILER), gcc)
> > >> EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op
> > >> -Wpacked-bitfield-compat -Wsync-nand \
> > >> -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc
> > >> -Wsuggest-attribute=const \
> > >> -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure
> > >> -Wtrampolines -Wjump-misses-init \
> > >> - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> > >> -Wp,-D_FORTIFY_SOURCE
> > >> + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> > >> -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
> > >> else
> > >> EXTRA_CFLAGS = -Wunused-command-line-argument
> > >> endif
> > >> diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
> > >> index d06ffd66893b..64ab877015c6 100644
> > >> --- a/libselinux/utils/Makefile
> > >> +++ b/libselinux/utils/Makefile
> > >> @@ -30,7 +30,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k
> > >> -Wformat-security -Winit-self -Wmissi
> > >> -Wformat-extra-args -Wformat-zero-length -Wformat=2
> > >> -Wmultichar \
> > >> -Woverflow -Wpointer-to-int-cast -Wpragmas \
> > >> -Wno-missing-field-initializers -Wno-sign-compare \
> > >> - -Wno-format-nonliteral
> > >> -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \
> > >> + -Wno-format-nonliteral
> > >> -Wframe-larger-than=$(MAX_STACK_SIZE) -U_FORTIFY_SOURCE
> > >> -D_FORTIFY_SOURCE=1 \
> > >> -fstack-protector-all --param=ssp-buffer-size=4
> > >> -fexceptions \
> > >> -fasynchronous-unwind-tables -fdiagnostics-show-option
> > >> -funit-at-a-time \
> > >> -Werror -Wno-aggregate-return -Wno-redundant-decls \
> > >>
> > >
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error
2018-12-18 16:03 ` William Roberts
@ 2018-12-18 19:02 ` William Roberts
2018-12-19 6:15 ` Jason Zaman
2018-12-19 9:12 ` Patrick Steinhardt
1 sibling, 1 reply; 12+ messages in thread
From: William Roberts @ 2018-12-18 19:02 UTC (permalink / raw)
To: Stephen Smalley, ps, berrange, Eric Paris, Eric Paris
Cc: selinux, William Roberts, James Carter
+ Eric and Dan, as Stephen pointed out to me, they introduced the -Wp
portion in commit 41649ca78
On Tue, Dec 18, 2018 at 8:03 AM William Roberts
<bill.c.roberts@gmail.com> wrote:
>
> Patrick,
>
> Hoping you could maybe weigh in on your choice for bypassing the
> compiler driver with -Wp and not setting _FORTIFY_SOURCE to something
> like 1 or 2?
>
> I'm seeing this issue on Ubuntu 16.04.5:
> <command-line>:0:0: error: "_FORTIFY_SOURCE" redefined [-Werror]
>
> gcc version:
> gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
>
> My thought is to undef/redef _FORTIFY_SOURCE in CFLAGS and set the
> level to 2. Setting CFLAGS via the env/make arg will override this
> behavior
> and use CFLAGS as is.
>
> On Fri, Dec 14, 2018 at 8:02 AM William Roberts
> <bill.c.roberts@gmail.com> wrote:
> >
> > On Fri, Dec 14, 2018 at 6:32 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > >
> > > On 12/14/18 8:43 AM, Stephen Smalley wrote:
> > > > On 12/13/18 4:32 PM, bill.c.roberts@gmail.com wrote:
> > > >> From: William Roberts <william.c.roberts@intel.com>
> > > >>
> > > >> Certain builds of gcc enable _FORTIFY_SOURCE which results in the error:
> > > >> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined
> > > >> <command-line>:0:0: note: this is the location of the previous definition
> > > >>
> > > >> Correct this by undefining it first and redefining it. Also, the previous
> > > >> command line option was using -Wp which passing the value *AS IS* to the
> > > >> pre-processor rather than to the compiler driver. The C pre-processor has
> > > >> an undocumented interface subject to change per man 1 gcc. Just use the
> > > >> -D option as is.
> > > >
> > > > See commit ca07a2ad46be141dad90d885dd33a2ac31c6559a ("libselinux: avoid
> > > > redefining _FORTIFY_SOURCE") for why we don't specify a value for
> > > > _FORTIFY_SOURCE here. Not sure about the -Wp,-D vs -D rationale.
> >
> > I'm not 100% convinced that the patch is the best solution or the commit message
> > is describing the problem correctly. I could also be understanding it
> > wrong here.
> > The man page is saying not to bypass the compiler driver via -Wp, and I don't
> > see a good reason for it either.
> >
> > See my comments below, they feed back into this.
> >
> > >
> > > I guess the issue here is that we want to provide sane defaults for
> > > building without breaking the build when others specify their own
> > > definitions and without weakening those definitions. By undefining and
> > > re-defining, it seems like we might weaken existing builds that were
> > > specifying 2.
> >
> > We conditionally assign to CFLAGS via ?= operator. Thus, CFLAGS and the
> > corresponding addition of EXTRA_CFLAGS which contains the undef/def
> > is not appended. CFLAGS specified via the environment or as an argument to
> > make will cause this assignment not to occur (via ?= semantics) and whatever
> > they specify for CFLAGS is sent to CC.
> >
> > Here is some sample output:
> > make CFLAGS='-D_FORTIFY_SOURCE=2'
> > cc -D_FORTIFY_SOURCE=2 -I../include -D_GNU_SOURCE -DNO_ANDROID_BACKEND
> > -fPIC -DSHARED -c -o stringrep.lo stringrep.c
> >
> > With that said, *i think its safe* to bump it back to '-D_FORTIFY_SOURCE=2'
> >
> > >
> > > >
> > > >>
> > > >> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> > > >> ---
> > > >> libselinux/src/Makefile | 2 +-
> > > >> libselinux/utils/Makefile | 2 +-
> > > >> 2 files changed, 2 insertions(+), 2 deletions(-)
> > > >>
> > > >> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> > > >> index 977b5c8cfcca..ee55bd0dbff7 100644
> > > >> --- a/libselinux/src/Makefile
> > > >> +++ b/libselinux/src/Makefile
> > > >> @@ -64,7 +64,7 @@ ifeq ($(COMPILER), gcc)
> > > >> EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op
> > > >> -Wpacked-bitfield-compat -Wsync-nand \
> > > >> -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc
> > > >> -Wsuggest-attribute=const \
> > > >> -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure
> > > >> -Wtrampolines -Wjump-misses-init \
> > > >> - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> > > >> -Wp,-D_FORTIFY_SOURCE
> > > >> + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> > > >> -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
> > > >> else
> > > >> EXTRA_CFLAGS = -Wunused-command-line-argument
> > > >> endif
> > > >> diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
> > > >> index d06ffd66893b..64ab877015c6 100644
> > > >> --- a/libselinux/utils/Makefile
> > > >> +++ b/libselinux/utils/Makefile
> > > >> @@ -30,7 +30,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k
> > > >> -Wformat-security -Winit-self -Wmissi
> > > >> -Wformat-extra-args -Wformat-zero-length -Wformat=2
> > > >> -Wmultichar \
> > > >> -Woverflow -Wpointer-to-int-cast -Wpragmas \
> > > >> -Wno-missing-field-initializers -Wno-sign-compare \
> > > >> - -Wno-format-nonliteral
> > > >> -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \
> > > >> + -Wno-format-nonliteral
> > > >> -Wframe-larger-than=$(MAX_STACK_SIZE) -U_FORTIFY_SOURCE
> > > >> -D_FORTIFY_SOURCE=1 \
> > > >> -fstack-protector-all --param=ssp-buffer-size=4
> > > >> -fexceptions \
> > > >> -fasynchronous-unwind-tables -fdiagnostics-show-option
> > > >> -funit-at-a-time \
> > > >> -Werror -Wno-aggregate-return -Wno-redundant-decls \
> > > >>
> > > >
> > >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error
2018-12-18 19:02 ` William Roberts
@ 2018-12-19 6:15 ` Jason Zaman
0 siblings, 0 replies; 12+ messages in thread
From: Jason Zaman @ 2018-12-19 6:15 UTC (permalink / raw)
To: William Roberts
Cc: Stephen Smalley, ps, berrange, Eric Paris, Eric Paris, selinux,
William Roberts, James Carter
On Tue, Dec 18, 2018 at 11:02:54AM -0800, William Roberts wrote:
> + Eric and Dan, as Stephen pointed out to me, they introduced the -Wp
> portion in commit 41649ca78
>
> On Tue, Dec 18, 2018 at 8:03 AM William Roberts
> <bill.c.roberts@gmail.com> wrote:
> >
> > Patrick,
> >
> > Hoping you could maybe weigh in on your choice for bypassing the
> > compiler driver with -Wp and not setting _FORTIFY_SOURCE to something
> > like 1 or 2?
> >
> > I'm seeing this issue on Ubuntu 16.04.5:
> > <command-line>:0:0: error: "_FORTIFY_SOURCE" redefined [-Werror]
> >
> > gcc version:
> > gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
> >
> > My thought is to undef/redef _FORTIFY_SOURCE in CFLAGS and set the
> > level to 2. Setting CFLAGS via the env/make arg will override this
> > behavior
> > and use CFLAGS as is.
Just wanted to say that in Gentoo Hardened our toolchain also already
has _FORTIFY_SOURCE set to 2. If anything we should set it to 2 not 1.
and if setting it, we gotta add the undefine first otherwise we get
this:
$ gcc -Werror -O2 -D_FORTIFY_SOURCE=2 main.c
<command-line>:0:0: error: "_FORTIFY_SOURCE" redefined [-Werror]
<built-in>: note: this is the location of the previous definition
<command-line>:0:17: error: expected identifier or ‘(’ before numeric constant
main.c:3:1: note: in expansion of macro ‘_FORTIFY_SOURCE’
_FORTIFY_SOURCE
^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Its not really a big deal since basically everyone sets their own CFLAGS
but =2 is safer so we should have it as the default.
-- Jason
> >
> > On Fri, Dec 14, 2018 at 8:02 AM William Roberts
> > <bill.c.roberts@gmail.com> wrote:
> > >
> > > On Fri, Dec 14, 2018 at 6:32 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > > >
> > > > On 12/14/18 8:43 AM, Stephen Smalley wrote:
> > > > > On 12/13/18 4:32 PM, bill.c.roberts@gmail.com wrote:
> > > > >> From: William Roberts <william.c.roberts@intel.com>
> > > > >>
> > > > >> Certain builds of gcc enable _FORTIFY_SOURCE which results in the error:
> > > > >> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined
> > > > >> <command-line>:0:0: note: this is the location of the previous definition
> > > > >>
> > > > >> Correct this by undefining it first and redefining it. Also, the previous
> > > > >> command line option was using -Wp which passing the value *AS IS* to the
> > > > >> pre-processor rather than to the compiler driver. The C pre-processor has
> > > > >> an undocumented interface subject to change per man 1 gcc. Just use the
> > > > >> -D option as is.
> > > > >
> > > > > See commit ca07a2ad46be141dad90d885dd33a2ac31c6559a ("libselinux: avoid
> > > > > redefining _FORTIFY_SOURCE") for why we don't specify a value for
> > > > > _FORTIFY_SOURCE here. Not sure about the -Wp,-D vs -D rationale.
> > >
> > > I'm not 100% convinced that the patch is the best solution or the commit message
> > > is describing the problem correctly. I could also be understanding it
> > > wrong here.
> > > The man page is saying not to bypass the compiler driver via -Wp, and I don't
> > > see a good reason for it either.
> > >
> > > See my comments below, they feed back into this.
> > >
> > > >
> > > > I guess the issue here is that we want to provide sane defaults for
> > > > building without breaking the build when others specify their own
> > > > definitions and without weakening those definitions. By undefining and
> > > > re-defining, it seems like we might weaken existing builds that were
> > > > specifying 2.
> > >
> > > We conditionally assign to CFLAGS via ?= operator. Thus, CFLAGS and the
> > > corresponding addition of EXTRA_CFLAGS which contains the undef/def
> > > is not appended. CFLAGS specified via the environment or as an argument to
> > > make will cause this assignment not to occur (via ?= semantics) and whatever
> > > they specify for CFLAGS is sent to CC.
> > >
> > > Here is some sample output:
> > > make CFLAGS='-D_FORTIFY_SOURCE=2'
> > > cc -D_FORTIFY_SOURCE=2 -I../include -D_GNU_SOURCE -DNO_ANDROID_BACKEND
> > > -fPIC -DSHARED -c -o stringrep.lo stringrep.c
> > >
> > > With that said, *i think its safe* to bump it back to '-D_FORTIFY_SOURCE=2'
> > >
> > > >
> > > > >
> > > > >>
> > > > >> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> > > > >> ---
> > > > >> libselinux/src/Makefile | 2 +-
> > > > >> libselinux/utils/Makefile | 2 +-
> > > > >> 2 files changed, 2 insertions(+), 2 deletions(-)
> > > > >>
> > > > >> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> > > > >> index 977b5c8cfcca..ee55bd0dbff7 100644
> > > > >> --- a/libselinux/src/Makefile
> > > > >> +++ b/libselinux/src/Makefile
> > > > >> @@ -64,7 +64,7 @@ ifeq ($(COMPILER), gcc)
> > > > >> EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op
> > > > >> -Wpacked-bitfield-compat -Wsync-nand \
> > > > >> -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc
> > > > >> -Wsuggest-attribute=const \
> > > > >> -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure
> > > > >> -Wtrampolines -Wjump-misses-init \
> > > > >> - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> > > > >> -Wp,-D_FORTIFY_SOURCE
> > > > >> + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> > > > >> -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
> > > > >> else
> > > > >> EXTRA_CFLAGS = -Wunused-command-line-argument
> > > > >> endif
> > > > >> diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
> > > > >> index d06ffd66893b..64ab877015c6 100644
> > > > >> --- a/libselinux/utils/Makefile
> > > > >> +++ b/libselinux/utils/Makefile
> > > > >> @@ -30,7 +30,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k
> > > > >> -Wformat-security -Winit-self -Wmissi
> > > > >> -Wformat-extra-args -Wformat-zero-length -Wformat=2
> > > > >> -Wmultichar \
> > > > >> -Woverflow -Wpointer-to-int-cast -Wpragmas \
> > > > >> -Wno-missing-field-initializers -Wno-sign-compare \
> > > > >> - -Wno-format-nonliteral
> > > > >> -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \
> > > > >> + -Wno-format-nonliteral
> > > > >> -Wframe-larger-than=$(MAX_STACK_SIZE) -U_FORTIFY_SOURCE
> > > > >> -D_FORTIFY_SOURCE=1 \
> > > > >> -fstack-protector-all --param=ssp-buffer-size=4
> > > > >> -fexceptions \
> > > > >> -fasynchronous-unwind-tables -fdiagnostics-show-option
> > > > >> -funit-at-a-time \
> > > > >> -Werror -Wno-aggregate-return -Wno-redundant-decls \
> > > > >>
> > > > >
> > > >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error
2018-12-18 16:03 ` William Roberts
2018-12-18 19:02 ` William Roberts
@ 2018-12-19 9:12 ` Patrick Steinhardt
2018-12-19 15:42 ` William Roberts
1 sibling, 1 reply; 12+ messages in thread
From: Patrick Steinhardt @ 2018-12-19 9:12 UTC (permalink / raw)
To: William Roberts; +Cc: Stephen Smalley, selinux, William Roberts, James Carter
[-- Attachment #1: Type: text/plain, Size: 6309 bytes --]
Hi,
On Tue, Dec 18, 2018 at 08:03:54AM -0800, William Roberts wrote:
> Patrick,
>
> Hoping you could maybe weigh in on your choice for bypassing the
> compiler driver with -Wp and not setting _FORTIFY_SOURCE to something
> like 1 or 2?
>
> I'm seeing this issue on Ubuntu 16.04.5:
> <command-line>:0:0: error: "_FORTIFY_SOURCE" redefined [-Werror]
>
> gcc version:
> gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
>
> My thought is to undef/redef _FORTIFY_SOURCE in CFLAGS and set the
> level to 2. Setting CFLAGS via the env/make arg will override this
> behavior
> and use CFLAGS as is.
I used "-Wp" simply because it was existing previously, so I just
stuck to what was there already. The original issue I had was
that Gentoo Hardened, as Jason notes, already defines
_FORTIFY_SOURCE as part of the compiler spec. Due to that, I was
seeing a lot of warnings.
So I set the flag to a simple define without setting a specific
value, which _seemed_ to let the issue go away. But going back to
the initial issue, this didn't seem to have solved it correctly.
Dunno what I've been doing back then to not see the warnings
after my change anymore, but I noticed that they have resurface
recently.
So I guess the real fix would be to redefine the value by first
undef'ing it and then redefining it to the desired value. And I
do agree that in that case, we should simply revert to
_FORTIFY_SOURCE=2.
Patrick
> On Fri, Dec 14, 2018 at 8:02 AM William Roberts
> <bill.c.roberts@gmail.com> wrote:
> >
> > On Fri, Dec 14, 2018 at 6:32 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > >
> > > On 12/14/18 8:43 AM, Stephen Smalley wrote:
> > > > On 12/13/18 4:32 PM, bill.c.roberts@gmail.com wrote:
> > > >> From: William Roberts <william.c.roberts@intel.com>
> > > >>
> > > >> Certain builds of gcc enable _FORTIFY_SOURCE which results in the error:
> > > >> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined
> > > >> <command-line>:0:0: note: this is the location of the previous definition
> > > >>
> > > >> Correct this by undefining it first and redefining it. Also, the previous
> > > >> command line option was using -Wp which passing the value *AS IS* to the
> > > >> pre-processor rather than to the compiler driver. The C pre-processor has
> > > >> an undocumented interface subject to change per man 1 gcc. Just use the
> > > >> -D option as is.
> > > >
> > > > See commit ca07a2ad46be141dad90d885dd33a2ac31c6559a ("libselinux: avoid
> > > > redefining _FORTIFY_SOURCE") for why we don't specify a value for
> > > > _FORTIFY_SOURCE here. Not sure about the -Wp,-D vs -D rationale.
> >
> > I'm not 100% convinced that the patch is the best solution or the commit message
> > is describing the problem correctly. I could also be understanding it
> > wrong here.
> > The man page is saying not to bypass the compiler driver via -Wp, and I don't
> > see a good reason for it either.
> >
> > See my comments below, they feed back into this.
> >
> > >
> > > I guess the issue here is that we want to provide sane defaults for
> > > building without breaking the build when others specify their own
> > > definitions and without weakening those definitions. By undefining and
> > > re-defining, it seems like we might weaken existing builds that were
> > > specifying 2.
> >
> > We conditionally assign to CFLAGS via ?= operator. Thus, CFLAGS and the
> > corresponding addition of EXTRA_CFLAGS which contains the undef/def
> > is not appended. CFLAGS specified via the environment or as an argument to
> > make will cause this assignment not to occur (via ?= semantics) and whatever
> > they specify for CFLAGS is sent to CC.
> >
> > Here is some sample output:
> > make CFLAGS='-D_FORTIFY_SOURCE=2'
> > cc -D_FORTIFY_SOURCE=2 -I../include -D_GNU_SOURCE -DNO_ANDROID_BACKEND
> > -fPIC -DSHARED -c -o stringrep.lo stringrep.c
> >
> > With that said, *i think its safe* to bump it back to '-D_FORTIFY_SOURCE=2'
> >
> > >
> > > >
> > > >>
> > > >> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> > > >> ---
> > > >> libselinux/src/Makefile | 2 +-
> > > >> libselinux/utils/Makefile | 2 +-
> > > >> 2 files changed, 2 insertions(+), 2 deletions(-)
> > > >>
> > > >> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> > > >> index 977b5c8cfcca..ee55bd0dbff7 100644
> > > >> --- a/libselinux/src/Makefile
> > > >> +++ b/libselinux/src/Makefile
> > > >> @@ -64,7 +64,7 @@ ifeq ($(COMPILER), gcc)
> > > >> EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op
> > > >> -Wpacked-bitfield-compat -Wsync-nand \
> > > >> -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc
> > > >> -Wsuggest-attribute=const \
> > > >> -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure
> > > >> -Wtrampolines -Wjump-misses-init \
> > > >> - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> > > >> -Wp,-D_FORTIFY_SOURCE
> > > >> + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> > > >> -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
> > > >> else
> > > >> EXTRA_CFLAGS = -Wunused-command-line-argument
> > > >> endif
> > > >> diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
> > > >> index d06ffd66893b..64ab877015c6 100644
> > > >> --- a/libselinux/utils/Makefile
> > > >> +++ b/libselinux/utils/Makefile
> > > >> @@ -30,7 +30,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k
> > > >> -Wformat-security -Winit-self -Wmissi
> > > >> -Wformat-extra-args -Wformat-zero-length -Wformat=2
> > > >> -Wmultichar \
> > > >> -Woverflow -Wpointer-to-int-cast -Wpragmas \
> > > >> -Wno-missing-field-initializers -Wno-sign-compare \
> > > >> - -Wno-format-nonliteral
> > > >> -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \
> > > >> + -Wno-format-nonliteral
> > > >> -Wframe-larger-than=$(MAX_STACK_SIZE) -U_FORTIFY_SOURCE
> > > >> -D_FORTIFY_SOURCE=1 \
> > > >> -fstack-protector-all --param=ssp-buffer-size=4
> > > >> -fexceptions \
> > > >> -fasynchronous-unwind-tables -fdiagnostics-show-option
> > > >> -funit-at-a-time \
> > > >> -Werror -Wno-aggregate-return -Wno-redundant-decls \
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error
2018-12-19 9:12 ` Patrick Steinhardt
@ 2018-12-19 15:42 ` William Roberts
2018-12-19 15:46 ` Stephen Smalley
0 siblings, 1 reply; 12+ messages in thread
From: William Roberts @ 2018-12-19 15:42 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: Stephen Smalley, selinux, William Roberts, James Carter
On Wed, Dec 19, 2018 at 1:12 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> Hi,
>
> On Tue, Dec 18, 2018 at 08:03:54AM -0800, William Roberts wrote:
> > Patrick,
> >
> > Hoping you could maybe weigh in on your choice for bypassing the
> > compiler driver with -Wp and not setting _FORTIFY_SOURCE to something
> > like 1 or 2?
> >
> > I'm seeing this issue on Ubuntu 16.04.5:
> > <command-line>:0:0: error: "_FORTIFY_SOURCE" redefined [-Werror]
> >
> > gcc version:
> > gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
> >
> > My thought is to undef/redef _FORTIFY_SOURCE in CFLAGS and set the
> > level to 2. Setting CFLAGS via the env/make arg will override this
> > behavior
> > and use CFLAGS as is.
>
> I used "-Wp" simply because it was existing previously, so I just
> stuck to what was there already. The original issue I had was
> that Gentoo Hardened, as Jason notes, already defines
> _FORTIFY_SOURCE as part of the compiler spec. Due to that, I was
> seeing a lot of warnings.
>
> So I set the flag to a simple define without setting a specific
> value, which _seemed_ to let the issue go away. But going back to
> the initial issue, this didn't seem to have solved it correctly.
> Dunno what I've been doing back then to not see the warnings
> after my change anymore, but I noticed that they have resurface
> recently.
>
> So I guess the real fix would be to redefine the value by first
> undef'ing it and then redefining it to the desired value. And I
> do agree that in that case, we should simply revert to
> _FORTIFY_SOURCE=2.
>
> Patrick
>
> > On Fri, Dec 14, 2018 at 8:02 AM William Roberts
> > <bill.c.roberts@gmail.com> wrote:
> > >
> > > On Fri, Dec 14, 2018 at 6:32 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> > > >
> > > > On 12/14/18 8:43 AM, Stephen Smalley wrote:
> > > > > On 12/13/18 4:32 PM, bill.c.roberts@gmail.com wrote:
> > > > >> From: William Roberts <william.c.roberts@intel.com>
> > > > >>
> > > > >> Certain builds of gcc enable _FORTIFY_SOURCE which results in the error:
> > > > >> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined
> > > > >> <command-line>:0:0: note: this is the location of the previous definition
> > > > >>
> > > > >> Correct this by undefining it first and redefining it. Also, the previous
> > > > >> command line option was using -Wp which passing the value *AS IS* to the
> > > > >> pre-processor rather than to the compiler driver. The C pre-processor has
> > > > >> an undocumented interface subject to change per man 1 gcc. Just use the
> > > > >> -D option as is.
> > > > >
> > > > > See commit ca07a2ad46be141dad90d885dd33a2ac31c6559a ("libselinux: avoid
> > > > > redefining _FORTIFY_SOURCE") for why we don't specify a value for
> > > > > _FORTIFY_SOURCE here. Not sure about the -Wp,-D vs -D rationale.
> > >
> > > I'm not 100% convinced that the patch is the best solution or the commit message
> > > is describing the problem correctly. I could also be understanding it
> > > wrong here.
> > > The man page is saying not to bypass the compiler driver via -Wp, and I don't
> > > see a good reason for it either.
> > >
> > > See my comments below, they feed back into this.
> > >
> > > >
> > > > I guess the issue here is that we want to provide sane defaults for
> > > > building without breaking the build when others specify their own
> > > > definitions and without weakening those definitions. By undefining and
> > > > re-defining, it seems like we might weaken existing builds that were
> > > > specifying 2.
> > >
> > > We conditionally assign to CFLAGS via ?= operator. Thus, CFLAGS and the
> > > corresponding addition of EXTRA_CFLAGS which contains the undef/def
> > > is not appended. CFLAGS specified via the environment or as an argument to
> > > make will cause this assignment not to occur (via ?= semantics) and whatever
> > > they specify for CFLAGS is sent to CC.
> > >
> > > Here is some sample output:
> > > make CFLAGS='-D_FORTIFY_SOURCE=2'
> > > cc -D_FORTIFY_SOURCE=2 -I../include -D_GNU_SOURCE -DNO_ANDROID_BACKEND
> > > -fPIC -DSHARED -c -o stringrep.lo stringrep.c
> > >
> > > With that said, *i think its safe* to bump it back to '-D_FORTIFY_SOURCE=2'
I agree that 2 is the better value. I'll re-roll these to use 2. No
one seems to be complaining
on using the -D/-U options.
> > >
> > > >
> > > > >
> > > > >>
> > > > >> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> > > > >> ---
> > > > >> libselinux/src/Makefile | 2 +-
> > > > >> libselinux/utils/Makefile | 2 +-
> > > > >> 2 files changed, 2 insertions(+), 2 deletions(-)
> > > > >>
> > > > >> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> > > > >> index 977b5c8cfcca..ee55bd0dbff7 100644
> > > > >> --- a/libselinux/src/Makefile
> > > > >> +++ b/libselinux/src/Makefile
> > > > >> @@ -64,7 +64,7 @@ ifeq ($(COMPILER), gcc)
> > > > >> EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op
> > > > >> -Wpacked-bitfield-compat -Wsync-nand \
> > > > >> -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc
> > > > >> -Wsuggest-attribute=const \
> > > > >> -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure
> > > > >> -Wtrampolines -Wjump-misses-init \
> > > > >> - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> > > > >> -Wp,-D_FORTIFY_SOURCE
> > > > >> + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> > > > >> -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
> > > > >> else
> > > > >> EXTRA_CFLAGS = -Wunused-command-line-argument
> > > > >> endif
> > > > >> diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
> > > > >> index d06ffd66893b..64ab877015c6 100644
> > > > >> --- a/libselinux/utils/Makefile
> > > > >> +++ b/libselinux/utils/Makefile
> > > > >> @@ -30,7 +30,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k
> > > > >> -Wformat-security -Winit-self -Wmissi
> > > > >> -Wformat-extra-args -Wformat-zero-length -Wformat=2
> > > > >> -Wmultichar \
> > > > >> -Woverflow -Wpointer-to-int-cast -Wpragmas \
> > > > >> -Wno-missing-field-initializers -Wno-sign-compare \
> > > > >> - -Wno-format-nonliteral
> > > > >> -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \
> > > > >> + -Wno-format-nonliteral
> > > > >> -Wframe-larger-than=$(MAX_STACK_SIZE) -U_FORTIFY_SOURCE
> > > > >> -D_FORTIFY_SOURCE=1 \
> > > > >> -fstack-protector-all --param=ssp-buffer-size=4
> > > > >> -fexceptions \
> > > > >> -fasynchronous-unwind-tables -fdiagnostics-show-option
> > > > >> -funit-at-a-time \
> > > > >> -Werror -Wno-aggregate-return -Wno-redundant-decls \
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error
2018-12-19 15:42 ` William Roberts
@ 2018-12-19 15:46 ` Stephen Smalley
2018-12-19 15:48 ` William Roberts
0 siblings, 1 reply; 12+ messages in thread
From: Stephen Smalley @ 2018-12-19 15:46 UTC (permalink / raw)
To: William Roberts, Patrick Steinhardt
Cc: selinux, William Roberts, James Carter
On 12/19/18 10:42 AM, William Roberts wrote:
> On Wed, Dec 19, 2018 at 1:12 AM Patrick Steinhardt <ps@pks.im> wrote:
>>
>> Hi,
>>
>> On Tue, Dec 18, 2018 at 08:03:54AM -0800, William Roberts wrote:
>>> Patrick,
>>>
>>> Hoping you could maybe weigh in on your choice for bypassing the
>>> compiler driver with -Wp and not setting _FORTIFY_SOURCE to something
>>> like 1 or 2?
>>>
>>> I'm seeing this issue on Ubuntu 16.04.5:
>>> <command-line>:0:0: error: "_FORTIFY_SOURCE" redefined [-Werror]
>>>
>>> gcc version:
>>> gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
>>>
>>> My thought is to undef/redef _FORTIFY_SOURCE in CFLAGS and set the
>>> level to 2. Setting CFLAGS via the env/make arg will override this
>>> behavior
>>> and use CFLAGS as is.
>>
>> I used "-Wp" simply because it was existing previously, so I just
>> stuck to what was there already. The original issue I had was
>> that Gentoo Hardened, as Jason notes, already defines
>> _FORTIFY_SOURCE as part of the compiler spec. Due to that, I was
>> seeing a lot of warnings.
>>
>> So I set the flag to a simple define without setting a specific
>> value, which _seemed_ to let the issue go away. But going back to
>> the initial issue, this didn't seem to have solved it correctly.
>> Dunno what I've been doing back then to not see the warnings
>> after my change anymore, but I noticed that they have resurface
>> recently.
>>
>> So I guess the real fix would be to redefine the value by first
>> undef'ing it and then redefining it to the desired value. And I
>> do agree that in that case, we should simply revert to
>> _FORTIFY_SOURCE=2.
>>
>> Patrick
>>
>>> On Fri, Dec 14, 2018 at 8:02 AM William Roberts
>>> <bill.c.roberts@gmail.com> wrote:
>>>>
>>>> On Fri, Dec 14, 2018 at 6:32 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>>>>
>>>>> On 12/14/18 8:43 AM, Stephen Smalley wrote:
>>>>>> On 12/13/18 4:32 PM, bill.c.roberts@gmail.com wrote:
>>>>>>> From: William Roberts <william.c.roberts@intel.com>
>>>>>>>
>>>>>>> Certain builds of gcc enable _FORTIFY_SOURCE which results in the error:
>>>>>>> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined
>>>>>>> <command-line>:0:0: note: this is the location of the previous definition
>>>>>>>
>>>>>>> Correct this by undefining it first and redefining it. Also, the previous
>>>>>>> command line option was using -Wp which passing the value *AS IS* to the
>>>>>>> pre-processor rather than to the compiler driver. The C pre-processor has
>>>>>>> an undocumented interface subject to change per man 1 gcc. Just use the
>>>>>>> -D option as is.
>>>>>>
>>>>>> See commit ca07a2ad46be141dad90d885dd33a2ac31c6559a ("libselinux: avoid
>>>>>> redefining _FORTIFY_SOURCE") for why we don't specify a value for
>>>>>> _FORTIFY_SOURCE here. Not sure about the -Wp,-D vs -D rationale.
>>>>
>>>> I'm not 100% convinced that the patch is the best solution or the commit message
>>>> is describing the problem correctly. I could also be understanding it
>>>> wrong here.
>>>> The man page is saying not to bypass the compiler driver via -Wp, and I don't
>>>> see a good reason for it either.
>>>>
>>>> See my comments below, they feed back into this.
>>>>
>>>>>
>>>>> I guess the issue here is that we want to provide sane defaults for
>>>>> building without breaking the build when others specify their own
>>>>> definitions and without weakening those definitions. By undefining and
>>>>> re-defining, it seems like we might weaken existing builds that were
>>>>> specifying 2.
>>>>
>>>> We conditionally assign to CFLAGS via ?= operator. Thus, CFLAGS and the
>>>> corresponding addition of EXTRA_CFLAGS which contains the undef/def
>>>> is not appended. CFLAGS specified via the environment or as an argument to
>>>> make will cause this assignment not to occur (via ?= semantics) and whatever
>>>> they specify for CFLAGS is sent to CC.
>>>>
>>>> Here is some sample output:
>>>> make CFLAGS='-D_FORTIFY_SOURCE=2'
>>>> cc -D_FORTIFY_SOURCE=2 -I../include -D_GNU_SOURCE -DNO_ANDROID_BACKEND
>>>> -fPIC -DSHARED -c -o stringrep.lo stringrep.c
>>>>
>>>> With that said, *i think its safe* to bump it back to '-D_FORTIFY_SOURCE=2'
>
> I agree that 2 is the better value. I'll re-roll these to use 2. No
> one seems to be complaining
> on using the -D/-U options.
The only alternative that I could see would be to just drop
FORTIFY_SOURCE altogether from the upstream Makefile and defer setting
of it entirely to the packagers...
>
>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: William Roberts <william.c.roberts@intel.com>
>>>>>>> ---
>>>>>>> libselinux/src/Makefile | 2 +-
>>>>>>> libselinux/utils/Makefile | 2 +-
>>>>>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
>>>>>>> index 977b5c8cfcca..ee55bd0dbff7 100644
>>>>>>> --- a/libselinux/src/Makefile
>>>>>>> +++ b/libselinux/src/Makefile
>>>>>>> @@ -64,7 +64,7 @@ ifeq ($(COMPILER), gcc)
>>>>>>> EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op
>>>>>>> -Wpacked-bitfield-compat -Wsync-nand \
>>>>>>> -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc
>>>>>>> -Wsuggest-attribute=const \
>>>>>>> -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure
>>>>>>> -Wtrampolines -Wjump-misses-init \
>>>>>>> - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
>>>>>>> -Wp,-D_FORTIFY_SOURCE
>>>>>>> + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
>>>>>>> -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
>>>>>>> else
>>>>>>> EXTRA_CFLAGS = -Wunused-command-line-argument
>>>>>>> endif
>>>>>>> diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
>>>>>>> index d06ffd66893b..64ab877015c6 100644
>>>>>>> --- a/libselinux/utils/Makefile
>>>>>>> +++ b/libselinux/utils/Makefile
>>>>>>> @@ -30,7 +30,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k
>>>>>>> -Wformat-security -Winit-self -Wmissi
>>>>>>> -Wformat-extra-args -Wformat-zero-length -Wformat=2
>>>>>>> -Wmultichar \
>>>>>>> -Woverflow -Wpointer-to-int-cast -Wpragmas \
>>>>>>> -Wno-missing-field-initializers -Wno-sign-compare \
>>>>>>> - -Wno-format-nonliteral
>>>>>>> -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \
>>>>>>> + -Wno-format-nonliteral
>>>>>>> -Wframe-larger-than=$(MAX_STACK_SIZE) -U_FORTIFY_SOURCE
>>>>>>> -D_FORTIFY_SOURCE=1 \
>>>>>>> -fstack-protector-all --param=ssp-buffer-size=4
>>>>>>> -fexceptions \
>>>>>>> -fasynchronous-unwind-tables -fdiagnostics-show-option
>>>>>>> -funit-at-a-time \
>>>>>>> -Werror -Wno-aggregate-return -Wno-redundant-decls \
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error
2018-12-19 15:46 ` Stephen Smalley
@ 2018-12-19 15:48 ` William Roberts
0 siblings, 0 replies; 12+ messages in thread
From: William Roberts @ 2018-12-19 15:48 UTC (permalink / raw)
To: Stephen Smalley
Cc: Patrick Steinhardt, selinux, William Roberts, James Carter
On Wed, Dec 19, 2018 at 7:44 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> On 12/19/18 10:42 AM, William Roberts wrote:
> > On Wed, Dec 19, 2018 at 1:12 AM Patrick Steinhardt <ps@pks.im> wrote:
> >>
> >> Hi,
> >>
> >> On Tue, Dec 18, 2018 at 08:03:54AM -0800, William Roberts wrote:
> >>> Patrick,
> >>>
> >>> Hoping you could maybe weigh in on your choice for bypassing the
> >>> compiler driver with -Wp and not setting _FORTIFY_SOURCE to something
> >>> like 1 or 2?
> >>>
> >>> I'm seeing this issue on Ubuntu 16.04.5:
> >>> <command-line>:0:0: error: "_FORTIFY_SOURCE" redefined [-Werror]
> >>>
> >>> gcc version:
> >>> gcc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
> >>>
> >>> My thought is to undef/redef _FORTIFY_SOURCE in CFLAGS and set the
> >>> level to 2. Setting CFLAGS via the env/make arg will override this
> >>> behavior
> >>> and use CFLAGS as is.
> >>
> >> I used "-Wp" simply because it was existing previously, so I just
> >> stuck to what was there already. The original issue I had was
> >> that Gentoo Hardened, as Jason notes, already defines
> >> _FORTIFY_SOURCE as part of the compiler spec. Due to that, I was
> >> seeing a lot of warnings.
> >>
> >> So I set the flag to a simple define without setting a specific
> >> value, which _seemed_ to let the issue go away. But going back to
> >> the initial issue, this didn't seem to have solved it correctly.
> >> Dunno what I've been doing back then to not see the warnings
> >> after my change anymore, but I noticed that they have resurface
> >> recently.
> >>
> >> So I guess the real fix would be to redefine the value by first
> >> undef'ing it and then redefining it to the desired value. And I
> >> do agree that in that case, we should simply revert to
> >> _FORTIFY_SOURCE=2.
> >>
> >> Patrick
> >>
> >>> On Fri, Dec 14, 2018 at 8:02 AM William Roberts
> >>> <bill.c.roberts@gmail.com> wrote:
> >>>>
> >>>> On Fri, Dec 14, 2018 at 6:32 AM Stephen Smalley <sds@tycho.nsa.gov> wrote:
> >>>>>
> >>>>> On 12/14/18 8:43 AM, Stephen Smalley wrote:
> >>>>>> On 12/13/18 4:32 PM, bill.c.roberts@gmail.com wrote:
> >>>>>>> From: William Roberts <william.c.roberts@intel.com>
> >>>>>>>
> >>>>>>> Certain builds of gcc enable _FORTIFY_SOURCE which results in the error:
> >>>>>>> <command-line>:0:0: warning: "_FORTIFY_SOURCE" redefined
> >>>>>>> <command-line>:0:0: note: this is the location of the previous definition
> >>>>>>>
> >>>>>>> Correct this by undefining it first and redefining it. Also, the previous
> >>>>>>> command line option was using -Wp which passing the value *AS IS* to the
> >>>>>>> pre-processor rather than to the compiler driver. The C pre-processor has
> >>>>>>> an undocumented interface subject to change per man 1 gcc. Just use the
> >>>>>>> -D option as is.
> >>>>>>
> >>>>>> See commit ca07a2ad46be141dad90d885dd33a2ac31c6559a ("libselinux: avoid
> >>>>>> redefining _FORTIFY_SOURCE") for why we don't specify a value for
> >>>>>> _FORTIFY_SOURCE here. Not sure about the -Wp,-D vs -D rationale.
> >>>>
> >>>> I'm not 100% convinced that the patch is the best solution or the commit message
> >>>> is describing the problem correctly. I could also be understanding it
> >>>> wrong here.
> >>>> The man page is saying not to bypass the compiler driver via -Wp, and I don't
> >>>> see a good reason for it either.
> >>>>
> >>>> See my comments below, they feed back into this.
> >>>>
> >>>>>
> >>>>> I guess the issue here is that we want to provide sane defaults for
> >>>>> building without breaking the build when others specify their own
> >>>>> definitions and without weakening those definitions. By undefining and
> >>>>> re-defining, it seems like we might weaken existing builds that were
> >>>>> specifying 2.
> >>>>
> >>>> We conditionally assign to CFLAGS via ?= operator. Thus, CFLAGS and the
> >>>> corresponding addition of EXTRA_CFLAGS which contains the undef/def
> >>>> is not appended. CFLAGS specified via the environment or as an argument to
> >>>> make will cause this assignment not to occur (via ?= semantics) and whatever
> >>>> they specify for CFLAGS is sent to CC.
> >>>>
> >>>> Here is some sample output:
> >>>> make CFLAGS='-D_FORTIFY_SOURCE=2'
> >>>> cc -D_FORTIFY_SOURCE=2 -I../include -D_GNU_SOURCE -DNO_ANDROID_BACKEND
> >>>> -fPIC -DSHARED -c -o stringrep.lo stringrep.c
> >>>>
> >>>> With that said, *i think its safe* to bump it back to '-D_FORTIFY_SOURCE=2'
> >
> > I agree that 2 is the better value. I'll re-roll these to use 2. No
> > one seems to be complaining
> > on using the -D/-U options.
>
> The only alternative that I could see would be to just drop
> FORTIFY_SOURCE altogether from the upstream Makefile and defer setting
> of it entirely to the packagers...
I like having a sane set of defaults and I think _FORTIFY_SOURCE=2 is
inline with it.
I guess we can try moving back to 2, and if someone complains, just
stop setting it.
>
> >
> >>>>
> >>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> >>>>>>> ---
> >>>>>>> libselinux/src/Makefile | 2 +-
> >>>>>>> libselinux/utils/Makefile | 2 +-
> >>>>>>> 2 files changed, 2 insertions(+), 2 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile
> >>>>>>> index 977b5c8cfcca..ee55bd0dbff7 100644
> >>>>>>> --- a/libselinux/src/Makefile
> >>>>>>> +++ b/libselinux/src/Makefile
> >>>>>>> @@ -64,7 +64,7 @@ ifeq ($(COMPILER), gcc)
> >>>>>>> EXTRA_CFLAGS = -fipa-pure-const -Wlogical-op
> >>>>>>> -Wpacked-bitfield-compat -Wsync-nand \
> >>>>>>> -Wcoverage-mismatch -Wcpp -Wformat-contains-nul -Wnormalized=nfc
> >>>>>>> -Wsuggest-attribute=const \
> >>>>>>> -Wsuggest-attribute=noreturn -Wsuggest-attribute=pure
> >>>>>>> -Wtrampolines -Wjump-misses-init \
> >>>>>>> - -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> >>>>>>> -Wp,-D_FORTIFY_SOURCE
> >>>>>>> + -Wno-suggest-attribute=pure -Wno-suggest-attribute=const
> >>>>>>> -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
> >>>>>>> else
> >>>>>>> EXTRA_CFLAGS = -Wunused-command-line-argument
> >>>>>>> endif
> >>>>>>> diff --git a/libselinux/utils/Makefile b/libselinux/utils/Makefile
> >>>>>>> index d06ffd66893b..64ab877015c6 100644
> >>>>>>> --- a/libselinux/utils/Makefile
> >>>>>>> +++ b/libselinux/utils/Makefile
> >>>>>>> @@ -30,7 +30,7 @@ CFLAGS ?= -O -Wall -W -Wundef -Wformat-y2k
> >>>>>>> -Wformat-security -Winit-self -Wmissi
> >>>>>>> -Wformat-extra-args -Wformat-zero-length -Wformat=2
> >>>>>>> -Wmultichar \
> >>>>>>> -Woverflow -Wpointer-to-int-cast -Wpragmas \
> >>>>>>> -Wno-missing-field-initializers -Wno-sign-compare \
> >>>>>>> - -Wno-format-nonliteral
> >>>>>>> -Wframe-larger-than=$(MAX_STACK_SIZE) -Wp,-D_FORTIFY_SOURCE \
> >>>>>>> + -Wno-format-nonliteral
> >>>>>>> -Wframe-larger-than=$(MAX_STACK_SIZE) -U_FORTIFY_SOURCE
> >>>>>>> -D_FORTIFY_SOURCE=1 \
> >>>>>>> -fstack-protector-all --param=ssp-buffer-size=4
> >>>>>>> -fexceptions \
> >>>>>>> -fasynchronous-unwind-tables -fdiagnostics-show-option
> >>>>>>> -funit-at-a-time \
> >>>>>>> -Werror -Wno-aggregate-return -Wno-redundant-decls \
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-12-19 15:48 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-13 21:32 [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error bill.c.roberts
2018-12-13 21:32 ` [PATCH 2/2] Makefile: add -Wstrict-overflow=5 to CFLAGS bill.c.roberts
2018-12-14 13:43 ` [PATCH 1/2] Makefile: fix _FORTIFY_SOURCE redefined build error Stephen Smalley
2018-12-14 14:34 ` Stephen Smalley
2018-12-14 16:02 ` William Roberts
2018-12-18 16:03 ` William Roberts
2018-12-18 19:02 ` William Roberts
2018-12-19 6:15 ` Jason Zaman
2018-12-19 9:12 ` Patrick Steinhardt
2018-12-19 15:42 ` William Roberts
2018-12-19 15:46 ` Stephen Smalley
2018-12-19 15:48 ` William Roberts
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).