Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PATH_MAX is undeclared on musl libc #4578

Closed
dm9pZCAq opened this issue Sep 27, 2021 · 1 comment · Fixed by #4583
Closed

PATH_MAX is undeclared on musl libc #4578

dm9pZCAq opened this issue Sep 27, 2021 · 1 comment · Fixed by #4583
Labels
bug Something isn't working
Projects
Release 0.9.68

Comments

@dm9pZCAq
Copy link
Contributor

Description

build failed due to PATH_MAX missed on musl libc

Steps to Reproduce

build on musl libc (voidlinux, alpinelinux, kisslinux)

Environment

  • Gentoo linux with musl profile
  • trying to build firejail-0.9.64.4

Log

main.c:1003:26: error: use of undeclared identifier 'PATH_MAX'
                if (strlen(argv[i]) >= MAX_ARG_LEN) {
                                       ^
./firejail.h:364:22: note: expanded from macro 'MAX_ARG_LEN'
#define MAX_ARG_LEN (PATH_MAX + 32) // --foobar=PATH
                     ^
main.c:1015:23: error: use of undeclared identifier 'PATH_MAX'
                if (strlen(*ptr) >= MAX_ENV_LEN) {
                                    ^
./firejail.h:658:22: note: expanded from macro 'MAX_ENV_LEN'
#define MAX_ENV_LEN (PATH_MAX + 32)     // FOOBAR=SOME_PATH
                     ^

How to fix

here is patch from voidlinux
https://github.com/void-linux/void-packages/blob/master/srcpkgs/firejail/patches/musl.patch

with this patch everything builds fine for me
@kmk3
Copy link
Collaborator

kmk3 commented Sep 27, 2021

@dm9pZCAq commented on Sep 27:

Description

build failed due to PATH_MAX missed on musl libc

Steps to Reproduce

build on musl libc (voidlinux, alpinelinux, kisslinux)

Environment

  • Gentoo linux with musl profile
  • trying to build firejail-0.9.64.4

Log

main.c:1003:26: error: use of undeclared identifier 'PATH_MAX'
                if (strlen(argv[i]) >= MAX_ARG_LEN) {
                                       ^
./firejail.h:364:22: note: expanded from macro 'MAX_ARG_LEN'
#define MAX_ARG_LEN (PATH_MAX + 32) // --foobar=PATH
                     ^
main.c:1015:23: error: use of undeclared identifier 'PATH_MAX'
                if (strlen(*ptr) >= MAX_ENV_LEN) {
                                    ^
./firejail.h:658:22: note: expanded from macro 'MAX_ENV_LEN'
#define MAX_ENV_LEN (PATH_MAX + 32)     // FOOBAR=SOME_PATH
                     ^

Thanks for the report.

How to fix

here is patch from voidlinux
https://github.com/void-linux/void-packages/blob/master/srcpkgs/firejail/patches/musl.patch

with this patch everything builds fine for me

The patch looks good to me; could you open a PR for it?

Cc: @Duncaen (who appears to be the patch author)

kmk3 added a commit to kmk3/firejail that referenced this issue Sep 28, 2021
@kmk3
main.c: remove unnecessary limits.h include
2d6ea74 
Relates to netblue30#4578.
kmk3 added a commit to kmk3/firejail that referenced this issue Sep 28, 2021
@kmk3
Remove unnecessary linux/limits.h include
ac78207 
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 4f003da
("prevent leaking user information by modifying /home directory,
/etc/passwd and /etc/group") and for every other file, it's on commit
1379851 ("Baseline firejail 0.9.28").

Relates to netblue30#4578.
kmk3 added a commit to kmk3/firejail that referenced this issue Sep 28, 2021
@kmk3
Fix wrong limits.h include (should be linux/limits.h)
f4171df 
ARG_MAX and PATH_MAX are defined on POSIX.1-2017's limits.h(0p)[1], on
linux/limits.h and on musl's limits.h, but not on glibc's limits.h (nor
on any of its includes):

    $ grep -F -e ' ARG_MAX' -e ' PATH_MAX' /usr/include/limits.h \
      /usr/include/linux/limits.h /usr/lib/musl/include/limits.h
    /usr/include/linux/limits.h:#define ARG_MAX       131072        /* # bytes of args + environ for exec() */
    /usr/include/linux/limits.h:#define PATH_MAX        4096        /* # chars in a path name including nul */
    /usr/lib/musl/include/limits.h:#define PATH_MAX 4096
    /usr/lib/musl/include/limits.h:#define ARG_MAX 131072

Environment:

    $ grep '^NAME' /etc/os-release
    NAME="Artix Linux"
    $ pacman -Qo /usr/include/limits.h /usr/include/linux/limits.h \
      /usr/lib/musl/include/limits.h
    /usr/include/limits.h is owned by glibc 2.33-5
    /usr/include/linux/limits.h is owned by linux-api-headers 5.12.3-1
    /usr/lib/musl/include/limits.h is owned by musl 1.2.2-1

Files that use the macros:

    $ git grep -Fl -e ARG_MAX -e PATH_MAX -- src
    src/firejail/cmdline.c
    src/firejail/firejail.h
    src/libtrace/libtrace.c
    src/libtracelog/libtracelog.c

Note: No other macro from linux/limits.h appears to be used.

Note2: The build works even without including any limits.h on the files
that this commit changes, so there is probably another include on them
that ends up defining ARG_MAX/PATH_MAX.

Relates to netblue30#4578.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html
kmk3 added a commit to kmk3/firejail that referenced this issue Sep 28, 2021
@kmk3
firejail.h: add missing linux/limits.h include
e228db3 
It uses PATH_MAX.

Fixes netblue30#4578.
This was referenced Sep 28, 2021
Merged
Merged
kmk3 added a commit to kmk3/firejail that referenced this issue Oct 1, 2021
@kmk3
firejail.h: add missing linux/limits.h include
579f856 
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 <limits.h> 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 <linux/limits.h>
    /* [...] */
    /* 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 netblue30#4578.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html
[2] https://sourceware.org/git/?p=glibc.git;a=commit;h=f96853beafc26d4f030961b0b67a79b5bfad5733
netblue30 added a commit that referenced this issue Oct 9, 2021
@netblue30
Merge pull request #4579 from dm9pZCAq/master
c57199a 
fix compilation on musl (#4578)
@kmk3 kmk3 added the bug Something isn't working label Jan 27, 2022
@kmk3 kmk3 added this to To do in Release 0.9.68 via automation Jan 27, 2022
@kmk3 kmk3 moved this from To do to To Document (RELNOTES/man) in Release 0.9.68 Jan 27, 2022
@kmk3 kmk3 changed the title PATH_MAX missed on musl libc PATH_MAX is undeclared on musl libc Feb 1, 2022
@kmk3 kmk3 changed the title PATH_MAX is undeclared on musl libc PATH_MAX is undeclared on musl libc Feb 1, 2022
kmk3 added a commit that referenced this issue Feb 3, 2022
@kmk3
RELNOTES: add more bugfixes
54d8d58 
Relates to #3958 #4362 #4365 #4476 #4578 #4579 #4583.
@kmk3 kmk3 moved this from To Document (RELNOTES/man) to Done (on RELNOTES) in Release 0.9.68 Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
No open projects
Release 0.9.68
  
Done (on RELNOTES)
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

firejail.h: add missing linux/limits.h include & include cleanup kmk3/firejail
2 participants
@kmk3 @dm9pZCAq