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.

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

Crash when mountinfo contains line over 4096 bytes #6450

Open
3 tasks done
warptozero opened this issue Aug 25, 2024 · 3 comments
Open
3 tasks done

Crash when mountinfo contains line over 4096 bytes #6450

warptozero opened this issue Aug 25, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@warptozero
Copy link

warptozero commented Aug 25, 2024

Description

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

Steps to Reproduce

  1. Use the following fish script to create and mount 100 overlay directories
  2. 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 util-linux/util-linux#2287 and util-linux/util-linux#1992 (comment).

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

  • The issues is caused by firejail (i.e. running the program by path (e.g. /usr/bin/vlc) "fixes" it).
  • I can reproduce the issue without custom modifications (e.g. globals.local).
  • 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

@kmk3 kmk3 added the overlayfs Issues related to the overlayfs feature (--overlay, etc) (currently unsupported; see #4178) label Aug 25, 2024
@warptozero
Copy link
Author

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
Copy link
Collaborator

# 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
Copy link
Author

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.

@warptozero warptozero changed the title Crash when overlayfs is mounted with long length Crash when mountinfo contains line over 4096 bytes Aug 27, 2024
@kmk3 kmk3 added bug Something isn't working and removed overlayfs Issues related to the overlayfs feature (--overlay, etc) (currently unsupported; see #4178) labels Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants