d_make_discardable(): warn if given a non-persistent dentry

At this point there are very few call chains that might lead to
d_make_discardable() on a dentry that hadn't been made persistent:
calls of simple_unlink() and simple_rmdir() in configfs and
apparmorfs.

Both filesystems do pin (part of) their contents in dcache, but
they are currently playing very unusual games with that.  Converting
them to more usual patterns might be possible, but it's definitely
going to be a long series of changes in both cases.

For now the easiest solution is to have both stop using simple_unlink()
and simple_rmdir() - that allows to make d_make_discardable() warn
when given a non-persistent dentry.

Rather than giving them full-blown private copies (with calls of
d_make_discardable() replaced with dput()), let's pull the parts of
simple_unlink() and simple_rmdir() that deal with timestamps and link
counts into separate helpers (__simple_unlink() and __simple_rmdir()
resp.) and have those used by configfs and apparmorfs.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 81f4f06bc87e755c1c7d3ddc25c4bc3581577dc8..e8f2f44012e9ab709c13c25b3e9daf9dcf9c8c7d 100644 (file)
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -400,8 +400,14 @@ static void remove_dir(struct dentry * d)
 
        configfs_remove_dirent(d);
 
-       if (d_really_is_positive(d))
-               simple_rmdir(d_inode(parent),d);
+       if (d_really_is_positive(d)) {
+               if (likely(simple_empty(d))) {
+                       __simple_rmdir(d_inode(parent),d);
+                       dput(d);
+               } else {
+                       pr_warn("remove_dir (%pd): attributes remain", d);
+               }
+       }
 
        pr_debug(" o %pd removing done (%d)\n", d, d_count(d));
 

diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index 1d2e3a5738d10d352f762878d80744d521bc8798..bcda3372e141ad6086786bc8194a306f84a190c8 100644 (file)
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -211,7 +211,8 @@ void configfs_drop_dentry(struct configfs_dirent * sd, struct dentry * parent)
                        dget_dlock(dentry);
                        __d_drop(dentry);
                        spin_unlock(&dentry->d_lock);
-                       simple_unlink(d_inode(parent), dentry);
+                       __simple_unlink(d_inode(parent), dentry);
+                       dput(dentry);
                } else
                        spin_unlock(&dentry->d_lock);
        }

diff --git a/fs/dcache.c b/fs/dcache.c
index 5ee2e78a91b394e74cbc2625d5205e286bf10bf9..824d620bb563bed69afe81c6149ae7d3e15d1365 100644 (file)
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -931,14 +931,7 @@ EXPORT_SYMBOL(dput);
 void d_make_discardable(struct dentry *dentry)
 {
        spin_lock(&dentry->d_lock);
-       /*
-        * By the end of the series we'll add
-        * WARN_ON(!(dentry->d_flags & DCACHE_PERSISTENT);
-        * here, but while object removal is done by a few common helpers,
-        * object creation tends to be open-coded (if nothing else, new inode
-        * needs to be set up), so adding a warning from the very beginning
-        * would make for much messier patch series.
-        */
+       WARN_ON(!(dentry->d_flags & DCACHE_PERSISTENT));
        dentry->d_flags &= ~DCACHE_PERSISTENT;
        dentry->d_lockref.count--;
        rcu_read_lock();

diff --git a/fs/libfs.c b/fs/libfs.c
index 80f288a771e30f7a118c52f0cafc8254bb14bc46..0aa630e7eb00f10a0f31b996c76aa34a762d5f1c 100644 (file)
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -790,13 +790,27 @@ out:
 }
 EXPORT_SYMBOL(simple_empty);
 
-int simple_unlink(struct inode *dir, struct dentry *dentry)
+void __simple_unlink(struct inode *dir, struct dentry *dentry)
 {
        struct inode *inode = d_inode(dentry);
 
        inode_set_mtime_to_ts(dir,
                              inode_set_ctime_to_ts(dir, inode_set_ctime_current(inode)));
        drop_nlink(inode);
+}
+EXPORT_SYMBOL(__simple_unlink);
+
+void __simple_rmdir(struct inode *dir, struct dentry *dentry)
+{
+       drop_nlink(d_inode(dentry));
+       __simple_unlink(dir, dentry);
+       drop_nlink(dir);
+}
+EXPORT_SYMBOL(__simple_rmdir);
+
+int simple_unlink(struct inode *dir, struct dentry *dentry)
+{
+       __simple_unlink(dir, dentry);
        d_make_discardable(dentry);
        return 0;
 }
@@ -807,9 +821,8 @@ int simple_rmdir(struct inode *dir, struct dentry *dentry)
        if (!simple_empty(dentry))
                return -ENOTEMPTY;
 
-       drop_nlink(d_inode(dentry));
-       simple_unlink(dir, dentry);
-       drop_nlink(dir);
+       __simple_rmdir(dir, dentry);
+       d_make_discardable(dentry);
        return 0;
 }
 EXPORT_SYMBOL(simple_rmdir);

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 95933ceaae51d8829171e8b44db3b6366b698c12..ef842adbd418bdb5f6d742878d999c9dee4bc8c3 100644 (file)
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3621,6 +3621,8 @@ extern int simple_open(struct inode *inode, struct file *file);
 extern int simple_link(struct dentry *, struct inode *, struct dentry *);
 extern int simple_unlink(struct inode *, struct dentry *);
 extern int simple_rmdir(struct inode *, struct dentry *);
+extern void __simple_unlink(struct inode *, struct dentry *);
+extern void __simple_rmdir(struct inode *, struct dentry *);
 void simple_rename_timestamp(struct inode *old_dir, struct dentry *old_dentry,
                             struct inode *new_dir, struct dentry *new_dentry);
 extern int simple_rename_exchange(struct inode *old_dir, struct dentry *old_dentry,

diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 391a586d0557f04b8ebe7e6175ceb71717a4ccdd..9b9090d38ea22673f272a72724fc9281be4584c4 100644 (file)
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -358,10 +358,15 @@ static void aafs_remove(struct dentry *dentry)
        dir = d_inode(dentry->d_parent);
        inode_lock(dir);
        if (simple_positive(dentry)) {
-               if (d_is_dir(dentry))
-                       simple_rmdir(dir, dentry);
-               else
-                       simple_unlink(dir, dentry);
+               if (d_is_dir(dentry)) {
+                       if (!WARN_ON(!simple_empty(dentry))) {
+                               __simple_rmdir(dir, dentry);
+                               dput(dentry);
+                       }
+               } else {
+                       __simple_unlink(dir, dentry);
+                       dput(dentry);
+               }
                d_delete(dentry);
                dput(dentry);
        }