All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] securityfs: Add missing d_delete() call on removal
@ 2020-05-05 23:40 Kees Cook
  2020-05-06  1:14 ` Al Viro
  0 siblings, 1 reply; 7+ messages in thread
From: Kees Cook @ 2020-05-05 23:40 UTC (permalink / raw)
  To: Al Viro
  Cc: James Morris, Serge E. Hallyn, linux-security-module, linux-kernel

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

Signed-off-by: Kees Cook <keescook@chromium.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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-05-06 22:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-05 23:40 [PATCH] securityfs: Add missing d_delete() call on removal Kees Cook
2020-05-06  1:14 ` Al Viro
2020-05-06  3:28   ` Kees Cook
2020-05-06  4:02     ` Al Viro
2020-05-06 15:34       ` Kees Cook
2020-05-06 18:49         ` Al Viro
2020-05-06 22:49           ` Kees Cook

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.