Thread (33 messages) 33 messages, 9 authors, 21d ago

Re: [PATCH RFC -next 00/12] landlock: Add READ_METADATA and WRITE_METADATA access rights

flat view

From: Justin Suess <hidden>
Date: 2026-10-01 13:47:51
Also in: driver-core, linux-block, linux-cifs, linux-doc, linux-fsdevel, linux-integrity, linux-kselftest, linux-nfs, linux-unionfs, lkml, netfs, selinux

On Tue, Sep 29, 2026 at 08:51:47PM +0200, Mickaël Salaün wrote:
On Tue, Sep 29, 2026 at 01:27:44PM -0400, Justin Suess wrote:
quoted
On Tue, Sep 29, 2026 at 02:12:42PM +0200, Günther Noack wrote:
quoted
On Mon, Sep 28, 2026 at 01:13:35PM -0400, Justin Suess wrote:
quoted
On Sat, Sep 26, 2026 at 09:56:28AM +0200, Günther Noack wrote:
quoted
Hello!

On Fri, Sep 25, 2026 at 02:03:05PM -0400, Justin Suess wrote:
quoted
On Thu, Sep 24, 2026 at 06:48:19PM +0800, Cai Xinchen wrote:
quoted
This series adds two new Landlock filesystem access rights,
LANDLOCK_ACCESS_FS_READ_METADATA and LANDLOCK_ACCESS_FS_WRITE_METADATA,
which control access to file and directory metadata such as inode
attributes (mode, ownership, timestamps), extended attributes and POSIX
ACLs.  It picks up the work from the "landlock: add chmod and chown
support" series [1] and follows the coarse-grained grouping discussed in
that thread [2]: instead of separate chmod/chown rights, metadata
operations are grouped into one read and one write right.

Landlock evaluates access rights on a per-path basis, but the metadata
related LSM hooks (inode_getattr, inode_setattr, inode_setxattr,
inode_getxattr, inode_listxattr, inode_removexattr, inode_set_acl,
inode_get_acl, inode_remove_acl) only receive the dentry of the accessed
object.  Patches 1-7 therefore first pass struct path instead of dentry
through the metadata-related VFS helpers and LSM hooks.  This is a pure
refactoring with no behavior change, split so that every patch builds
and works on its own:
I like these patches, but is the ability to read metadata already
sorta controlled by LANDLOCK_ACCESS_FS_READ_DIR on the parent
directory?

The one case I see this being different is:

1. if you wanted to grant read access to the file, but not metadata
read access, but I can't think of any usecase for being able to read
the contents of a file, but not the metadata. (see below)

2. If you had the absolute path already and didn't need READ_DIR.

I see introducing this READ_METADATA as causing potential
hard-to-diagnose issues.

Say you handle READ_METADATA and READ_FILE, but only grant READ_FILE.

The program can technically open the file with the READ_FILE permission,
but it may error out because the stat() on it beforehand failed.
It's pretty common for programs to do that kind of thing (stat before
open), like for checking for config files (strace bash and you see it
stat .profile, /etc/profile)

There may be other bugs, because being able to set permissions to read
a file *but not read it's metadata* isn't possible currently in posix
acl and userspace may not work well if that assumption no longer holds.
* posix acl and linux DAC.
quoted
quoted
So maybe WRITE_METADATA is good enough?
The existing use cases are the combinations of (a) READ_DIR
allowed/denied and (b) READ_METADATA allowed/denied.  Because these
two access rights overlap slightly, it seems likely that for a given
directory or file, users will want to either grant both, or deny both.

At the moment, where the (not yet existing) READ_METADATA is
implicitly always allowed, the problematic case is the one where the
Landlock user wants to deny READ_DIR, but where much of the same
metadata is still available through stat() and the various
get-attribute syscalls.  (c.f. the warning box in the Landlock docs
[1])

In my view the READ_METADATA right closes a gap that READ_DIR left
open (which is also potentially surprising to callers if they did not
read the docs closely).  Also, if its implementation is symmetric to
WRITE_METADATA, I feel that it's worth having it in the same patch
set.

–Günther

P.S.: I know, even after we can control stat(), there are likely ways
to infer the presence of a file by observing Landlock error codes.
This would be nice to fix as well, but is harder to do without
controlling the path walk itself [2].  But also, the fact that this is
currently not controllable is not an excuse for leaving READ_METADATA
open IMHO.
I'm still sort of concerned about the case where read access is allowed,
but metadata isn't, due to how many applications will not cleanly handle
such an unexpected condition. You can reproduce this with a seccomp
policy forbidding stat().

