Crash when mountinfo contains line over 4096 bytes

warptozero commented 2 weeks ago

Description

Firejail crashes with Error: cannot read /proc/self/mountinfo when an overlayfs is mounted with a lot of lowerdirs.

Steps to Reproduce

Use the following fish script to create and mount 100 overlay directories
Run firejail --noprofile bash

overlayfs_test.fish

``` #!/usr/bin/fish sudo umount mnt &>/dev/null rm -r lower upper mkdir -p lower upper work mnt for x in (seq 0 100) set -l d (string repeat -n 30 d) mkdir -p "lower/$d$x" touch "lower/$d$x/file_$x" end set dirs (fd -td -d1 . lower/ | path normalize) set lower (string join --no-empty -- ',lowerdir+=' $dirs) set cmd mount -t overlay overlay -o lowerdir+=$lower,upperdir=upper,workdir=work mnt echo $cmd sudo $cmd tree mnt cat /proc/self/mountinfo | wc -L ```

Note that this uses the newer lowerdir+ syntax to add lowerdirs one by one, which is only supported since kernel version 6.7 or 6.8.

Additional context

It seems to be related to a 4096 byte limit somewhere and not the amount of lowerdirs.

When util-linux in switched to the new kernel mount API with v2.39 the normal mount syntax of appending lowerdirs with lowerdir=d1:d2:d... stopped working for lines longer then 256 bytes. This is why the newer lowerdir+ option is needed.
The old API can still be enabled with LIBMOUNT_FORCE_MOUNT2=always and is limited to a longer option length of max 4096 bytes.
Using the newer lowerdir+ syntax allows to create a much longer entry in /proc/self/mountinfo then was previously possible.

See https://github.com/util-linux/util-linux/issues/2287 and https://github.com/util-linux/util-linux/issues/1992#issuecomment-1486475153.

Environment

Void Linux x86_64 with kernel 6.10.6_1
Firejail version 0.9.72

Compile time support

    - always force nonewprivs support is disabled
    - AppArmor support is enabled
    - AppImage support is enabled
    - chroot support is enabled
    - D-BUS proxy support is enabled
    - file transfer support is enabled
    - firetunnel support is disabled
    - IDS support is disabled
    - networking support is enabled
    - output logging is enabled
    - overlayfs support is disabled
    - private-home support is enabled
    - private-cache and tmpfs as user enabled
    - SELinux support is disabled
    - user namespace support is enabled
    - X11 sandboxing support is enabled

Checklist

[x] The issues is caused by firejail (i.e. running the program by path (e.g. /usr/bin/vlc) "fixes" it).
[x] I can reproduce the issue without custom modifications (e.g. globals.local).
[x] I have performed a short search for similar issues (to avoid opening a duplicate).

Log

Output of env LC_ALL=C firejail --noprofile bash

``` Parent pid 23279, child pid 23280 Error: cannot read /proc/self/mountinfo Error: proc 23279 cannot sync with peer: unexpected EOF Peer 23280 unexpectedly exited with status 1 ```

Output of env LC_ALL=C firejail --debug --noprofile bash

``` Building quoted command line: 'bash' Command name #bash# DISPLAY=:0 parsed as 0 Using the local network stack Parent pid 3458, child pid 3459 Initializing child process Host network configured PID namespace installed Mounting tmpfs on /run/firejail/mnt directory Creating empty /run/firejail/mnt/seccomp directory Creating empty /run/firejail/mnt/seccomp/seccomp.protocol file Creating empty /run/firejail/mnt/seccomp/seccomp.postexec file Creating empty /run/firejail/mnt/seccomp/seccomp.postexec32 file Mounting /proc filesystem representing the PID namespace Basic read-only filesystem: Error: cannot read /proc/self/mountinfo Error: proc 3458 cannot sync with peer: unexpected EOF Peer 3459 unexpectedly exited with status 1 ```

warptozero commented 2 weeks ago

Note that this is not about overlayfs support in firejail, but failing to start when an existing overlayfs is mounted. This might also happen with other mounts that create a very long line in mountinfo.

Unfortunately x-* options don't show up in mountinfo so I haven't found a way to test this yet.

rusty-snake commented 2 weeks ago

# Actually 255 bytes for almost everything except of FAT32, but you know, trailing-nul-byte, some implementations, ...
max_filename="$(head -c 254 < /dev/zero | tr '\0' 'x')"
# (254 "x" + 1 "/") * 16 = 4080; MAX_PATH is 4096
very_long_path="$(for _ in {1..16}; do echo -n "/$max_filename"; done)"
# Create it in a tmpfs, in the worst case you need to hard-reboot but it will not break the next boot if there is some buggy tool.
mkdir -p /tmp$very_long_path
mount -o bind /tmp$very_long_path /tmp$very_long_path
# Test firejail
umount /tmp$very_long_path
rm -rf /tmp/xxx<TAB>

warptozero commented 2 weeks ago

Thanks, I could reproduce this with a long path length.

Looking at the code a quick fix is to simply increase MAX_BUF in mountinfo.c. xargs --show-limits reports the actually used max command buffer as 131072 (128 1024). So to be able to handle a mount command near this size I had to set MAX_BUF to 129 1024.

Starting a firejail with a long mountinfo line of 131112 bytes takes around 241ms instead of the normal 6.4ms without it mounted. So this probably needs a better solution in the parser.

netblue30 / firejail