Re: [PATCH v3 2/4] landlock: Fix handling of disconnected directories
From: Mickaël Salaün <mic@digikod.net>
Date: 2025-07-24 17:10:23
Also in:
linux-fsdevel
On Thu, Jul 24, 2025 at 04:49:24PM +0200, Günther Noack wrote:
On Wed, Jul 23, 2025 at 11:01:42PM +0200, Mickaël Salaün wrote:quoted
On Tue, Jul 22, 2025 at 07:04:02PM +0100, Tingmao Wang wrote:quoted
On the other hand, I'm still a bit uncertain about the domain check semantics. While it would not cause a rename to be allowed if it is otherwise not allowed by any rules on or above the mountpoint, this gets a bit weird if we have a situation where renames are allowed on the mountpoint or everywhere, but not read/writes, however read/writes are allowed directly on a file, but the dir containing that file gets disconnected so the sandboxed application can't read or write to it. (Maybe someone would set up such a policy where renames are allowed, expecting Landlock to always prevent renames where additional permissions would be exposed?) In the above situation, if the file is then moved to a connected directory, it will become readable/writable again.We can generalize this issue to not only the end file but any component of the path: disconnected directories. In fact, the main issue is the potential inconsistency of access checks over time (e.g. between two renames). This could be exploited to bypass the security checks done for FS_REFER. I see two solutions: 1. *Always* walk down to the IS_ROOT directory, and then jump to the mount point. This makes it possible to have consistent access checks for renames and open/use. The first downside is that that would change the current behavior for bind mounts that could get more access rights (if the policy explicitly sets rights for the hidden directories). The second downside is that we'll do more walk. 2. Return -EACCES (or -ENOENT) for actions involving disconnected directories, or renames of disconnected opened files. This second solution is simpler and safer but completely disables the use of disconnected directories and the rename of disconnected files for sandboxed processes. It would be much better to be able to handle opened directories as (object) capabilities, but that is not currently possible because of the way paths are handled by the VFS and LSM hooks. Tingmao, Günther, Jann, what do you think?I have to admit that so far, I still failed to wrap my head around the full patch set and its possible corner cases. I hope I did not misunderstand things all too badly below: As far as I understand the proposed patch, we are "checkpointing" the intermediate results of the path walk at every mount point boundary, and in the case where we run into a disconnected directory in one of the nested mount points, we restore from the intermediate result at the previous mount point directory and skip to the next mount point.
Correct
Visually speaking, if the layout is this (where ":" denotes a
mountpoint boundary between the mountpoints MP1, MP2, MP3):
dirfd
|
: V :
: ham <--- spam <--- eggs <--- x.txt
: (disconn.) :
: :
/ <--- foo <--- bar <--- baz :
: :
MP1 MP2 MP3
When a process holds a reference to the "spam" directory, which is now
disconnected, and invokes openat(dirfd, "eggs/x.txt", ...), then we
would:
* traverse x.txt
* traverse eggs (checkpointing the intermediate result) <-.
* traverse spam |
* traverse ham |
* discover that ham is disconnected: |
* restore the intermediate result from "eggs" ---------'
* continue the walk at foo
* end up at the root
So effectively, since the results from "spam" and "ham" are discarded,
we would traverse only the inodes in the outer and inner mountpoints
MP1 and MP3, but effectively return a result that looks like we did
not traverse MP2?We'd still check MP2's inode, but otherwise yes.
Maybe (likely) I misread the code. :) It's not clear to me what the thinking behind this is. Also, if there was another directory in between "spam" and "eggs" in MP2, wouldn't we be missing the access rights attached to this directory?
Yes, we would ignore this access right because we don't know that the path was resolved from spam.
Regarding the capability approach: I agree that a "capability" approach would be the better solution, but it seems infeasible with the existing LSM hooks at the moment. I would be in favor of it though.
Yes, it would be a new feature with potential important changes. In the meantime, we still need a fix for disconnected directories, and this fix needs to be backported. That's why the capability approach is not part of the two solutions. ;)
To spell it out a bit more explicitly what that would mean in my mind: When a path is looked up relative to a dirfd, the path walk upwards would terminate at the dirfd and use previously calculated access rights stored in the associated struct file. These access rights would be determined at the time of opening the dirfd, similar to how we are already storing the "truncate" access right today for regular files. (Remark: There might still be corner cases where we have to solve it the hard way, if someone uses ".." together with a dirfd-relative lookup.)
Yep, real capabilities don't have ".." in their design. On Linux (and Landlock), we need to properly handle "..", which is challenging.
I also looked at what it would take to change the LSM hooks to pass the directory that the lookup was done relative to, but it seems that this would have to be passed through a bunch of VFS callbacks as well, which seems like a larger change. I would be curious whether that would be deemed an acceptable change. —Günther P.S. Related to relative directory lookups, there is some movement in the BSDs as well to use dirfds as capabilities, by adding a flag to open directories that enforces O_BENEATH on subsequent opens: * https://undeadly.org/cgi?action=article;sid=20250529080623 * https://reviews.freebsd.org/D50371 (both found via https://news.ycombinator.com/item?id=44575361) If a dirfd had such a flag, that would get rid of the corner case above.
This would be nice but it would not solve the current issue because we cannot force all processes to use this flag (which breaks some use cases). FYI, Capsicum is a more complete implementation: https://man.freebsd.org/cgi/man.cgi?query=capsicum&sektion=4 See the vfs.lookup_cap_dotdot sysctl too.