Would it make sense to have the existing READ rights
(READ_DIR/READ_FILE) imply READ_METADATA on the files/directories 
if READ_METADATA is handled? Reading a file's contents should always
imply that you can read the metadata.
I am wary of situations where the handling of access rights implicity
implies other access rights.  We have discussed such schemes in the
past (e.g. we considered a design for RESOLVE_UNIX where we'd have
both a "scoped" and a "access_fs" right that would interact with each
other), but in the end we always settled for approaches where such
interactions would not be necessary.  One of the concerns was that it
would complicate "best effort" fallback logic in all Landlock
libraries and in the countless places where people use the syscalls
directly.

To throw another option in the mix.  (To be clear, I have only 70%
confidence, so feel free to push back, but it feels like it might
work?):

Is this similar to the "truncate" right?
----------------------------------------

With "truncate", there was an existing common operation (open(2) with
O_TRUNC, a.k.a. creat(2)) which called the truncation hook and checked
for the truncation access right.  But that was in fact OK.  The way we
resolved it at the time was by documenting very loudly that WRITE_FILE
and TRUNCATE access rights should always be requested in lockstep, if
TRUNCATE is handled.

If READ_DIR and READ_FILE do in fact read and return metadata to the
user, maybe the right thing would be to do it the same way here and
*require* that we have READ_METADATA to do these operations?

(READ_METADATA is automatically allowed as long as it's not handled,
so that approach does not break existing programs.  Programs who
consciously start handling READ_METADATA must simply take into account
that reading directories and opening files for reading requires
READ_METADATA.)

(BTW, I can see it for reading directories, but I am not sure I fully
follow the argument why opening files for reading means that you can
read the metadata?  Can't that be guarded on fstat()-like operations?)
Many standard libraries will call stat before / after opening a file to
allocate a buffer matching the file size. Or to figure out if mmap
is more efficient than opening it directly.

So Python's open().read() or Go's os.ReadFile they may throw an error
when opening the file, making it look as if the file is inaccessible
when its contents are. They make the assumption that if a file is
readable, the metadata is too.

There's also a lot of metadata that is already leaked by just having
READ_FILE. FS_IOC_GETFLAGS/FS_IOC_FSGETXATTR ioctl / fileattr_get are
unrestrictable and allow you to see inode attributes. There's also
access(2) which isn't restricted here and allows you to see your rights
on the file. The size can be found by simply seeking to the beginning
and end. So much of the metadata is obtainable with just READ_DIR/READ_FILE.

...

Another problem I'm just realizing is that this READ_METADATA
right is inconsistent with open file descriptor behavior. With most
rights, already open file descriptors are exempted, but here, fstat,
(and the other stat-family calls which takes a file descriptor) become
denied even on already open files. It's a catch 22 here, if you change
the fstat to not apply to opened files, then READ_FILE becomes a bypass
for READ_METADATA. But if you leave it as is, then it's inconsistent with
the other Landlock rights wrt already opened files.
Indeed, these access rights should follow the
LANDLOCK_ACCESS_FS_TRUNCATE mechanic.
Ahh good idea. Forgot about that! So associate the
READ_METADATA right with the fd at open time like with truncate.
quoted
Which isn't the end of the world, but just shows that either
approach is going to have it's quirks. 

I think it would be better to just avoid the cat and mouse game here
of trying to seperate READ_FILE/READ_METADATA and either require
READ_METADATA be specified with READ_FILE if READ_METADATA is handled
like Gunther proposed, or have READ_FILE imply READ_METADATA.

Documenting it strongly is OK too, but really there are really zero
usecases where you'd want to grant READ_FILE without READ_METADATA
so it would need to be made extremely clear.
What about a program that just need to read files? We can think about
sanboxes such as those used in web browsers, but that applies to other
tailored processes that don't need/want to access metadata e.g., for
confidentiality or personal information (UID, timestamp) concerns.

This is similar to FILE_WRITE vs. TRUNCATE: most of the time we want
them to be grouped.
Your argument is persuasive :)

I still have open questions about file_getattr(2)
and the FS_* ioctls above, which I don't think are handled by this
READ_METADATA right. access(2) is a seperate question, but I'm
unsure if that's worth it/in scope for restricting.

(I think WRITE_METADTA also doesn't handle file_setattr(2) and
the FS_IOC_FSSETFLAGS ioctls)

Justin
quoted
Thanks,
Justin
quoted
—Günther
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help