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

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

[Günther Noack]

On Thu, Sep 24, 2026 at 06:48:19PM +0800, Cai Xinchen wrote:
> 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:
> 
>   1: notify_change() and its callers
>   2: inode_setsecctx hook (must come before 3: the SELinux and Smack
>      implementations call __vfs_setxattr_locked internally)
>   3: xattr helpers, which also drops a redundant EVM xattr size sanity
>      check whose vfs_getxattr() call only has a dentry and therefore
>      cannot be migrated to the new path-based signature
>   4: POSIX ACL helpers
>   5: inode_setattr hook
>   6: inode xattr hooks
>   7: inode POSIX ACL hooks
> 
> Two deliberate scoping decisions for this refactor:
> 
> - The hooks consistently take struct path rather than struct file.  The
>   VFS call sites involved (chmod(2), chown(2), utimensat(2), xattr(2)
>   and ACL syscalls) operate on paths, and several of them (lstat(2),
>   lchown(2), llistxattr(2), ...) have no struct file to begin with.
> 
> - struct inode_operations->setattr still receives (idmap, dentry, attr).
>   Only the VFS boundary (notify_change()) and the LSM hook layer see the
>   path, which keeps the refactor contained to fs/attr.c and the LSM
>   infrastructure instead of touching every filesystem.
> 
> Patches 8-12 then implement the new rights, their tests, the sandboxer
> sample and the documentation.  Semantics:
> 
> - READ_METADATA covers stat(2) and friends, getxattr(2) and friends,
>   listxattr(2) and friends, and POSIX ACL reads.
> - WRITE_METADATA covers chmod(2), chown(2), utimensat(2), setxattr(2),
>   removexattr(2) and friends, and POSIX ACL set and remove.
> - Only explicit metadata changes requested by user space are restricted.
>   Implicit changes performed by the kernel (e.g. timestamp updates on
>   write(2), size changes on truncate(2)) are not, and neither are
>   chmod(2)/chown(2) calls that change nothing (e.g. chown(2) with
>   (-1, -1), which never reaches the hook), matching the SELinux
>   inode_setattr behavior.
> - Kernel-internal accesses performed with override_creds() (e.g.
>   overlayfs, cachefiles) and kernel threads without a Landlock domain
>   (e.g. nfsd, ksmbd) are not restricted.
> 
> The Landlock ABI version is incremented from 11 to 12.
> 
> The series is based on linux-next commit 5c4d4169604b ("Add linux-next
> specific files for 20260921").
> 
> Testing: each patch has been built for aarch64 (gcc, -Werror) and the
> landlock selftests (445 tests, including the new ones) pass in QEMU on
> aarch64; base_test reports ABI v12.
> 
> [1] https://lore.kernel.org/all/20220827111215.131442-1-xiujianfeng@huawei.com/
> [2] https://lore.kernel.org/all/abc960a1-e66e-792e-6869-cfd201c29dbe@digikod.net/

Thank you for sending this patch set!

Some meta-remarks at the beginning:

* You might want to link the bugtracker feature request:
  https://github.com/landlock-lsm/linux/issues/11
* In the final version, I think it's preferred to merge patches 8
  (adding the access right enums) and 9 (adding the LSM hooks that use
  them).  Having the feature as an atomic commit makes it harder to
  accidentally mess it up during a backport, because you can't patch 8
  without 9.
* As Paul alluded to, the changes to the LSM hook interface and to the
  existing callers in VFS are likely the hardest part of this patch
  set.  Alexei from the BPF subsystem has also reiterated recently
  that he wants BPF to be looped into such changes.  BPF hooks do not
  give the same backwards compatibility guarantees as the syscall
  layer, but there are existing users of LSM hooks specifically
  through the BPF LSM.

* In https://github.com/landlock-lsm/linux/issues/18, we came across
  statfs(), which returns file system meta-information based for the
  file system that a given file belongs to.  I have weak confidence
  that READ_METADATA would be the right access right to protect this
  with, but it's a somewhat related operation.  Maybe you have some
  thoughts on this?

More concrete questions:

* If a "inode" LSM hook gets a "path" argument now, should it be
  renamed from "inode_..." to "path_..."?

  (Maybe the BPF people can chime in about to what extent that would
  cause additional churn for BPF users, in a situation where they
  anyway already need to make a change due to the changing function
  signature?)

–Günther

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

[Cai Xinchen]

Thank you for review!

I will link the feature request
(https://github.com/landlock-lsm/linux/issues/11) in the v2 cover letter.

For https://github.com/landlock-lsm/linux/issues/18, the current
READ_METADATA cannot solve this problem, and I don't have a good idea 
for now.

For path_* rename, I'd rather keep the inode_* names:

* There is precedent for the mismatch: the inode_getattr hook
   upstream already takes a "const struct path *" (this series just
   aligns the other metadata hooks with it).

* The path_* prefix has an established meaning in the LSM hook
   interface: it denotes the pathname-based (TOMOYO/AppArmor-style)
   hooks that are invoked at the VFS path level *before* the
   corresponding inode_* hook, as a paired, duplicate check
   serving a different class of LSMs.  For example, a chmod(2)
   currently runs security_path_chmod() (fs/open.c, for TOMOYO and
   AppArmor) and then, via notify_change(),
   security_inode_setattr() (for SELinux, Smack and now Landlock);
   vfs_mknod() similarly calls both security_path_mknod() and
   security_inode_mknod().  Renaming inode_setattr to
   path_setattr would put two path_-prefixed hooks with different
   call contracts into the same syscall, and path_chown(path,
   uid, gid) next to a path_setattr(path, attr) would read as a
   redundant pair, even though they belong to different hook
   families.  Landlock itself already uses the path_* family for
   its real path-level hooks (e.g. path_truncate for the TRUNCATE
   right), so mixing renamed inode hooks into that prefix would
   also blur its own hook table.

* If the consensus is that these hooks should be renamed, I think
   that should be a standalone, tree-wide rename series (including
   inode_getattr), so that BPF programs only break once instead of
   twice.

On 9/26/2026 4:27 PM, Günther Noack wrote:
> On Thu, Sep 24, 2026 at 06:48:19PM +0800, Cai Xinchen wrote:
>> 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:
>>
>>    1: notify_change() and its callers
>>    2: inode_setsecctx hook (must come before 3: the SELinux and Smack
>>       implementations call __vfs_setxattr_locked internally)
>>    3: xattr helpers, which also drops a redundant EVM xattr size sanity
>>       check whose vfs_getxattr() call only has a dentry and therefore
>>       cannot be migrated to the new path-based signature
>>    4: POSIX ACL helpers
>>    5: inode_setattr hook
>>    6: inode xattr hooks
>>    7: inode POSIX ACL hooks
>>
>> Two deliberate scoping decisions for this refactor:
>>
>> - The hooks consistently take struct path rather than struct file.  The
>>    VFS call sites involved (chmod(2), chown(2), utimensat(2), xattr(2)
>>    and ACL syscalls) operate on paths, and several of them (lstat(2),
>>    lchown(2), llistxattr(2), ...) have no struct file to begin with.
>>
>> - struct inode_operations->setattr still receives (idmap, dentry, attr).
>>    Only the VFS boundary (notify_change()) and the LSM hook layer see the
>>    path, which keeps the refactor contained to fs/attr.c and the LSM
>>    infrastructure instead of touching every filesystem.
>>
>> Patches 8-12 then implement the new rights, their tests, the sandboxer
>> sample and the documentation.  Semantics:
>>
>> - READ_METADATA covers stat(2) and friends, getxattr(2) and friends,
>>    listxattr(2) and friends, and POSIX ACL reads.
>> - WRITE_METADATA covers chmod(2), chown(2), utimensat(2), setxattr(2),
>>    removexattr(2) and friends, and POSIX ACL set and remove.
>> - Only explicit metadata changes requested by user space are restricted.
>>    Implicit changes performed by the kernel (e.g. timestamp updates on
>>    write(2), size changes on truncate(2)) are not, and neither are
>>    chmod(2)/chown(2) calls that change nothing (e.g. chown(2) with
>>    (-1, -1), which never reaches the hook), matching the SELinux
>>    inode_setattr behavior.
>> - Kernel-internal accesses performed with override_creds() (e.g.
>>    overlayfs, cachefiles) and kernel threads without a Landlock domain
>>    (e.g. nfsd, ksmbd) are not restricted.
>>
>> The Landlock ABI version is incremented from 11 to 12.
>>
>> The series is based on linux-next commit 5c4d4169604b ("Add linux-next
>> specific files for 20260921").
>>
>> Testing: each patch has been built for aarch64 (gcc, -Werror) and the
>> landlock selftests (445 tests, including the new ones) pass in QEMU on
>> aarch64; base_test reports ABI v12.
>>
>> [1] https://lore.kernel.org/all/20220827111215.131442-1-xiujianfeng@huawei.com/
>> [2] https://lore.kernel.org/all/abc960a1-e66e-792e-6869-cfd201c29dbe@digikod.net/
> Thank you for sending this patch set!
>
> Some meta-remarks at the beginning:
>
> * You might want to link the bugtracker feature request:
>    https://github.com/landlock-lsm/linux/issues/11
> * In the final version, I think it's preferred to merge patches 8
>    (adding the access right enums) and 9 (adding the LSM hooks that use
>    them).  Having the feature as an atomic commit makes it harder to
>    accidentally mess it up during a backport, because you can't patch 8
>    without 9.
> * As Paul alluded to, the changes to the LSM hook interface and to the
>    existing callers in VFS are likely the hardest part of this patch
>    set.  Alexei from the BPF subsystem has also reiterated recently
>    that he wants BPF to be looped into such changes.  BPF hooks do not
>    give the same backwards compatibility guarantees as the syscall
>    layer, but there are existing users of LSM hooks specifically
>    through the BPF LSM.
>
> * In https://github.com/landlock-lsm/linux/issues/18, we came across
>    statfs(), which returns file system meta-information based for the
>    file system that a given file belongs to.  I have weak confidence
>    that READ_METADATA would be the right access right to protect this
>    with, but it's a somewhat related operation.  Maybe you have some
>    thoughts on this?
>
> More concrete questions:
>
> * If a "inode" LSM hook gets a "path" argument now, should it be
>    renamed from "inode_..." to "path_..."?
>
>    (Maybe the BPF people can chime in about to what extent that would
>    cause additional churn for BPF users, in a situation where they
>    anyway already need to make a change due to the changing function
>    signature?)
>
> –Günther

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

[Mickaël Salaün]

Thank for this patch series!

I'll send more reviews tomorrow, but in the meantime here are some
answers:

On Mon, Sep 28, 2026 at 02:52:02PM +0800, Cai Xinchen wrote:
> Thank you for review!
> 
> I will link the feature request
> (https://github.com/landlock-lsm/linux/issues/11) in the v2 cover letter.
> 
> For https://github.com/landlock-lsm/linux/issues/18, the current
> READ_METADATA cannot solve this problem, and I don't have a good idea for
> now.
> 
> For path_* rename, I'd rather keep the inode_* names:
> 
> * There is precedent for the mismatch: the inode_getattr hook
>   upstream already takes a "const struct path *" (this series just
>   aligns the other metadata hooks with it).
> 
> * The path_* prefix has an established meaning in the LSM hook
>   interface: it denotes the pathname-based (TOMOYO/AppArmor-style)
>   hooks that are invoked at the VFS path level *before* the
>   corresponding inode_* hook, as a paired, duplicate check
>   serving a different class of LSMs.  For example, a chmod(2)
>   currently runs security_path_chmod() (fs/open.c, for TOMOYO and
>   AppArmor) and then, via notify_change(),
>   security_inode_setattr() (for SELinux, Smack and now Landlock);
>   vfs_mknod() similarly calls both security_path_mknod() and
>   security_inode_mknod().  Renaming inode_setattr to
>   path_setattr would put two path_-prefixed hooks with different
>   call contracts into the same syscall, and path_chown(path,
>   uid, gid) next to a path_setattr(path, attr) would read as a
>   redundant pair, even though they belong to different hook
>   families.  Landlock itself already uses the path_* family for
>   its real path-level hooks (e.g. path_truncate for the TRUNCATE
>   right), so mixing renamed inode hooks into that prefix would
>   also blur its own hook table.

I had the same though as Günther on this, but I'll leave that up to
Paul.

Regarding the BPF LSM use case, I don't think it would make a big
difference.  Anyway, some BPF selftests need to be updated at the same
time, which will also make the changes clear to the BPF folks.

> 
> * If the consensus is that these hooks should be renamed, I think
>   that should be a standalone, tree-wide rename series (including
>   inode_getattr), so that BPF programs only break once instead of
>   twice.

As a general rule, please create bisectable patches (e.g. any patch must
pass BPF and Landlock selftests; all dependent kernel code must build).
But new tests still deserve their own patches.

> 
> On 9/26/2026 4:27 PM, Günther Noack wrote:
> > On Thu, Sep 24, 2026 at 06:48:19PM +0800, Cai Xinchen wrote:
> > > 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:
> > > 
> > >    1: notify_change() and its callers
> > >    2: inode_setsecctx hook (must come before 3: the SELinux and Smack
> > >       implementations call __vfs_setxattr_locked internally)
> > >    3: xattr helpers, which also drops a redundant EVM xattr size sanity
> > >       check whose vfs_getxattr() call only has a dentry and therefore
> > >       cannot be migrated to the new path-based signature
> > >    4: POSIX ACL helpers
> > >    5: inode_setattr hook
> > >    6: inode xattr hooks
> > >    7: inode POSIX ACL hooks
> > > 
> > > Two deliberate scoping decisions for this refactor:
> > > 
> > > - The hooks consistently take struct path rather than struct file.  The
> > >    VFS call sites involved (chmod(2), chown(2), utimensat(2), xattr(2)
> > >    and ACL syscalls) operate on paths, and several of them (lstat(2),
> > >    lchown(2), llistxattr(2), ...) have no struct file to begin with.
> > > 
> > > - struct inode_operations->setattr still receives (idmap, dentry, attr).
> > >    Only the VFS boundary (notify_change()) and the LSM hook layer see the
> > >    path, which keeps the refactor contained to fs/attr.c and the LSM
> > >    infrastructure instead of touching every filesystem.
> > > 
> > > Patches 8-12 then implement the new rights, their tests, the sandboxer
> > > sample and the documentation.  Semantics:
> > > 
> > > - READ_METADATA covers stat(2) and friends, getxattr(2) and friends,
> > >    listxattr(2) and friends, and POSIX ACL reads.
> > > - WRITE_METADATA covers chmod(2), chown(2), utimensat(2), setxattr(2),
> > >    removexattr(2) and friends, and POSIX ACL set and remove.
> > > - Only explicit metadata changes requested by user space are restricted.
> > >    Implicit changes performed by the kernel (e.g. timestamp updates on
> > >    write(2), size changes on truncate(2)) are not, and neither are
> > >    chmod(2)/chown(2) calls that change nothing (e.g. chown(2) with
> > >    (-1, -1), which never reaches the hook), matching the SELinux
> > >    inode_setattr behavior.
> > > - Kernel-internal accesses performed with override_creds() (e.g.
> > >    overlayfs, cachefiles) and kernel threads without a Landlock domain
> > >    (e.g. nfsd, ksmbd) are not restricted.
> > > 
> > > The Landlock ABI version is incremented from 11 to 12.
> > > 
> > > The series is based on linux-next commit 5c4d4169604b ("Add linux-next
> > > specific files for 20260921").
> > > 
> > > Testing: each patch has been built for aarch64 (gcc, -Werror) and the
> > > landlock selftests (445 tests, including the new ones) pass in QEMU on
> > > aarch64; base_test reports ABI v12.
> > > 
> > > [1] https://lore.kernel.org/all/20220827111215.131442-1-xiujianfeng@huawei.com/
> > > [2] https://lore.kernel.org/all/abc960a1-e66e-792e-6869-cfd201c29dbe@digikod.net/
> > Thank you for sending this patch set!
> > 
> > Some meta-remarks at the beginning:
> > 
> > * You might want to link the bugtracker feature request:
> >    https://github.com/landlock-lsm/linux/issues/11
> > * In the final version, I think it's preferred to merge patches 8
> >    (adding the access right enums) and 9 (adding the LSM hooks that use
> >    them).  Having the feature as an atomic commit makes it harder to
> >    accidentally mess it up during a backport, because you can't patch 8
> >    without 9.
> > * As Paul alluded to, the changes to the LSM hook interface and to the
> >    existing callers in VFS are likely the hardest part of this patch
> >    set.  Alexei from the BPF subsystem has also reiterated recently
> >    that he wants BPF to be looped into such changes.  BPF hooks do not
> >    give the same backwards compatibility guarantees as the syscall
> >    layer, but there are existing users of LSM hooks specifically
> >    through the BPF LSM.
> > 
> > * In https://github.com/landlock-lsm/linux/issues/18, we came across
> >    statfs(), which returns file system meta-information based for the
> >    file system that a given file belongs to.  I have weak confidence
> >    that READ_METADATA would be the right access right to protect this
> >    with, but it's a somewhat related operation.  Maybe you have some
> >    thoughts on this?
> > 
> > More concrete questions:
> > 
> > * If a "inode" LSM hook gets a "path" argument now, should it be
> >    renamed from "inode_..." to "path_..."?
> > 
> >    (Maybe the BPF people can chime in about to what extent that would
> >    cause additional churn for BPF users, in a situation where they
> >    anyway already need to make a change due to the changing function
> >    signature?)
> > 
> > –Günther
>