[Buildroot] [PATCH v2,1/2] lxc: fix missing include for va_list

[Buildroot] [PATCH v2,1/2] lxc: fix missing include for va_list

[Fabrice Fontaine]

Fixes:
 - http://autobuild.buildroot.org/results/0b90e7dca2984652842832a41abad93ac49a9b86

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 ...01-stringutils-include-stdarg-for-va_list.patch | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 package/lxc/0001-stringutils-include-stdarg-for-va_list.patch

diff --git a/package/lxc/0001-stringutils-include-stdarg-for-va_list.patch b/package/lxc/0001-stringutils-include-stdarg-for-va_list.patch
new file mode 100644
index 0000000000..d0a450e8cf
--- /dev/null
+++ b/package/lxc/0001-stringutils-include-stdarg-for-va_list.patch
@@ -0,0 +1,30 @@
+From b7df06ad14c04f18b7db5b64d5142b802bf64cb2 Mon Sep 17 00:00:00 2001
+From: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+Date: Mon, 3 Dec 2018 22:18:16 +0100
+Subject: [PATCH] stringutils: include stdarg for va_list
+
+Fixes:
+ - http://autobuild.buildroot.org/results/0b90e7dca2984652842832a41abad93ac49a9b86
+
+Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+[Upstream status: https://github.com/lxc/lxc/pull/2745]
+---
+ src/lxc/string_utils.h | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/src/lxc/string_utils.h b/src/lxc/string_utils.h
+index 4065e4e6..d4e633cc 100644
+--- a/src/lxc/string_utils.h
++++ b/src/lxc/string_utils.h
+@@ -20,6 +20,8 @@
+ #ifndef __LXC_STRING_UTILS_H
+ #define __LXC_STRING_UTILS_H
+ 
++#include <stdarg.h>
++
+ #include "config.h"
+ 
+ #include "initutils.h"
+-- 
+2.14.1
+
-- 
2.14.1

[Buildroot] [PATCH v2,2/2] lxc: fix build without stack protector

[Fabrice Fontaine]

Stack protector has been added in version 3.0.3 by
https://github.com/lxc/lxc/commit/2268c27754152aa538db2c9e3753d72d19bcd17a

Disable it through
ax_cv_check_cflags__Werror__fstack_protector_strong=no

Fixes:
 - http://autobuild.buildroot.org/results/0b90e7dca2984652842832a41abad93ac49a9b86

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
Changes v1 -> v2 (after review of Thomas Petazzoni):
 - Drop patch to add a new option, instead use 
   ax_cv_check_cflags__Werror__fstack_protector_strong=no

 package/lxc/lxc.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/package/lxc/lxc.mk b/package/lxc/lxc.mk
index 48d5b20329..0b7b03bcac 100644
--- a/package/lxc/lxc.mk
+++ b/package/lxc/lxc.mk
@@ -11,6 +11,8 @@ LXC_LICENSE_FILES = COPYING
 LXC_DEPENDENCIES = host-pkgconf
 LXC_INSTALL_STAGING = YES
 
+LXC_CONF_ENV = ax_cv_check_cflags__Werror__fstack_protector_strong=no
+
 LXC_CONF_OPTS = --disable-apparmor --with-distro=buildroot \
 	--disable-werror \
 	$(if $(BR2_PACKAGE_BASH),,--disable-bash)
-- 
2.14.1

[Buildroot] [PATCH v2, 2/2] lxc: fix build without stack protector

[Baruch Siach]

Hi Fabrice,

On Mon, Dec 03, 2018 at 11:38:55PM +0100, Fabrice Fontaine wrote:
> Stack protector has been added in version 3.0.3 by
> https://github.com/lxc/lxc/commit/2268c27754152aa538db2c9e3753d72d19bcd17a
> 
> Disable it through
> ax_cv_check_cflags__Werror__fstack_protector_strong=no
> 
> Fixes:
>  - http://autobuild.buildroot.org/results/0b90e7dca2984652842832a41abad93ac49a9b86
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
> Changes v1 -> v2 (after review of Thomas Petazzoni):
>  - Drop patch to add a new option, instead use 
>    ax_cv_check_cflags__Werror__fstack_protector_strong=no
> 
>  package/lxc/lxc.mk | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/package/lxc/lxc.mk b/package/lxc/lxc.mk
> index 48d5b20329..0b7b03bcac 100644
> --- a/package/lxc/lxc.mk
> +++ b/package/lxc/lxc.mk
> @@ -11,6 +11,8 @@ LXC_LICENSE_FILES = COPYING
>  LXC_DEPENDENCIES = host-pkgconf
>  LXC_INSTALL_STAGING = YES
>  
> +LXC_CONF_ENV = ax_cv_check_cflags__Werror__fstack_protector_strong=no

Can't we make this depend on BR2_TOOLCHAIN_HAS_SSP? See the packages ntp or 
sox, for example.

An explicit ssp link compiler test in configure might be even better.

> +
>  LXC_CONF_OPTS = --disable-apparmor --with-distro=buildroot \
>  	--disable-werror \
>  	$(if $(BR2_PACKAGE_BASH),,--disable-bash)

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH v2, 2/2] lxc: fix build without stack protector

