[PATCH] security_flags.inc: Use -O with -D_FORTIFY_SOURCE

[PATCH] security_flags.inc: Use -O with -D_FORTIFY_SOURCE

[Khem Raj]

compiler can only use fortify options when some level of optimization is
on, otherwise it ends up sending some warnings.

warning: _FORTIFY_SOURCE requires compiling with optimization (-O) [-W#warnings]

this is usually OK, since -O<level> would be added via CFLAGS to
compiler cmdline in normal compile stages, however during configure
there are problems when CC,CPP,CXX are probed alone in configure tests
which results in above warning, which confuses the configure results and
autotools 2.70+ detects it as error e.g.

configure:17292: error: C preprocessor "riscv32-yoe-linux-clang -target riscv32-yoe-linux      -mlittle-endian -mno-relax -Qunused-arguments -fstack-protector-strong  -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -Werror=format-security --sysroot=/mnt/b/yoe/master/build/tmp/work/riscv32-yoe-linux/ndpi/3.4-r0/recipe-sysroot -E" fails sanity check
See `config.log' for more details

therefore adding a -O ( which actually is -O1 ) to lcl_maybe_fortify
means we can properly test these configure tests and real -O<level> will
still override -O added here, so overrall behavior improves

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 meta/conf/distro/include/security_flags.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/conf/distro/include/security_flags.inc b/meta/conf/distro/include/security_flags.inc
index 4e64eb99f9..05253b2df9 100644
--- a/meta/conf/distro/include/security_flags.inc
+++ b/meta/conf/distro/include/security_flags.inc
@@ -10,7 +10,7 @@ GCCPIE ?= "--enable-default-pie"
 
 # _FORTIFY_SOURCE requires -O1 or higher, so disable in debug builds as they use
 # -O0 which then results in a compiler warning.
-lcl_maybe_fortify ?= "${@oe.utils.conditional('DEBUG_BUILD','1','','-D_FORTIFY_SOURCE=2',d)}"
+lcl_maybe_fortify ?= "${@oe.utils.conditional('DEBUG_BUILD','1','','-O -D_FORTIFY_SOURCE=2',d)}"
 
 # Error on use of format strings that represent possible security problems
 SECURITY_STRINGFORMAT ?= "-Wformat -Wformat-security -Werror=format-security"
-- 
2.30.0

Re: [OE-core] [PATCH] security_flags.inc: Use -O with -D_FORTIFY_SOURCE

[Anuj Mittal]

On Fri, 2021-02-05 at 22:31 -0800, Khem Raj wrote:
> compiler can only use fortify options when some level of optimization
> is
> on, otherwise it ends up sending some warnings.
> 
> warning: _FORTIFY_SOURCE requires compiling with optimization (-O) [-
> W#warnings]
> 
> this is usually OK, since -O<level> would be added via CFLAGS to
> compiler cmdline in normal compile stages, however during configure
> there are problems when CC,CPP,CXX are probed alone in configure
> tests
> which results in above warning, which confuses the configure results
> and
> autotools 2.70+ detects it as error e.g.
> 
> configure:17292: error: C preprocessor "riscv32-yoe-linux-clang -
> target riscv32-yoe-linux      -mlittle-endian -mno-relax -Qunused-
> arguments -fstack-protector-strong  -D_FORTIFY_SOURCE=2 -Wformat -
> Wformat-security -Werror=format-security --
> sysroot=/mnt/b/yoe/master/build/tmp/work/riscv32-yoe-linux/ndpi/3.4-
> r0/recipe-sysroot -E" fails sanity check
> See `config.log' for more details
> 
> therefore adding a -O ( which actually is -O1 ) to lcl_maybe_fortify
> means we can properly test these configure tests and real -O<level>
> will
> still override -O added here, so overrall behavior improves

gcc man page says that the last specified O option will take effect.

In case of ncurses for example using poky:

x86_64-poky-linux-gcc -m64 -march=skylake -mtune=generic -mavx2 -
mfpmath=sse --sysroot=/home/anmitta2/work/poky/build/tmp/work/skylake-
64-poky-linux/ncurses/6.2-r0/recipe-sysroot -DHAVE_CONFIG_H -
I../ncurses -I. -I../../../git/ncurses -I../include -
I../../../git/ncurses/../include -D_FORTIFY_SOURCE=2 -D_DEFAULT_SOURCE
-D_XOPEN_SOURCE=600 -DNDEBUG -O2 -pipe -g -feliminate-unused-debug-
types -fmacro-prefix-
map=/home/anmitta2/work/poky/build/tmp/work/skylake-64-poky-
linux/ncurses/6.2-r0=/usr/src/debug/ncurses/6.2-r0                    
-fdebug-prefix-map=/home/anmitta2/work/poky/build/tmp/work/skylake-64-
poky-linux/ncurses/6.2-r0=/usr/src/debug/ncurses/6.2-r0               
-fdebug-prefix-map=/home/anmitta2/work/poky/build/tmp/work/skylake-64-
poky-linux/ncurses/6.2-r0/recipe-sysroot=                      -fdebug-
prefix-map=/home/anmitta2/work/poky/build/tmp/work/skylake-64-poky-
linux/ncurses/6.2-r0/recipe-sysroot-native=  -fstack-protector-strong -
O -Wformat -Wformat-security -Werror=format-security --param max-
inline-insns-single=1200 -fPIC -DUSE_TERMLIB -c
../../../git/ncurses/tinfo/doalloc.c -o ../obj_s/doalloc.o

I see -O after -O2 so is O2 really taking effect? 

Thanks,

Anuj

> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
>  meta/conf/distro/include/security_flags.inc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/conf/distro/include/security_flags.inc
> b/meta/conf/distro/include/security_flags.inc
> index 4e64eb99f9..05253b2df9 100644
> --- a/meta/conf/distro/include/security_flags.inc
> +++ b/meta/conf/distro/include/security_flags.inc
> @@ -10,7 +10,7 @@ GCCPIE ?= "--enable-default-pie"
>  
>  # _FORTIFY_SOURCE requires -O1 or higher, so disable in debug builds
> as they use
>  # -O0 which then results in a compiler warning.
> -lcl_maybe_fortify ?=
> "${@oe.utils.conditional('DEBUG_BUILD','1','','-
> D_FORTIFY_SOURCE=2',d)}"
> +lcl_maybe_fortify ?=
> "${@oe.utils.conditional('DEBUG_BUILD','1','','-O -
> D_FORTIFY_SOURCE=2',d)}"
>  
>  # Error on use of format strings that represent possible security
> problems
>  SECURITY_STRINGFORMAT ?= "-Wformat -Wformat-security -Werror=format-
> security"
> 
> 
> 

Re: [OE-core] [PATCH] security_flags.inc: Use -O with -D_FORTIFY_SOURCE

[Khem Raj]

On Tue, Feb 9, 2021 at 8:47 PM Mittal, Anuj <anuj.mittal@intel.com> wrote:
>
> On Fri, 2021-02-05 at 22:31 -0800, Khem Raj wrote:
> > compiler can only use fortify options when some level of optimization
> > is
> > on, otherwise it ends up sending some warnings.
> >
> > warning: _FORTIFY_SOURCE requires compiling with optimization (-O) [-
> > W#warnings]
> >
> > this is usually OK, since -O<level> would be added via CFLAGS to
> > compiler cmdline in normal compile stages, however during configure
> > there are problems when CC,CPP,CXX are probed alone in configure
> > tests
> > which results in above warning, which confuses the configure results
> > and
> > autotools 2.70+ detects it as error e.g.
> >
> > configure:17292: error: C preprocessor "riscv32-yoe-linux-clang -
> > target riscv32-yoe-linux      -mlittle-endian -mno-relax -Qunused-
> > arguments -fstack-protector-strong  -D_FORTIFY_SOURCE=2 -Wformat -
> > Wformat-security -Werror=format-security --
> > sysroot=/mnt/b/yoe/master/build/tmp/work/riscv32-yoe-linux/ndpi/3.4-
> > r0/recipe-sysroot -E" fails sanity check
> > See `config.log' for more details
> >
> > therefore adding a -O ( which actually is -O1 ) to lcl_maybe_fortify
> > means we can properly test these configure tests and real -O<level>
> > will
> > still override -O added here, so overrall behavior improves
>
> gcc man page says that the last specified O option will take effect.
>
> In case of ncurses for example using poky:
>
> x86_64-poky-linux-gcc -m64 -march=skylake -mtune=generic -mavx2 -
> mfpmath=sse --sysroot=/home/anmitta2/work/poky/build/tmp/work/skylake-
> 64-poky-linux/ncurses/6.2-r0/recipe-sysroot -DHAVE_CONFIG_H -
> I../ncurses -I. -I../../../git/ncurses -I../include -
> I../../../git/ncurses/../include -D_FORTIFY_SOURCE=2 -D_DEFAULT_SOURCE
> -D_XOPEN_SOURCE=600 -DNDEBUG -O2 -pipe -g -feliminate-unused-debug-
> types -fmacro-prefix-
> map=/home/anmitta2/work/poky/build/tmp/work/skylake-64-poky-
> linux/ncurses/6.2-r0=/usr/src/debug/ncurses/6.2-r0
> -fdebug-prefix-map=/home/anmitta2/work/poky/build/tmp/work/skylake-64-
> poky-linux/ncurses/6.2-r0=/usr/src/debug/ncurses/6.2-r0
> -fdebug-prefix-map=/home/anmitta2/work/poky/build/tmp/work/skylake-64-
> poky-linux/ncurses/6.2-r0/recipe-sysroot=                      -fdebug-
> prefix-map=/home/anmitta2/work/poky/build/tmp/work/skylake-64-poky-
> linux/ncurses/6.2-r0/recipe-sysroot-native=  -fstack-protector-strong -
> O -Wformat -Wformat-security -Werror=format-security --param max-
> inline-insns-single=1200 -fPIC -DUSE_TERMLIB -c
> ../../../git/ncurses/tinfo/doalloc.c -o ../obj_s/doalloc.o
>
> I see -O after -O2 so is O2 really taking effect?

In this case -O  will take effect sadly. and it seems to be that
autconf munges the compiler cmdline
while generating CFLAGS in generated Makefiles and appends the value
of -On coming from CC
variable last.

I think right solution would be to add same -O<level> as specified in
SELECTED_OPTIMIZATION so it remains
in sync always, I have sent a patch to ml. Could you test it out and
let me know if it works for you as well.

>
> Thanks,
>
> Anuj
>
> >
> > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > ---
> >  meta/conf/distro/include/security_flags.inc | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/meta/conf/distro/include/security_flags.inc
> > b/meta/conf/distro/include/security_flags.inc
> > index 4e64eb99f9..05253b2df9 100644
> > --- a/meta/conf/distro/include/security_flags.inc
> > +++ b/meta/conf/distro/include/security_flags.inc
> > @@ -10,7 +10,7 @@ GCCPIE ?= "--enable-default-pie"
> >
> >  # _FORTIFY_SOURCE requires -O1 or higher, so disable in debug builds
> > as they use
> >  # -O0 which then results in a compiler warning.
> > -lcl_maybe_fortify ?=
> > "${@oe.utils.conditional('DEBUG_BUILD','1','','-
> > D_FORTIFY_SOURCE=2',d)}"
> > +lcl_maybe_fortify ?=
> > "${@oe.utils.conditional('DEBUG_BUILD','1','','-O -
> > D_FORTIFY_SOURCE=2',d)}"
> >
> >  # Error on use of format strings that represent possible security
> > problems
> >  SECURITY_STRINGFORMAT ?= "-Wformat -Wformat-security -Werror=format-
> > security"
> >
> > 
> >
>

Re: [OE-core] [PATCH] security_flags.inc: Use -O with -D_FORTIFY_SOURCE

[Mikko Rapeli]

Hi,

On Tue, Feb 09, 2021 at 11:37:39PM -0800, Khem Raj wrote:
> In this case -O  will take effect sadly. and it seems to be that
> autconf munges the compiler cmdline
> while generating CFLAGS in generated Makefiles and appends the value
> of -On coming from CC
> variable last.
> 
> I think right solution would be to add same -O<level> as specified in
> SELECTED_OPTIMIZATION so it remains
> in sync always, I have sent a patch to ml. Could you test it out and
> let me know if it works for you as well.

Or let it go? A lot of recipes amend their own optimization flags and override
distro wide optimization and other compiler flags. I once fixes all recipes
in a project which were not obeying Os until buildhistory showed change in binary
sizes... that was a lot of work for a PoC..

Cheers,

-Mikko

Re: [OE-core] [PATCH] security_flags.inc: Use -O with -D_FORTIFY_SOURCE

[Andre McCurdy]

On Wed, Feb 10, 2021 at 12:48 AM Mikko Rapeli <mikko.rapeli@bmw.de> wrote:
>
> Hi,
>
> On Tue, Feb 09, 2021 at 11:37:39PM -0800, Khem Raj wrote:
> > In this case -O  will take effect sadly. and it seems to be that
> > autconf munges the compiler cmdline
> > while generating CFLAGS in generated Makefiles and appends the value
> > of -On coming from CC
> > variable last.
> >
> > I think right solution would be to add same -O<level> as specified in
> > SELECTED_OPTIMIZATION so it remains
> > in sync always, I have sent a patch to ml. Could you test it out and
> > let me know if it works for you as well.
>
> Or let it go? A lot of recipes amend their own optimization flags and override
> distro wide optimization and other compiler flags. I once fixes all recipes
> in a project which were not obeying Os until buildhistory showed change in binary
> sizes... that was a lot of work for a PoC..

If the goal is to ensure that the optimisation flag from
FULL_OPTIMIZATION and the -D_FORTIFY_SOURCE flag from
lcl_maybe_fortify are always applied together then isn't the easiest
solution to move -D_FORTIFY_SOURCE out of lcl_maybe_fortify and into
FULL_OPTIMIZATION ?

Putting a separate optimisation flag in lcl_maybe_fortify and trying
to arrange for it not to clash with or override the one already in
FULL_OPTIMIZATION seems like an ugly solution, even if it can be made
to work.

Re: [OE-core] [PATCH] security_flags.inc: Use -O with -D_FORTIFY_SOURCE

[Khem Raj]

On Wed, Feb 10, 2021 at 12:48 AM Mikko Rapeli <mikko.rapeli@bmw.de> wrote:
>
> Hi,
>
> On Tue, Feb 09, 2021 at 11:37:39PM -0800, Khem Raj wrote:
> > In this case -O  will take effect sadly. and it seems to be that
> > autconf munges the compiler cmdline
> > while generating CFLAGS in generated Makefiles and appends the value
> > of -On coming from CC
> > variable last.
> >
> > I think right solution would be to add same -O<level> as specified in
> > SELECTED_OPTIMIZATION so it remains
> > in sync always, I have sent a patch to ml. Could you test it out and
> > let me know if it works for you as well.
>
> Or let it go? A lot of recipes amend their own optimization flags and override
> distro wide optimization and other compiler flags. I once fixes all recipes
> in a project which were not obeying Os until buildhistory showed change in binary
> sizes... that was a lot of work for a PoC..
>

I think we need to solve this. I have seen many cases where configure
tests silently fails due to these warnings and we
don't necessarily notice it because configure just disables the failed
part and the package might not fail to build

We still are fine if a package is overriding these flags but we want
to be consistent about what we pass.

> Cheers,
>
> -Mikko
> 
>

Re: [OE-core] [PATCH] security_flags.inc: Use -O with -D_FORTIFY_SOURCE

[Khem Raj]

On Wed, Feb 10, 2021 at 1:57 AM Andre McCurdy <armccurdy@gmail.com> wrote:
>
> On Wed, Feb 10, 2021 at 12:48 AM Mikko Rapeli <mikko.rapeli@bmw.de> wrote:
> >
> > Hi,
> >
> > On Tue, Feb 09, 2021 at 11:37:39PM -0800, Khem Raj wrote:
> > > In this case -O  will take effect sadly. and it seems to be that
> > > autconf munges the compiler cmdline
> > > while generating CFLAGS in generated Makefiles and appends the value
> > > of -On coming from CC
> > > variable last.
> > >
> > > I think right solution would be to add same -O<level> as specified in
> > > SELECTED_OPTIMIZATION so it remains
> > > in sync always, I have sent a patch to ml. Could you test it out and
> > > let me know if it works for you as well.
> >
> > Or let it go? A lot of recipes amend their own optimization flags and override
> > distro wide optimization and other compiler flags. I once fixes all recipes
> > in a project which were not obeying Os until buildhistory showed change in binary
> > sizes... that was a lot of work for a PoC..
>
> If the goal is to ensure that the optimisation flag from
> FULL_OPTIMIZATION and the -D_FORTIFY_SOURCE flag from
> lcl_maybe_fortify are always applied together then isn't the easiest
> solution to move -D_FORTIFY_SOURCE out of lcl_maybe_fortify and into
> FULL_OPTIMIZATION ?
>

The problem is that we insert the flags inconsistently and it depends
on underlying build systems
interpretation of these flags. e.g. We add D_FORTIFY_SOURCE to CC/CXX
but -O<n> to CFLAGS/CXXFLAGS
many tests e.g. do not use CC and CFLAGS together, if we remove
D_FORTIFY_SOURCE from CC/CXX then it does not get tested in configure
tests but final compile uses it and cause miscompiles
and this all is also need to keep in mind that we might have external
toolchains which are compiled with its own
set of options by default. Thats why we have to be explicit about
these flags so they can be customized if needed.

> Putting a separate optimisation flag in lcl_maybe_fortify and trying
> to arrange for it not to clash with or override the one already in
> FULL_OPTIMIZATION seems like an ugly solution, even if it can be made
> to work.