On Mon, Apr 07, 2025 at 12:19:45PM +0200, Mateusz Guzik wrote:
> On Mon, Apr 7, 2025 at 11:54 AM Christian Brauner <brauner@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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()
/*
* Historically anonymous inodes didn't have a type at all and
* userspace has come to rely on this. Internally they're just
* regular files but S_IFREG is masked off when reporting
* information to userspace.
*/
> 2. commit messages for selftests could spell out what's being added
> instead of being counted, it's all one-liners
I've replaced the count with chown(), chmod(), exec(), and open().
Thanks!