[Thomas Petazzoni]

Hello,

+Arnout, Peter, Yann in Cc.

On Tue, 4 Dec 2018 07:54:39 +0200, Baruch Siach wrote:

> > +LXC_CONF_ENV = ax_cv_check_cflags__Werror__fstack_protector_strong=no  
> 
> Can't we make this depend on BR2_TOOLCHAIN_HAS_SSP? See the packages ntp or 
> sox, for example.

The question is whether we want SSP support to be enabled as soon as
the toolchain *has* SSP support, or only when the user explicitly
request SSP support using BR2_SSP_{REGULAR,STRONG,ALL} ?

This is a real policy decision:

 - Do we let the packages default to what they think is good (of course
   as long as the toolchain provides what's needed) ?

 - Or do we enforce the system-level configuration options that
   Buildroot has ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2, 2/2] lxc: fix build without stack protector

[Baruch Siach]

Hi Thomas,

On Tue, Dec 04, 2018 at 09:10:29AM +0100, Thomas Petazzoni wrote:
> +Arnout, Peter, Yann in Cc.
> 
> On Tue, 4 Dec 2018 07:54:39 +0200, Baruch Siach wrote:
> 
> > > +LXC_CONF_ENV = ax_cv_check_cflags__Werror__fstack_protector_strong=no  
> > 
> > Can't we make this depend on BR2_TOOLCHAIN_HAS_SSP? See the packages ntp or 
> > sox, for example.
> 
> The question is whether we want SSP support to be enabled as soon as
> the toolchain *has* SSP support, or only when the user explicitly
> request SSP support using BR2_SSP_{REGULAR,STRONG,ALL} ?
> 
> This is a real policy decision:
> 
>  - Do we let the packages default to what they think is good (of course
>    as long as the toolchain provides what's needed) ?
> 
>  - Or do we enforce the system-level configuration options that
>    Buildroot has ?

I think we should let upstream packages decide when to enable SSP. This patch, 
however, disables SSP unconditionally, AFAICS. I don't think we want to do 
that. So I suggest to force SSP disable only when BR2_TOOLCHAIN_HAS_SSP is 
disabled.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH v2, 2/2] lxc: fix build without stack protector

[Thomas Petazzoni]

Hello,

On Tue, 4 Dec 2018 10:15:11 +0200, Baruch Siach wrote:

> > The question is whether we want SSP support to be enabled as soon as
> > the toolchain *has* SSP support, or only when the user explicitly
> > request SSP support using BR2_SSP_{REGULAR,STRONG,ALL} ?
> > 
> > This is a real policy decision:
> > 
> >  - Do we let the packages default to what they think is good (of course
> >    as long as the toolchain provides what's needed) ?
> > 
> >  - Or do we enforce the system-level configuration options that
> >    Buildroot has ?  
> 
> I think we should let upstream packages decide when to enable SSP. This patch, 
> however, disables SSP unconditionally, AFAICS. I don't think we want to do 
> that. So I suggest to force SSP disable only when BR2_TOOLCHAIN_HAS_SSP is 
> disabled.

Well, Fabrice patch doesn't really disable SSP unconditionally: it
tells the package to never enable SSP on its own.

However, if one of the global BR2_SSP_{REGULAR,STRONG,ALL} options are
enabled, the compiler wrapper will properly build everything with SSP
support, including lxc. So basically, Fabrice's patch is a correct
implementation for the option (2) I described above.

I don't (yet?) have a strong opinion on which of the two options we
want to chose, but Fabrice's solution does implement one of them
correctly :)

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2, 2/2] lxc: fix build without stack protector

