On Fri, Dec 14, 2018 at 6:32 AM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
>
> On 12/14/18 8:43 AM, Stephen Smalley wrote:
> > On 12/13/18 4:32 PM, bill.c.roberts@xxxxxxxxx wrote:
> >> From: William Roberts <william.c.roberts@xxxxxxxxx>
> >>
> >> 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@xxxxxxxxx>
> >> ---
> >> 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 \
> >>
> >
>
