linux-kernel - [PATCH] securityfs: Add missing d

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-next>] [day] [month] [year] [list]

Message-ID: <202005051626.7648DC65@keescook>
Date:   Tue, 5 May 2020 16:40:35 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] securityfs: Add missing d_delete() call on removal

After using simple_unlink(), a call to d_delete() is needed in addition
to dput().

Signed-off-by: Kees Cook <keescook@...omium.org>
---
Is this correct? I went looking around and there are a lot of variations
on the simple_unlink() pattern...

Many using explicit locking and combinations of d_drop(), __d_drop(), etc.


Some missing d_delete()?

security/inode.c:			simple_unlink(dir, dentry);
security/inode.c-		d_delete(dentry);
security/inode.c-		dput(dentry);
--
arch/powerpc/platforms/cell/spufs/inode.c:			simple_unlink(d_inode(dir), dentry);
arch/powerpc/platforms/cell/spufs/inode.c-			/* XXX: what was dcache_lock protecting here? Other
arch/powerpc/platforms/cell/spufs/inode.c-			 * filesystems (IB, configfs) release dcache_lock
arch/powerpc/platforms/cell/spufs/inode.c-			 * before unlink */
arch/powerpc/platforms/cell/spufs/inode.c-			dput(dentry);

Should use d_delete() instead of d_drop()?

arch/s390/hypfs/inode.c:			simple_unlink(d_inode(parent), dentry);
arch/s390/hypfs/inode.c-	}
arch/s390/hypfs/inode.c-	d_drop(dentry);
arch/s390/hypfs/inode.c-	dput(dentry);
arch/s390/hypfs/inode.c-	inode_unlock(d_inode(parent));
arch/s390/hypfs/inode.c-}

Correct?

drivers/android/binderfs.c:		simple_unlink(parent_inode, dentry);
drivers/android/binderfs.c-		d_delete(dentry);
drivers/android/binderfs.c-		dput(dentry);
--
fs/nfsd/nfsctl.c:	ret = simple_unlink(dir, dentry);
fs/nfsd/nfsctl.c-	d_delete(dentry);
fs/nfsd/nfsctl.c-	dput(dentry);
--
net/sunrpc/rpc_pipe.c:	ret = simple_unlink(dir, dentry);
net/sunrpc/rpc_pipe.c-	if (!ret)
net/sunrpc/rpc_pipe.c-		fsnotify_unlink(dir, dentry);
net/sunrpc/rpc_pipe.c-	d_delete(dentry);
net/sunrpc/rpc_pipe.c-	dput(dentry);
--
security/apparmor/apparmorfs.c:			simple_unlink(dir, dentry);
security/apparmor/apparmorfs.c-		d_delete(dentry);
security/apparmor/apparmorfs.c-		dput(dentry);

---
 security/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/security/inode.c b/security/inode.c
index 6c326939750d..606f390d21d2 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -306,6 +306,7 @@ void securityfs_remove(struct dentry *dentry)
 			simple_rmdir(dir, dentry);
 		else
 			simple_unlink(dir, dentry);
+		d_delete(dentry);
 		dput(dentry);
 	}
 	inode_unlock(dir);
-- 
2.20.1


-- 
Kees Cook