[Baruch Siach]

Hi Thomas,

On Tue, Dec 04, 2018 at 09:31:42AM +0100, Thomas Petazzoni wrote:
> On Tue, 4 Dec 2018 10:15:11 +0200, Baruch Siach wrote:
> > > The question is whether we want SSP support to be enabled as soon as
> > > the toolchain *has* SSP support, or only when the user explicitly
> > > request SSP support using BR2_SSP_{REGULAR,STRONG,ALL} ?
> > > 
> > > This is a real policy decision:
> > > 
> > >  - Do we let the packages default to what they think is good (of course
> > >    as long as the toolchain provides what's needed) ?
> > > 
> > >  - Or do we enforce the system-level configuration options that
> > >    Buildroot has ?  
> > 
> > I think we should let upstream packages decide when to enable SSP. This patch, 
> > however, disables SSP unconditionally, AFAICS. I don't think we want to do 
> > that. So I suggest to force SSP disable only when BR2_TOOLCHAIN_HAS_SSP is 
> > disabled.
> 
> Well, Fabrice patch doesn't really disable SSP unconditionally: it
> tells the package to never enable SSP on its own.
> 
> However, if one of the global BR2_SSP_{REGULAR,STRONG,ALL} options are
> enabled, the compiler wrapper will properly build everything with SSP
> support, including lxc. So basically, Fabrice's patch is a correct
> implementation for the option (2) I described above.
> 
> I don't (yet?) have a strong opinion on which of the two options we
> want to chose, but Fabrice's solution does implement one of them
> correctly :)

Thanks for the clarification. I guess we need at least a comment to clarify 
that.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] Stack protector choices [was: [PATCH v2, 2/2] lxc: fix build without stack protector]

[Arnout Vandecappelle]

