-
Notifications
You must be signed in to change notification settings - Fork 567
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
landlock: "Invalid argument" error when creating the ruleset #6195
Comments
Cannot reproduce it on Artix with: firejail --profile=firefox true What is the command-line used? What is the kernel version? PR #6187 has Landlock-related changes but it did not change the full ruleset. Can you try to bisect? |
I can (fully) reproduce on my Arch Linux. Will try to find some time to bisect. |
Nothing special. Just
6.7.3-arch1-2 The latest 2 commits didn't change anything:
|
Bisecting shows 760f50f as the first commit where this starts to show. As it happens that is the commit that introduced |
Are the firefox profile changes needed to reproduce the errors (other than Could you run the following in 760f50f and post the output? firejail --debug --profile=firefox --landlock true |
Does it work without the profile changes (but with Could you run the following and post the output in a gist? firejail --debug --profile=firefox --landlock.enforce true At least from the |
Negative. The errors show, even when there's only one line in the firefox.local:
Here are the logs.
|
Recently (as of Landlock ABI 4), the `handled_access_net` field was added to the `landlock_ruleset_attr` struct in the Linux kernel (in linux/landlock.h). In src/firejail/landlock.c, that field is not being set in the struct (as we currently do not use it) before passing it to the `landlock_create_full_ruleset` syscall, so it may contain random garbage when used, resulting in the syscall sometimes returning EINVAL (depending on whether the garbage is valid)[1]: ll_is_supported: Detected Landlock ABI version 4 ll_restrict: Starting Landlock restrict ll_create_full_ruleset: Creating Landlock ruleset (abi=4 fs=1fff) ll_create_full_ruleset: Error: failed to create Landlock ruleset (abi=4 fs=1fff): Invalid argument ll_read: Adding Landlock rule (abi=4 fs=c) for / Error: ll_read: failed to add Landlock rule (abi=4 fs=c) for /: Bad file descriptor ll_create_full_ruleset: Creating Landlock ruleset (abi=4 fs=1fff) ll_create_full_ruleset: Error: failed to create Landlock ruleset (abi=4 fs=1fff): Invalid argument ll_read: Adding Landlock rule (abi=4 fs=c) for /proc Error: ll_read: failed to add Landlock rule (abi=4 fs=c) for /proc: Bad file descriptor [...] So ensure that all structs in landlock.c are initialized to 0 before using them. Note: This currently affects Arch but not Artix, as the former packages a more recent version of the Linux headers (linux-api-headers 6.7-1 vs 6.4-1). Fixes netblue30#6195. Relates to netblue30#6078. [1] netblue30#6195 (comment)
Recently (as of Landlock ABI 4), the `handled_access_net` field was added to the `landlock_ruleset_attr` struct in the Linux kernel (in linux/landlock.h). In src/firejail/landlock.c, that field is not being set in the struct (as we currently do not use it) before passing it to the `landlock_create_full_ruleset` syscall, so it may contain random garbage when used, resulting in the syscall sometimes returning EINVAL (depending on whether the garbage is valid)[1]: ll_is_supported: Detected Landlock ABI version 4 ll_restrict: Starting Landlock restrict ll_create_full_ruleset: Creating Landlock ruleset (abi=4 fs=1fff) ll_create_full_ruleset: Error: failed to create Landlock ruleset (abi=4 fs=1fff): Invalid argument ll_read: Adding Landlock rule (abi=4 fs=c) for / Error: ll_read: failed to add Landlock rule (abi=4 fs=c) for /: Bad file descriptor ll_create_full_ruleset: Creating Landlock ruleset (abi=4 fs=1fff) ll_create_full_ruleset: Error: failed to create Landlock ruleset (abi=4 fs=1fff): Invalid argument ll_read: Adding Landlock rule (abi=4 fs=c) for /proc Error: ll_read: failed to add Landlock rule (abi=4 fs=c) for /proc: Bad file descriptor [...] So ensure that all structs in landlock.c are initialized to 0 before using them. Note: Arch has recently (2024-01-31) updated the linux-api-headers package from version 6.4-1 to 6.7-1[2]. The former version is not affected (as it does not contain the extra struct field in linux/landlock.h), while the latter is. Fixes netblue30#6195. Relates to netblue30#6078. [1] netblue30#6195 (comment) [2] https://gitlab.archlinux.org/archlinux/packaging/packages/linux-api-headers/-/commit/b4223b0c2bfba54c26acc4dc289415b81b15989f
Recently (as of Landlock ABI 4), the `handled_access_net` field was added to the `landlock_ruleset_attr` struct in the Linux kernel (in linux/landlock.h). In src/firejail/landlock.c, that field is not being set in the struct (as we currently do not use it) before passing it to the `landlock_create_full_ruleset` syscall, so it is likely to contain random garbage when used, resulting in the syscall returning EINVAL: $ firejail --debug --profile=/etc/firejail/landlock-common.inc \ --landlock.enforce true [...] ll_is_supported: Detected Landlock ABI version 4 ll_restrict: Starting Landlock restrict ll_create_full_ruleset: Creating Landlock ruleset (abi=4 fs=1fff) Error: ll_create_full_ruleset: failed to create Landlock ruleset (abi=4 fs=1fff): Invalid argument ll_read: Adding Landlock rule (abi=4 fs=c) for / Error: ll_read: failed to add Landlock rule (abi=4 fs=c) for /: Bad file descriptor [...] Not enforcing Landlock So ensure that all structs in src/firejail/landlock.c are initialized to 0 before using them. Note: Arch has recently (2024-01-31) updated the linux-api-headers package from version 6.4-1 to 6.7-1[1]. The former version is not affected (as it does not contain the extra struct field in linux/landlock.h), while the latter is. Fixes netblue30#6195. Relates to netblue30#6078. [1] https://gitlab.archlinux.org/archlinux/packaging/packages/linux-api-headers/-/commit/b4223b0c2bfba54c26acc4dc289415b81b15989f Reported-by: @curiosityseeker
The only thing that I could imagine being an invalid argument in that syscall It should be fixed in #6200. Thanks for reporting/testing/bisecting. |
Recently (as of Landlock ABI 4), the `handled_access_net` field was added to the `landlock_ruleset_attr` struct in the Linux kernel (in linux/landlock.h). In src/firejail/landlock.c, that field is not being set in the struct (as we currently do not use it) before passing it to the `landlock_create_full_ruleset` syscall, so it is likely to contain random garbage when used, resulting in the syscall returning EINVAL: $ firejail --debug --profile=/etc/firejail/landlock-common.inc \ --landlock.enforce true [...] ll_is_supported: Detected Landlock ABI version 4 ll_restrict: Starting Landlock restrict ll_create_full_ruleset: Creating Landlock ruleset (abi=4 fs=1fff) Error: ll_create_full_ruleset: failed to create Landlock ruleset (abi=4 fs=1fff): Invalid argument ll_read: Adding Landlock rule (abi=4 fs=c) for / Error: ll_read: failed to add Landlock rule (abi=4 fs=c) for /: Bad file descriptor [...] Not enforcing Landlock So ensure that all structs in src/firejail/landlock.c are initialized to 0 before using them. Note: Arch has recently (2024-01-31) updated the linux-api-headers package from version 6.4-1 to 6.7-1[1]. The former version is not affected (as it does not contain the extra struct field in linux/landlock.h), while the latter is. Fixes #6195. Relates to #6078. [1] https://gitlab.archlinux.org/archlinux/packaging/packages/linux-api-headers/-/commit/b4223b0c2bfba54c26acc4dc289415b81b15989f Reported-by: @curiosityseeker
Cool! I can confirm that that commit fixed the Issue:
Thanks a lot, @kmk3 ! |
Description
After adding several Landlock rules I'm seeing errors after today's update of firejail-git
Steps to Reproduce
Steps to reproduce the behavior
Add the following rules to ~/.config/firejail/firefox:
Expected behavior
Until yesterday I haven't seen Landlock-related errors.
Actual behavior
Environment
The text was updated successfully, but these errors were encountered: