[PATCH] gfs2: avoid sysfs freeze and unmount lock inversion

["Jiacheng Xu" <stitch@zju.edu.cn>]

The gfs2 freeze sysfs callback runs with kernfs active protection held.
freeze_super() and thaw_super() may acquire sb->s_umount.

At the same time, the unmount path holds s_umount and calls
gfs2_sys_fs_del(), which drains the same kernfs node. This creates an
ABBA dependency:

freeze_store()       kernfs active -> s_umount
gfs2_put_super()     s_umount -> kernfs active

Break the active protection before accessing s_umount. Since this allows
unmount to proceed concurrently, acquire an active superblock reference
while holding s_umount for reading. Release that reference from
gfs2_freeze_wq so a final superblock reference cannot enter
gfs2_put_super() from the sysfs callback itself.

Return -EAGAIN if the superblock is being mounted, unmounted, or otherwise
has s_umount held. Reject invalid freeze values before taking any
references or changing sysfs active protection.

Fixes: b3b94faa5fe5 ("[GFS2] The core of GFS2")
Signed-off-by: Jiacheng Xu <stitch@zju.edu.cn>

---
fs/gfs2/sys.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 76 insertions(+), 8 deletions(-)

diff --git a/fs/gfs2/sys.c b/fs/gfs2/sys.c
index ea2c7b9e4a77..78bc310be1fb 100644
--- a/fs/gfs2/sys.c
+++ b/fs/gfs2/sys.c
@@ -11,11 +11,14 @@
#include <linux/spinlock.h>
#include <linux/completion.h>
#include <linux/buffer_head.h>
+#include <linux/slab.h>
#include <linux/module.h>
#include <linux/kobject.h>
+#include <linux/sysfs.h>
#include <linux/uaccess.h>
#include <linux/gfs2_ondisk.h>
#include <linux/blkdev.h>
+#include <linux/workqueue.h>

#include "gfs2.h"
#include "incore.h"
@@ -33,6 +36,24 @@ struct gfs2_attr {
      ssize_t (*store)(struct gfs2_sbd *, const char *, size_t);
};

+/* Forward declaration for freeze_store(), defined with the other attributes. */
+static struct gfs2_attr gfs2_attr_freeze;
+
+struct gfs2_sysfs_sb_ref {
+     struct work_struct work;
+     struct super_block *sb;
+};
+
+static void gfs2_sysfs_sb_ref_put(struct work_struct *work)
+{
+     struct gfs2_sysfs_sb_ref *ref =
+             container_of(work, struct gfs2_sysfs_sb_ref, work);
+     struct super_block *sb = ref->sb;
+
+     deactivate_super(sb);
+     kfree(ref);
+}
+
static ssize_t gfs2_attr_show(struct kobject *kobj, struct attribute *attr,
                        char *buf)
{
@@ -153,6 +174,9 @@ static ssize_t freeze_show(struct gfs2_sbd *sdp, char *buf)

static ssize_t freeze_store(struct gfs2_sbd *sdp, const char *buf, size_t len)
{
+     struct super_block *sb = sdp->sd_vfs;
+     struct gfs2_sysfs_sb_ref *ref = NULL;
+     struct kernfs_node *kn;
      int error, n;

      error = kstrtoint(buf, 0, &n);
@@ -162,23 +186,67 @@ static ssize_t freeze_store(struct gfs2_sbd *sdp, const char *buf, size_t
len)
      if (!capable(CAP_SYS_ADMIN))
            return -EPERM;

+     if (n != 0 && n != 1)
+             return -EINVAL;
+
+     /*
+      * The sysfs callback normally holds kn->active.  Break that
+      * dependency before taking s_umount, otherwise unmount can wait for
+      * this callback while the callback waits for s_umount.
+      */
+     kn = sysfs_break_active_protection(&sdp->sd_kobj,
+                                        &gfs2_attr_freeze.attr);
+     if (!kn)
+             return -ENODEV;
+
+     ref = kmalloc_obj(*ref);
+     if (!ref) {
+             error = -ENOMEM;
+             goto out_unbreak;
+     }
+     INIT_WORK(&ref->work, gfs2_sysfs_sb_ref_put);
+     ref->sb = sb;
+
+     /*
+      * Pin the superblock before running an operation which may wait for
+      * s_umount.  Release the pin from a separate work item so a last
+      * reference cannot enter gfs2_put_super() on the sysfs call path.
+      */
+     if (!down_read_trylock(&sb->s_umount)) {
+             error = -EAGAIN;
+             goto out_unbreak;
+     }
+     if (!(sb->s_flags & SB_ACTIVE)) {
+             up_read(&sb->s_umount);
+             error = -EAGAIN;
+             goto out_unbreak;
+     }
+     atomic_inc(&sb->s_active);
+     up_read(&sb->s_umount);
+
      switch (n) {
      case 0:
-             error = thaw_super(sdp->sd_vfs, FREEZE_HOLDER_USERSPACE, NULL);
+             error = thaw_super(sb, FREEZE_HOLDER_USERSPACE, NULL);
            break;
      case 1:
-             error = freeze_super(sdp->sd_vfs, FREEZE_HOLDER_USERSPACE, NULL);
+             error = freeze_super(sb, FREEZE_HOLDER_USERSPACE, NULL);
            break;
-     default:
-             return -EINVAL;
      }

-     if (error) {
+     if (error)
            fs_warn(sdp, "freeze %d error %d\n", n, error);
-             return error;
-     }

-     return len;
+     /* No sdp access is allowed after active protection is restored. */
+     sysfs_unbreak_active_protection(kn);
+     /* gfs2 module teardown drains this queue before unloading the module. */
+     queue_work(gfs2_freeze_wq, &ref->work);
+
+     return error ? error : len;
+
+out_unbreak:
+     sysfs_unbreak_active_protection(kn);
+     kfree(ref);
+     return error;
}

static ssize_t withdraw_show(struct gfs2_sbd *sdp, char *buf)