Re: [PATCH 0/9] fs: harden anon inodes

[Mateusz Guzik <mjguzik@gmail.com>]

On Mon, Apr 7, 2025 at 11:54 AM Christian Brauner <brauner@kernel.org> wrote:
>
> * Anonymous inodes currently don't come with a proper mode causing
>   issues in the kernel when we want to add useful VFS debug assert. Fix
>   that by giving them a proper mode and masking it off when we report it
>   to userspace which relies on them not having any mode.
>
> * Anonymous inodes currently allow to change inode attributes because
>   the VFS falls back to simple_setattr() if i_op->setattr isn't
>   implemented. This means the ownership and mode for every single user
>   of anon_inode_inode can be changed. Block that as it's either useless
>   or actively harmful. If specific ownership is needed the respective
>   subsystem should allocate anonymous inodes from their own private
>   superblock.
>
> * Port pidfs to the new anon_inode_{g,s}etattr() helpers.
>
> * Add proper tests for anonymous inode behavior.
>
> The anonymous inode specific fixes should ideally be backported to all
> LTS kernels.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
> Christian Brauner (9):
>       anon_inode: use a proper mode internally
>       pidfs: use anon_inode_getattr()
>       anon_inode: explicitly block ->setattr()
>       pidfs: use anon_inode_setattr()
>       anon_inode: raise SB_I_NODEV and SB_I_NOEXEC
>       selftests/filesystems: add first test for anonymous inodes
>       selftests/filesystems: add second test for anonymous inodes
>       selftests/filesystems: add third test for anonymous inodes
>       selftests/filesystems: add fourth test for anonymous inodes
>

I have two nits, past that LGTM

1. I would add a comment explaining why S_IFREG in alloc_anon_inode()
2. commit messages for selftests could spell out what's being added
instead of being counted, it's all one-liners

for example:
selftests/filesystems: validate that anonymous inodes cannot be chown()ed

-- 
Mateusz Guzik <mjguzik gmail.com>