Skip to contentSkip to conversation

lxc: fix missing include for va_list

PATCH Superseded Fabrice Fontaine 5 participants 10 messages Dec 3, 2018 – Dec 4, 2018

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/2 lxc: fix missing include for va_list

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

---
 ...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
package/lxc/0001-stringutils-include-stdarg-for-va_list.patch new file +30 -0
new file mode 100644
index 0000000000..d0a450e8cf
@@ -0,0 +1,30 @@
+From b7df06ad14c04f18b7db5b64d5142b802bf64cb2 Mon Sep 17 00:00:00 2001
+From: Fabrice Fontaine <[email protected]>
+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 <[email protected]>
+[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
+
Signature · 1 line
2.14.1
#1 lore raw

PATCH v2 2/2 lxc: fix build without stack protector

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

---
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(+)
package/lxc/lxc.mk +2 -0
index 48d5b20329..0b7b03bcac 100644
@@ -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)
Signature · 1 line
2.14.1
#2 lore raw

PATCH v2 2/2 lxc: fix build without stack protector

Hi Fabrice,

Quoted 23 lines · 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

---
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(+)
package/lxc/lxc.mk +0 -0
index 48d5b20329..0b7b03bcac 100644

package/lxc/lxc.mk LXC_LICENSE_FILES = COPYING · line 14

Quoted 4 lines
package/lxc/lxc.mk +1 -0
@@ -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.

package/lxc/lxc.mk LXC_LICENSE_FILES = COPYING · line 18

Quoted 4 lines
package/lxc/lxc.mk +1 -0
+
 LXC_CONF_OPTS = --disable-apparmor --with-distro=buildroot \
 	--disable-werror \
 	$(if $(BR2_PACKAGE_BASH),,--disable-bash)

baruch

Signature · 3 lines
     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 -
#3 lore raw

PATCH v2 2/2 lxc: fix build without stack protector

Hello,

+Arnout, Peter, Yann in Cc.

Quoted 4 lines · Baruch Siach
Quoted 1 line

+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

Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
#4 lore raw

PATCH v2 2/2 lxc: fix build without stack protector

Hi Thomas,

Quoted 13 lines · Thomas Petazzoni

+Arnout, Peter, Yann in Cc.

Quoted 4 lines · Baruch Siach
Quoted 1 line

+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} ?

Quoted 7 lines

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

Signature · 3 lines
     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 -
#5 lore raw

PATCH v2 2/2 lxc: fix build without stack protector

Hello,

Quoted 9 lines · Baruch Siach
Quoted 9 lines

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) ?

Quoted 7 lines
Quoted 2 lines

- 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

Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
#6 lore raw

PATCH v2 2/2 lxc: fix build without stack protector

Hi Thomas,

Quoted 21 lines · Thomas Petazzoni
Quoted 9 lines · Baruch Siach
Quoted 9 lines

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) ?

Quoted 7 lines
Quoted 2 lines

- 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.

Quoted 8 lines

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

Signature · 3 lines
     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 -
#7 lore raw

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

In reply to Thomas Petazzoni
Quoted 15 lines · Thomas Petazzoni

Hello,

+Arnout, Peter, Yann in Cc.

Quoted 4 lines · Baruch Siach
Quoted 1 line

+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} ?

Quoted 7 lines

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

#8 lore raw

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

Hello,

Quoted 4 lines · Arnout Vandecappelle

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.

Quoted 14 lines

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).

Quoted 6 lines

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

Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
#9 lore raw

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

Thomas,

Quoted 10 lines · Thomas Petazzoni

Hello,

Quoted 4 lines · Arnout Vandecappelle

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.

Quoted 5 lines

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.

Quoted 7 lines
Quoted 7 lines

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.

Quoted 7 lines
Quoted 7 lines

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).

Quoted 6 lines
Quoted 6 lines

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