[PATCH] gfs2: keep LRU glocks with pending AIL during withdraw

[Helen Koike]

The shrinker can queue a glock to be demoted while racing with a
withdraw.
Then do_xmote skips go_sync() since the fs is in withdraw, so the AIL
list isn't drained and it proceeds setting the glock to LM_ST_UNLOCKED,
the ref drops to zero, and the rcu_core eventually calls
gfs2_glock_dealloc() freeing its memory. And when we are unlucky, the
rcu grace period + mem free finish before do_withdraw finds the bd
during gfs2_ail_drain() that points to the freed glock, causing a UAF.

Prevent the LRU shrinker from evicting such a glock; leave it on the
LRU until unmount.

Reported-by: syzbot+84a21b16aba9f279d96a@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=84a21b16aba9f279d96a
Fixes: 767fd5a01607 ("gfs2: Revise glock reference counting model")
Signed-off-by: Helen Koike <koike@xxxxxxxxxx>
---

Hi all,

I was investigating the mentioned UAF issue reported by syzbot.
After some digging it seems to me this is the cause of the issue, but I
couldn't validate 100% since it is hard to reproduce, even using the
reproducer from syzbot.
The reasoning is mostly based on static analysis of the crash log and
the code paths. Please let me know if I've misread the race or if
there's a better place to intervene.

Thanks
---
 fs/gfs2/glock.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index b8a144d3a73b..0d81f3c40165 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -1896,7 +1896,9 @@ static bool can_free_glock(struct gfs2_glock *gl)
 	return !test_bit(GLF_LOCK, &gl->gl_flags) &&
 	       !gl->gl_lockref.count &&
 	       (!test_bit(GLF_LFLUSH, &gl->gl_flags) ||
-		test_bit(SDF_KILL, &sdp->sd_flags));
+		test_bit(SDF_KILL, &sdp->sd_flags)) &&
+	       /* Avoid racing with gfs2_ail_drain() during withdraw. */
+	       !(gfs2_withdrawn(sdp) && atomic_read(&gl->gl_ail_count));
 }
 
 /**
-- 
2.54.0