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

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

Reply via email to