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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 2 Mar 2023 19:02:10 +0000
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Mateusz Guzik <mjguzik@...il.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Christian Brauner <brauner@...nel.org>, serge@...lyn.com,
        paul@...l-moore.com, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCH v3 2/2] vfs: avoid duplicating creds in faccessat if
 possible

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();