Re: [PATCH v3 2/2] vfs: avoid duplicating creds in faccessat if possible

From: Al Viro
Date: Thu Mar 02 2023 - 14:02:21 EST


On Thu, Mar 02, 2023 at 06:43:39PM +0000, Al Viro wrote:
> On Thu, Mar 02, 2023 at 07:22:17PM +0100, Mateusz Guzik wrote:
>
> > Ops, I meant "names_cache", here:
> > names_cachep = kmem_cache_create_usercopy("names_cache", PATH_MAX, 0,
> > SLAB_HWCACHE_ALIGN|SLAB_PANIC, 0, PATH_MAX, NULL);
> >
> > it is fs/dcache.c and I brainfarted into the above.
>
> So you mean __getname() stuff?

The thing is, getname_flags()/getname_kernel() is not the only user of that
thing; grep and you'll see (and keep in mind that cifs alloc_dentry_path()
is a __getname() wrapper, with its own callers). We might have bugs papered
over^W^Whardened away^W^Wpapered over in some of those users.

I agree that getname_flags()/getname_kernel()/sys_getcwd() have no need of
pre-zeroing; fw_get_filesystem_firmware(), ceph_mdsc_build_path(),
[hostfs]dentry_name() and ima_d_path() seem to be safe. So's
vboxsf_path_from_dentry() (I think). But with this bunch I'd need
a review before I'd be willing to say "this security theatre buys us
nothing here":

fs/cifs/cifsproto.h:67: return __getname();
fs/exfat/dir.c:195: nb->lfn = __getname();
fs/fat/dir.c:287: *unicode = __getname();
fs/fat/namei_vfat.c:598: uname = __getname();
fs/ntfs3/dir.c:394: name = __getname();
fs/ntfs3/inode.c:1289: new_de = __getname();
fs/ntfs3/inode.c:1694: de = __getname();
fs/ntfs3/inode.c:1732: de = __getname();
fs/ntfs3/namei.c:71: struct cpu_str *uni = __getname();
fs/ntfs3/namei.c:286: de = __getname();
fs/ntfs3/namei.c:355: struct cpu_str *uni = __getname();
fs/ntfs3/namei.c:494: uni = __getname();
fs/ntfs3/namei.c:555: uni1 = __getname();
fs/ntfs3/xattr.c:532: buf = __getname();

fs/cifs/cifs_dfs_ref.c:168: page = alloc_dentry_path();
fs/cifs/cifsacl.c:1697: page = alloc_dentry_path();
fs/cifs/cifsacl.c:1760: page = alloc_dentry_path();
fs/cifs/cifsproto.h:65:static inline void *alloc_dentry_path(void)
fs/cifs/dir.c:187: void *page = alloc_dentry_path();
fs/cifs/dir.c:604: page = alloc_dentry_path();
fs/cifs/dir.c:664: page = alloc_dentry_path();
fs/cifs/file.c:594: page = alloc_dentry_path();
fs/cifs/file.c:796: page = alloc_dentry_path();
fs/cifs/file.c:2223: void *page = alloc_dentry_path();
fs/cifs/file.c:2255: void *page = alloc_dentry_path();
fs/cifs/inode.c:1663: page = alloc_dentry_path();
fs/cifs/inode.c:1938: page = alloc_dentry_path();
fs/cifs/inode.c:2001: void *page = alloc_dentry_path();
fs/cifs/inode.c:2170: page1 = alloc_dentry_path();
fs/cifs/inode.c:2171: page2 = alloc_dentry_path();
fs/cifs/inode.c:2446: page = alloc_dentry_path();
fs/cifs/inode.c:2738: void *page = alloc_dentry_path();
fs/cifs/inode.c:2893: void *page = alloc_dentry_path();
fs/cifs/ioctl.c:34: void *page = alloc_dentry_path();
fs/cifs/link.c:491: page1 = alloc_dentry_path();
fs/cifs/link.c:492: page2 = alloc_dentry_path();
fs/cifs/misc.c:803: page = alloc_dentry_path();
fs/cifs/readdir.c:1071: void *page = alloc_dentry_path();
fs/cifs/smb2ops.c:2059: void *page = alloc_dentry_path();
fs/cifs/xattr.c:112: page = alloc_dentry_path();
fs/cifs/xattr.c:277: page = alloc_dentry_path();
fs/cifs/xattr.c:382: page = alloc_dentry_path();