On Wed, Sep 21, 2022 at 05:06:57AM +0200, Miklos Szeredi wrote: > On Tue, 20 Sept 2022 at 22:57, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > > > > On Tue, Sep 20, 2022 at 09:36:30PM +0200, Miklos Szeredi wrote: > > > > > inode = child->d_inode; > > > > Better > > inode = file_inode(file); > > > > so that child would be completely ignored after dput(). > > > > > + error = vfs_tmpfile(mnt_userns, &path, file, op->mode); > > > + if (error) > > > goto out2; > > > - dput(path.dentry); > > > - path.dentry = child; > > > - audit_inode(nd->name, child, 0); > > > + audit_inode(nd->name, file->f_path.dentry, 0); > > > /* Don't check for other permissions, the inode was just created */ > > > - error = may_open(mnt_userns, &path, 0, op->open_flag); > > > > Umm... I'm not sure that losing it is the right thing - it might > > be argued that ->permission(..., MAY_OPEN) is to be ignored for > > tmpfile (and the only thing checking for MAY_OPEN is nfs, which is > > *not* going to grow tmpfile any time soon - certainly not with these > > calling conventions), but you are also dropping the call of > > security_inode_permission(inode, MAY_OPEN) and that's a change > > compared to what LSM crowd used to get... > > Not losing it, just moving it into vfs_tmpfile(). Afaict, we haven't called may_open() for tmpfile creation in either cachefiles or overlayfs before. So from that perspective I wonder if there's a good reason for us to do it now. The fact that we don't account these kernel internal tmpfiles feels like another exemption that points to them being internal files that don't need to be subject to common restrictions.