[lvc-project] [PATCH] gfs2: lock glock before dumping consistency errors

[Alexey Nepomnyashih]

gfs2_dump_glock() walks the glock holder list and dump_holder()
dereferences holder fields, including the owner pid. The holder list is
protected by gl->gl_lockref.lock, but the consistency error paths call
gfs2_dump_glock() without taking that lock.

This can race with holder removal or reinitialization and make
dump_holder() operate on a stale holder.

  Thread 1                         Thread 2
  --------                         --------
  gfs2_consist_inode_i()
    gfs2_dump_glock()
      gh = first holder
                                   gfs2_glock_dq_uninit(gh)
                                     gfs2_glock_dq(gh)
                                       spin_lock(&gl->gl_lockref.lock)
                                       list_del_init(&gh->gh_list)
                                       spin_unlock(&gl->gl_lockref.lock)
                                     gfs2_holder_uninit(gh)
                                       put_pid(gh->gh_owner_pid)
                                       gfs2_holder_mark_uninitialized(gh)
      dump_holder(gh)
        pid_is_meaningful(gh)
        pid_nr(gh->gh_owner_pid)

Depending on where the stale holder is dereferenced, this can show up as
a fault in pid_is_meaningful() or as a KASAN report in pid_nr().

Reuse the existing locked glock dump wrapper for the consistency dumps.

Reported-by: syzbot+7efd59a5a532c57037e6 at syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7efd59a5a532c57037e6
Fixes: a739765cd8e6 ("gfs2: dump glocks from gfs2_consist_OBJ_i")
Cc: stable at vger.kernel.org
Signed-off-by: Alexey Nepomnyashih <sdl at nppct.ru>
---
 fs/gfs2/glock.c | 6 +++---
 fs/gfs2/glock.h | 2 ++
 fs/gfs2/util.c  | 4 ++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index b8a144d3a73b..548ce5f8866f 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -2100,7 +2100,7 @@ void gfs2_glock_thaw(struct gfs2_sbd *sdp)
 	glock_hash_walk(thaw_glock, sdp);
 }
 
-static void dump_glock(struct seq_file *seq, struct gfs2_glock *gl, bool fsid)
+void gfs2_dump_glock_locked(struct seq_file *seq, struct gfs2_glock *gl, bool fsid)
 {
 	spin_lock(&gl->gl_lockref.lock);
 	gfs2_dump_glock(seq, gl, fsid);
@@ -2109,7 +2109,7 @@ static void dump_glock(struct seq_file *seq, struct gfs2_glock *gl, bool fsid)
 
 static void dump_glock_func(struct gfs2_glock *gl)
 {
-	dump_glock(NULL, gl, true);
+	gfs2_dump_glock_locked(NULL, gl, true);
 }
 
 static void withdraw_glock(struct gfs2_glock *gl)
@@ -2537,7 +2537,7 @@ static void gfs2_glock_seq_stop(struct seq_file *seq, void *iter_ptr)
 
 static int gfs2_glock_seq_show(struct seq_file *seq, void *iter_ptr)
 {
-	dump_glock(seq, iter_ptr, false);
+	gfs2_dump_glock_locked(seq, iter_ptr, false);
 	return 0;
 }
 
diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h
index 6341ac9b863f..f58c532d193a 100644
--- a/fs/gfs2/glock.h
+++ b/fs/gfs2/glock.h
@@ -217,6 +217,8 @@ int gfs2_glock_nq_m(unsigned int num_gh, struct gfs2_holder *ghs);
 void gfs2_glock_dq_m(unsigned int num_gh, struct gfs2_holder *ghs);
 void gfs2_dump_glock(struct seq_file *seq, struct gfs2_glock *gl,
 			    bool fsid);
+void gfs2_dump_glock_locked(struct seq_file *seq, struct gfs2_glock *gl,
+			    bool fsid);
 #define GLOCK_BUG_ON(gl,x) do { if (unlikely(x)) {		\
 			gfs2_dump_glock(NULL, gl, true);	\
 			BUG(); } } while(0)
diff --git a/fs/gfs2/util.c b/fs/gfs2/util.c
index 83b8bb6446e5..3417d1553b13 100644
--- a/fs/gfs2/util.c
+++ b/fs/gfs2/util.c
@@ -342,7 +342,7 @@ void gfs2_consist_inode_i(struct gfs2_inode *ip,
 		(unsigned long long)ip->i_no_formal_ino,
 		(unsigned long long)ip->i_no_addr,
 		function, file, line);
-	gfs2_dump_glock(NULL, ip->i_gl, 1);
+	gfs2_dump_glock_locked(NULL, ip->i_gl, true);
 	gfs2_withdraw(sdp);
 }
 
@@ -364,7 +364,7 @@ void gfs2_consist_rgrpd_i(struct gfs2_rgrpd *rgd,
 		"function = %s, file = %s, line = %u\n",
 		(unsigned long long)rgd->rd_addr,
 		function, file, line);
-	gfs2_dump_glock(NULL, rgd->rd_gl, 1);
+	gfs2_dump_glock_locked(NULL, rgd->rd_gl, true);
 	gfs2_withdraw(sdp);
 }
 
-- 
2.43.0