On 04/12/2018 09:10, Thomas Petazzoni wrote:
> Hello,
> 
> +Arnout, Peter, Yann in Cc.
> 
> On Tue, 4 Dec 2018 07:54:39 +0200, Baruch Siach wrote:
> 
>>> +LXC_CONF_ENV = ax_cv_check_cflags__Werror__fstack_protector_strong=no  
>>
>> Can't we make this depend on BR2_TOOLCHAIN_HAS_SSP? See the packages ntp or 
>> sox, for example.
> 
> The question is whether we want SSP support to be enabled as soon as
> the toolchain *has* SSP support, or only when the user explicitly
> request SSP support using BR2_SSP_{REGULAR,STRONG,ALL} ?
> 
> This is a real policy decision:
> 
>  - Do we let the packages default to what they think is good (of course
>    as long as the toolchain provides what's needed) ?
> 
>  - Or do we enforce the system-level configuration options that
>    Buildroot has ?

 If you look at it from a practical point of view, we can only do option (1), or
"we sometimes enforce the system-level configuration options, but not
consistently". Indeed, if a package does the stack protector detection
correctly, chances are we'll never even notice that it enables stack protector.
So option (2) is simply not realistic.

 So in practice, I think the policy should be (as it is for other policy
options, e.g. debug):

1. If there is no configuration option for it, let the package decide.

2. If there is a configuration option that does the same as our option, disable
it and let the toolchain wrapper apply the correct option.

3. If there is a configuration option and it does something more, enable it
automatically based on the global Buildroot option.

4. In very exceptional cases where (3) is even more invasive, offer an option to
the user (but only if the toolchain supports it, of course).

 An example of (3) would be the kernel's stack protector which is a little more
than just -fstack-protector so it needs to be enabled explicitly. An example of
(4) would be -fsanitize=address vs. KASAN - KASAN is a way more invasive
operation than gcc's address sanitizer. Note that specifically for the kernel,
we of course always do (4) - kernel config is provided separately. But the
kernel is just an easy example that everybody knows.

 Regards,
 Arnout

[Buildroot] Stack protector choices [was: [PATCH v2, 2/2] lxc: fix build without stack protector]

[Thomas Petazzoni]

Hello,

On Tue, 4 Dec 2018 10:35:05 +0100, Arnout Vandecappelle wrote:

>  If you look at it from a practical point of view, we can only do option (1), or
> "we sometimes enforce the system-level configuration options, but not
> consistently". Indeed, if a package does the stack protector detection
> correctly, chances are we'll never even notice that it enables stack protector.

I was going to say that the checksec tool could help us detect such
situations (i.e SSP is disabled at the Buildroot level, but some
binaries end up being built with SSP). However, it seems like checksec
is not checking SSP support, but PIE, RELRO and a few other things.

>  So in practice, I think the policy should be (as it is for other policy
> options, e.g. debug):
> 
> 1. If there is no configuration option for it, let the package decide.
> 
> 2. If there is a configuration option that does the same as our option, disable
> it and let the toolchain wrapper apply the correct option.
> 
> 3. If there is a configuration option and it does something more, enable it
> automatically based on the global Buildroot option.
> 
> 4. In very exceptional cases where (3) is even more invasive, offer an option to
> the user (but only if the toolchain supports it, of course).
> 
>  An example of (3) would be the kernel's stack protector which is a little more
> than just -fstack-protector so it needs to be enabled explicitly. An example of
> (4) would be -fsanitize=address vs. KASAN - KASAN is a way more invasive
> operation than gcc's address sanitizer. Note that specifically for the kernel,
> we of course always do (4) - kernel config is provided separately. But the
> kernel is just an easy example that everybody knows.

OK, works for me. Should we write this down somewhere ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [External] Re: Stack protector choices [was: [PATCH v2, 2/2] lxc: fix build without stack protector]

[Matthew Weber]

Thomas,

On Tue, Dec 4, 2018 at 4:09 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> On Tue, 4 Dec 2018 10:35:05 +0100, Arnout Vandecappelle wrote:
>
> >  If you look at it from a practical point of view, we can only do option (1), or
> > "we sometimes enforce the system-level configuration options, but not
> > consistently". Indeed, if a package does the stack protector detection
> > correctly, chances are we'll never even notice that it enables stack protector.
>
> I was going to say that the checksec tool could help us detect such
> situations (i.e SSP is disabled at the Buildroot level, but some
> binaries end up being built with SSP). However, it seems like checksec
> is not checking SSP support, but PIE, RELRO and a few other things.
>

It can look for stack canaries (unsure if it would catch all types of
SSP).  However, checksec takes a lot of time to run readelf on each
executable.

> >  So in practice, I think the policy should be (as it is for other policy
> > options, e.g. debug):
> >
> > 1. If there is no configuration option for it, let the package decide.
> >
> > 2. If there is a configuration option that does the same as our option, disable
> > it and let the toolchain wrapper apply the correct option.

Some package level test/cfg logic may have issues with the toolchain
wrapper enforcing it.  I ran into a case with FORTIFY_SOURCE and the
Kernel toolchain feature test cases where a optimization level must be
provided with the FORTIFY option.  Since they were dropping all flags
and we set the FORTIFY_SOURCE, the build failed with a optimization
level required error.  SSP at least doesn't have that sort of a
dependency.

> >
> > 3. If there is a configuration option and it does something more, enable it
> > automatically based on the global Buildroot option.
> >
> > 4. In very exceptional cases where (3) is even more invasive, offer an option to
> > the user (but only if the toolchain supports it, of course).
> >
> >  An example of (3) would be the kernel's stack protector which is a little more
> > than just -fstack-protector so it needs to be enabled explicitly. An example of
> > (4) would be -fsanitize=address vs. KASAN - KASAN is a way more invasive
> > operation than gcc's address sanitizer. Note that specifically for the kernel,
> > we of course always do (4) - kernel config is provided separately. But the
> > kernel is just an easy example that everybody knows.

(Thomas, let me know if this email was rejected when set to gmail users)

Matt