| Message ID | 20190219223530.15956-1-christian@paral.in |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/3] runc: depend on linux headers >= 3.11 for O_TMPFILE | expand |
Hello Christian, On Tue, 19 Feb 2019 14:35:28 -0800 Christian Stewart <christian@paral.in> wrote: > Add dependency on headers >= 3.11. O_TMPFILE, required by runc, was not > available until this version. > > Fixes: > > http://autobuild.buildroot.net/results/63e9d88ae5177541be463f1e2aafec59aa410479 I don't see how it can fix this build issue, because the defconfig for this failure has: BR2_TOOLCHAIN_HEADERS_AT_LEAST="3.12" so it *is* using kernel headers more recent than 3.11. I had a look at your build issue following your e-mail yesterday, and indeed O_TMPFILE was added in the kernel headers in 3.11, but apparently having 3.11 kernel headers is not sufficient. I suppose some code is also needed in the C library, and because the toolchain used by this configuration has an old glibc, it doesn't have everything needed for O_TMPFILE. So, your patch is OK in the sense that indeed >= 3.11 is needed, but it is wrong in the sense that it is not going to fix the build issue you're pointing to. Best regards, Thomas
On 20/02/2019 09:01, Thomas Petazzoni wrote: > Hello Christian, > > On Tue, 19 Feb 2019 14:35:28 -0800 > Christian Stewart <christian@paral.in> wrote: > >> Add dependency on headers >= 3.11. O_TMPFILE, required by runc, was not >> available until this version. >> >> Fixes: >> >> http://autobuild.buildroot.net/results/63e9d88ae5177541be463f1e2aafec59aa410479 > > I don't see how it can fix this build issue, because the defconfig for > this failure has: > > BR2_TOOLCHAIN_HEADERS_AT_LEAST="3.12" > > so it *is* using kernel headers more recent than 3.11. > > I had a look at your build issue following your e-mail yesterday, and > indeed O_TMPFILE was added in the kernel headers in 3.11, but > apparently having 3.11 kernel headers is not sufficient. I suppose some > code is also needed in the C library, and because the toolchain used by > this configuration has an old glibc, it doesn't have everything needed > for O_TMPFILE. Darn. So how do we handle that? We don't have anything for GLIBC_AT_LEAST (much less MUSL_AT_LEAST or UCLIBC_AT_LEAST...). I don't really want to start adding *those* either... > > So, your patch is OK in the sense that indeed >= 3.11 is needed, but it > is wrong in the sense that it is not going to fix the build issue > you're pointing to. If it doesn't fix the autobuild issue, it's kind of useless, so I've marked this patch as Changes Requested. Regards, Arnout
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes: > Hello Christian, > On Tue, 19 Feb 2019 14:35:28 -0800 > Christian Stewart <christian@paral.in> wrote: >> Add dependency on headers >= 3.11. O_TMPFILE, required by runc, was not >> available until this version. >> >> Fixes: >> >> http://autobuild.buildroot.net/results/63e9d88ae5177541be463f1e2aafec59aa410479 > I don't see how it can fix this build issue, because the defconfig for > this failure has: > BR2_TOOLCHAIN_HEADERS_AT_LEAST="3.12" > so it *is* using kernel headers more recent than 3.11. > I had a look at your build issue following your e-mail yesterday, and > indeed O_TMPFILE was added in the kernel headers in 3.11, but > apparently having 3.11 kernel headers is not sufficient. I suppose some > code is also needed in the C library, and because the toolchain used by > this configuration has an old glibc, it doesn't have everything needed > for O_TMPFILE. Indeed, O_TMPFILE support was only added to glibc in 2.19 with the following commit: commit ffdd31816a67f48697ea4d6b852e58d2886d42ca Author: Andreas Schwab <schwab@suse.de> Date: Wed Sep 11 11:15:45 2013 +0200 Add O_TMPFILE to <fcntl.h> And to musl in 0.9.15 with the following commit: commit f7d348ec39ce31efdc4963eb4a8f16f48e5ef095 Author: Szabolcs Nagy <nsz@port70.net> Date: Sat Nov 23 23:47:48 2013 +0000 add O_TMPFILE flag, new in linux 3.11 definition in linux: #define O_TMPFILE (__O_TMPFILE | O_DIRECTORY) where __O_TMPFILE and O_DIRECTORY are arch specific We have already dropped support for glibc <= 2.17 back in 2017.08, perhaps we should bump that to <= 2.18 now? A number of our preconfigured toolchains do use glibc 2.18 though, E.G. the codesourcery / mentor ARM toolchain and a number of the toolchains we use on the autobuilders.
Hello, On Mon, 25 Feb 2019 23:59:13 +0100 Peter Korsgaard <peter@korsgaard.com> wrote: > Indeed, O_TMPFILE support was only added to glibc in 2.19 with the > following commit: > > commit ffdd31816a67f48697ea4d6b852e58d2886d42ca > Author: Andreas Schwab <schwab@suse.de> > Date: Wed Sep 11 11:15:45 2013 +0200 > > Add O_TMPFILE to <fcntl.h> > > And to musl in 0.9.15 with the following commit: > > commit f7d348ec39ce31efdc4963eb4a8f16f48e5ef095 > Author: Szabolcs Nagy <nsz@port70.net> > Date: Sat Nov 23 23:47:48 2013 +0000 > > add O_TMPFILE flag, new in linux 3.11 > > definition in linux: > #define O_TMPFILE (__O_TMPFILE | O_DIRECTORY) > where __O_TMPFILE and O_DIRECTORY are arch specific > > We have already dropped support for glibc <= 2.17 back in 2017.08, > perhaps we should bump that to <= 2.18 now? > > A number of our preconfigured toolchains do use glibc 2.18 though, > E.G. the codesourcery / mentor ARM toolchain and a number of the > toolchains we use on the autobuilders. While I understand the maintenance burden that it causes, I would like to try to keep support for fairly old toolchains. Even if it seems crazy, a number of companies are still using oldish toolchains (typically provided by SoC vendors) to build stuff with relatively recent Buildroot versions. It is becoming more and more difficult due to the use of C++11 in many userspace components (which requires gcc >= 4.8), but still. So we should be careful about bumping our glibc version requirements in a global way, when only a package like "runc" is affected, which serves some very specific purpose. Can we work around the lack of O_TMPFILE in the C library directly in the package source code, and then rely on a >= 3.11 kernel headers dependency ? Systemd has this logic in missing_fcntl.h: /* The precise definition of __O_TMPFILE is arch specific; use the * values defined by the kernel (note: some are hexa, some are octal, * duplicated as-is from the kernel definitions): * - alpha, parisc, sparc: each has a specific value; * - others: they use the "generic" value. */ #ifndef __O_TMPFILE #if defined(__alpha__) #define __O_TMPFILE 0100000000 #elif defined(__parisc__) || defined(__hppa__) #define __O_TMPFILE 0400000000 #elif defined(__sparc__) || defined(__sparc64__) #define __O_TMPFILE 0x2000000 #else #define __O_TMPFILE 020000000 #endif #endif /* a horrid kludge trying to make sure that this will fail on old kernels */ #ifndef O_TMPFILE #define O_TMPFILE (__O_TMPFILE | O_DIRECTORY) #endif I suppose the first part, that defines __O_TMPFILE is only needed if you have kernel headers < 3.11, so we would not need that. Only the second part would be needed ? Best regards, Thomas
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes: Hi, >> We have already dropped support for glibc <= 2.17 back in 2017.08, >> perhaps we should bump that to <= 2.18 now? >> >> A number of our preconfigured toolchains do use glibc 2.18 though, >> E.G. the codesourcery / mentor ARM toolchain and a number of the >> toolchains we use on the autobuilders. > While I understand the maintenance burden that it causes, I would like > to try to keep support for fairly old toolchains. Even if it seems > crazy, a number of companies are still using oldish toolchains > (typically provided by SoC vendors) to build stuff with relatively > recent Buildroot versions. It is becoming more and more difficult due > to the use of C++11 in many userspace components (which requires gcc >= > 4.8), but still. So we should be careful about bumping our glibc > version requirements in a global way, when only a package like "runc" > is affected, which serves some very specific purpose. I understand that dropping support for stuff isn't nice for the users. Just to make sure I understand the use case - These people are using old toolchains for some reason but uptodate Linux kernels? Why can the kernel be updated but not the toolchain? If they are also using old kernels, then fixing up these things will just lead to runtime errors instead, which is even worse. Last time I was stuck with an ancient BSP, I even had to backport new syscalls to build and use modern user space (I believe it was accept4 or recvmmsg). I could imagine more software starting to use O_TMPFILE in the future as well. In general, mixing components of different vintage is a recipe for pain. > Can we work around the lack of O_TMPFILE in the C library directly in > the package source code, and then rely on a >= 3.11 kernel headers > dependency ? > Systemd has this logic in missing_fcntl.h: I also had a look at doing that, but it is not completely trivial as the definitions are arch specific (as you show) and fcntl.h (on glibc at least) does not include asm/fcntl.h According to https://lwn.net/Articles/619146/, glibc also used to forward arguments wrong to the open syscall: https://sourceware.org/bugzilla/show_bug.cgi?id=17523 So given how security sensitive this is, I'm wary of patching it up - I don't want to end up like Debian with the openssl patch :/ As the recent iproute2 patch showed, we do still have users on glibc <= 2.17 even though we do not officially support it, and do accept patches when they are simple - But people are on their own if some package doesn't build with such old toolchains, so the real impact of dropping official support for glibc 2.18 as well is not so big, E.G. it would just mean that we drop the preconfigured toolchains using it and stop testing on the autobuilders.
Hello, On Tue, 26 Feb 2019 09:33:25 +0100 Peter Korsgaard <peter@korsgaard.com> wrote: > I understand that dropping support for stuff isn't nice for the users. > > Just to make sure I understand the use case - These people are using old > toolchains for some reason but uptodate Linux kernels? Why can the > kernel be updated but not the toolchain? To be honest, for the one customer I had in mind that still uses ancient toolchains, I don't know which kernel version they are using. Presumably something old, but I'm not sure how old. However, the thing here is that we're talking about an issue in "runc", which is a fairly modern piece of software, unlikely to be used by those companies that are stuck with old toolchains/kernels. What bothers me is that we are going to raise our glibc version requirement just because some modern/fancy/new software package like "runc" needs it, even though it is practically not going to impact those users of old toolchains/kernels that most likely don't use runc. > As the recent iproute2 patch showed, we do still have users on glibc <= > 2.17 even though we do not officially support it, and do accept patches > when they are simple - But people are on their own if some package > doesn't build with such old toolchains, so the real impact of dropping > official support for glibc 2.18 as well is not so big, E.G. it would > just mean that we drop the preconfigured toolchains using it and stop > testing on the autobuilders. Dropping for the autobuilders just because of runc means we are not going to catch any other build issue caused by glibc <= 2.18, some of which might easily be fixed, which isn't really nice :-/ Should we just bite the bullet and have BR2_TOOLCHAIN_GLIBC_HAS_AT_LEAST_X_Y ? It's annoying because of glibc/uclibc/musl, but well... Thomas
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes: Hi, >> Just to make sure I understand the use case - These people are using old >> toolchains for some reason but uptodate Linux kernels? Why can the >> kernel be updated but not the toolchain? > To be honest, for the one customer I had in mind that still uses > ancient toolchains, I don't know which kernel version they are using. > Presumably something old, but I'm not sure how old. ;) > However, the thing here is that we're talking about an issue in "runc", > which is a fairly modern piece of software, unlikely to be used by > those companies that are stuck with old toolchains/kernels. What > bothers me is that we are going to raise our glibc version requirement > just because some modern/fancy/new software package like "runc" needs > it, even though it is practically not going to impact those users of > old toolchains/kernels that most likely don't use runc. Correct, but what then when the next package breaks, E.G. systemd, eudev, kodi, .. ? >> As the recent iproute2 patch showed, we do still have users on glibc <= >> 2.17 even though we do not officially support it, and do accept patches >> when they are simple - But people are on their own if some package >> doesn't build with such old toolchains, so the real impact of dropping >> official support for glibc 2.18 as well is not so big, E.G. it would >> just mean that we drop the preconfigured toolchains using it and stop >> testing on the autobuilders. > Dropping for the autobuilders just because of runc means we are not > going to catch any other build issue caused by glibc <= 2.18, some of > which might easily be fixed, which isn't really nice :-/ It is glibc == 2.18, not <=. We already dropped support for 2.17 and older and (shouldn't be) testing such old toolchains any more. I'm not sure how this differs from how we dropped 2.17 support back in 2017.08? glibc 2.18 was released just 6 months after 2.17 and we are now close to 2 years later. > Should we just bite the bullet and have > BR2_TOOLCHAIN_GLIBC_HAS_AT_LEAST_X_Y ? It's annoying because of > glibc/uclibc/musl, but well... I would prefer to just not support such old toolchains instead of this extra complexity.
>>>>> "Christian" == Christian Stewart <christian@paral.in> writes: > Add dependency on headers >= 3.11. O_TMPFILE, required by runc, was not > available until this version. > Fixes: > http://autobuild.buildroot.net/results/63e9d88ae5177541be463f1e2aafec59aa410479 > Signed-off-by: Christian Stewart <christian@paral.in> > --- > package/runc/Config.in | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > diff --git a/package/runc/Config.in b/package/runc/Config.in > index 47c850ef30..44713222e4 100644 > --- a/package/runc/Config.in > +++ b/package/runc/Config.in > @@ -3,6 +3,7 @@ config BR2_PACKAGE_RUNC > depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS > depends on BR2_PACKAGE_HOST_GO_CGO_LINKING_SUPPORTS > depends on BR2_TOOLCHAIN_HAS_THREADS > + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_11 # O_TMPFILE > depends on !BR2_TOOLCHAIN_USES_UCLIBC # no fexecve > help > runC is a CLI tool for spawning and running containers > @@ -10,7 +11,8 @@ config BR2_PACKAGE_RUNC > https://github.com/opencontainers/runc > -comment "runc needs a glibc or musl toolchain toolchain w/ threads" > +comment "runc needs a glibc or musl toolchain w/ threads, headers >= 3.11" > depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS && \ > BR2_PACKAGE_HOST_GO_CGO_LINKING_SUPPORTS > - depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_TOOLCHAN_USES_UCLIBC > + depends on !BR2_TOOLCHAIN_HAS_THREADS || \ > + !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_11 || BR2_TOOLCHAN_USES_UCLIBC This line should be indented by a tab as pointed out by check-package. Committed series after squashing into a single commit and extending the commit message to explain when this issue was introduced and the C library dependencies, thanks.
diff --git a/package/runc/Config.in b/package/runc/Config.in index 47c850ef30..44713222e4 100644 --- a/package/runc/Config.in +++ b/package/runc/Config.in @@ -3,6 +3,7 @@ config BR2_PACKAGE_RUNC depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS depends on BR2_PACKAGE_HOST_GO_CGO_LINKING_SUPPORTS depends on BR2_TOOLCHAIN_HAS_THREADS + depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_11 # O_TMPFILE depends on !BR2_TOOLCHAIN_USES_UCLIBC # no fexecve help runC is a CLI tool for spawning and running containers @@ -10,7 +11,8 @@ config BR2_PACKAGE_RUNC https://github.com/opencontainers/runc -comment "runc needs a glibc or musl toolchain toolchain w/ threads" +comment "runc needs a glibc or musl toolchain w/ threads, headers >= 3.11" depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS && \ BR2_PACKAGE_HOST_GO_CGO_LINKING_SUPPORTS - depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_TOOLCHAN_USES_UCLIBC + depends on !BR2_TOOLCHAIN_HAS_THREADS || \ + !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_11 || BR2_TOOLCHAN_USES_UCLIBC
Add dependency on headers >= 3.11. O_TMPFILE, required by runc, was not available until this version. Fixes: http://autobuild.buildroot.net/results/63e9d88ae5177541be463f1e2aafec59aa410479 Signed-off-by: Christian Stewart <christian@paral.in> --- package/runc/Config.in | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)