From 2d6ea74d9586179e6dc250f14fc3b7321e03999e Mon Sep 17 00:00:00 2001 From: "Kelvin M. Klann" Date: Mon, 27 Sep 2021 23:42:08 -0300 Subject: [PATCH 1/3] main.c: remove unnecessary limits.h include Relates to #4578. --- src/firejail/main.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/firejail/main.c b/src/firejail/main.c index 81d148257af..a99249be9c3 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -32,7 +32,6 @@ #include #include #include -//#include #include #include #include From ac78207f7c578cb1464bf7abcf9f605a675a7164 Mon Sep 17 00:00:00 2001 From: "Kelvin M. Klann" Date: Mon, 27 Sep 2021 23:42:43 -0300 Subject: [PATCH 2/3] Remove unnecessary linux/limits.h include None of the files affected use any macros from linux/limits.h: $ git grep -Fl 'NGROUPS_MAX ARG_MAX LINK_MAX MAX_CANON MAX_INPUT NAME_MAX PATH_MAX PIPE_BUF XATTR_NAME_MAX XATTR_SIZE_MAX XATTR_LIST_MAX RTSIG_MAX' -- src src/firejail/cmdline.c src/firejail/firejail.h src/libtrace/libtrace.c src/libtracelog/libtracelog.c Environment: $ grep '^NAME' /etc/os-release NAME="Artix Linux" $ pacman -Qo /usr/include/linux/limits.h /usr/include/linux/limits.h is owned by linux-api-headers 5.12.3-1 Note: This include has been present on all of the affected files since their inception. For restrict_users.c, that's on commit 4f003daec ("prevent leaking user information by modifying /home directory, /etc/passwd and /etc/group") and for every other file, it's on commit 137985136 ("Baseline firejail 0.9.28"). Relates to #4578. --- src/firejail/fs.c | 1 - src/firejail/fs_dev.c | 1 - src/firejail/fs_home.c | 1 - src/firejail/fs_hostname.c | 1 - src/firejail/fs_trace.c | 1 - src/firejail/fs_var.c | 1 - src/firejail/restrict_users.c | 1 - 7 files changed, 7 deletions(-) diff --git a/src/firejail/fs.c b/src/firejail/fs.c index dd4c2139d85..7e0a6e347a1 100644 --- a/src/firejail/fs.c +++ b/src/firejail/fs.c @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include diff --git a/src/firejail/fs_dev.c b/src/firejail/fs_dev.c index 8cc3ecc627e..d8bb1aded15 100644 --- a/src/firejail/fs_dev.c +++ b/src/firejail/fs_dev.c @@ -20,7 +20,6 @@ #include "firejail.h" #include #include -#include #include #include #include diff --git a/src/firejail/fs_home.c b/src/firejail/fs_home.c index 0ed47606366..45889b27f95 100644 --- a/src/firejail/fs_home.c +++ b/src/firejail/fs_home.c @@ -19,7 +19,6 @@ */ #include "firejail.h" #include -#include #include #include #include diff --git a/src/firejail/fs_hostname.c b/src/firejail/fs_hostname.c index 7d320e90b12..f7ce8c18fe5 100644 --- a/src/firejail/fs_hostname.c +++ b/src/firejail/fs_hostname.c @@ -20,7 +20,6 @@ #include "firejail.h" #include #include -#include #include #include #include diff --git a/src/firejail/fs_trace.c b/src/firejail/fs_trace.c index 475a391ecbd..9463fbcd0c4 100644 --- a/src/firejail/fs_trace.c +++ b/src/firejail/fs_trace.c @@ -20,7 +20,6 @@ #include "firejail.h" #include #include -#include #include #include #include diff --git a/src/firejail/fs_var.c b/src/firejail/fs_var.c index 20e262d80ba..5ba38d46cb7 100644 --- a/src/firejail/fs_var.c +++ b/src/firejail/fs_var.c @@ -20,7 +20,6 @@ #include "firejail.h" #include #include -#include #include #include #include diff --git a/src/firejail/restrict_users.c b/src/firejail/restrict_users.c index 6f17231a4a0..59077dadae7 100644 --- a/src/firejail/restrict_users.c +++ b/src/firejail/restrict_users.c @@ -21,7 +21,6 @@ #include "../include/firejail_user.h" #include #include -#include #include #include #include From 579f856c56c41153a45ae3529224a01babf2aa6a Mon Sep 17 00:00:00 2001 From: "Kelvin M. Klann" Date: Mon, 27 Sep 2021 23:51:36 -0300 Subject: [PATCH 3/3] firejail.h: add missing linux/limits.h include firejail.h uses PATH_MAX when defining a macro. Note that ARG_MAX and PATH_MAX are not guaranteed to be (and potentially should not be) defined. From POSIX.1-2017's limits.h(0p)[1]: > A definition of one of the symbolic constants in the following list > shall be omitted from the header on specific > implementations where the corresponding value is equal to or greater > than the stated minimum, but where the value can vary depending on the > file to which it is applied. The actual value supported for a > specific pathname shall be provided by the pathconf() function. Use linux/limits.h instead of limits.h because glibc's limits.h deliberately undefines ARG_MAX. See glibc commit f96853beaf ("* sysdeps/unix/sysv/linux/bits/local_lim.h: Undefined ARG_MAX if", 2008-03-27)[2]. From /usr/include/bits/local_lim.h (glibc 2.33-5 on Artix Linux): #ifndef ARG_MAX # define __undef_ARG_MAX #endif /* The kernel sources contain a file with all the needed information. */ #include /* [...] */ /* Have to remove ARG_MAX? */ #ifdef __undef_ARG_MAX # undef ARG_MAX # undef __undef_ARG_MAX #endif So if a file uses ARG_MAX (currently only cmdline.c) and limits.h (or a firejail.h that includes limits.h) is included before linux/limits.h, then the build will fail on glibc. Build log from using limits.h (instead of linux/limits.h) on firejail.h: $ make clean >/dev/null && make >/dev/null cmdline.c:145:12: error: use of undeclared identifier 'ARG_MAX'; did you mean 'CFG_MAX'? if (len > ARG_MAX) { ^~~~~~~ CFG_MAX ./firejail.h:805:2: note: 'CFG_MAX' declared here CFG_MAX // this should always be the last entry ^ [...] Fixes #4578. [1] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html [2] https://sourceware.org/git/?p=glibc.git;a=commit;h=f96853beafc26d4f030961b0b67a79b5bfad5733 --- src/firejail/firejail.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index 2a7d8857558..f554a32044f 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -22,6 +22,7 @@ #include "../include/common.h" #include "../include/euid_common.h" #include "../include/rundefs.h" +#include // Note: Plain limits.h may break ARG_MAX (see #4583) #include #include