clean_journal is called within a read lock on sdp->sd_log_flush_lock.
However, clean_journal makes modifications on sdp-> members such as
sd_log_flush_head. That field is always protected using that same lock.
As this field is modified under a read lock instead of a write lock,
this can lead to race conditions. The solution is to change
{down,up}_read to {down,up}_write on the callsite.
Note:
I am currently working on a static analyser to detect missing locks.
This was a reported case. I manually verified the report by looking
at the code, so that I do not send wrong information or patches.
After concluding that this seems to be a true positive, I created
this patch. I was only able to compile-test this patch.
Signed-off-by: Niels Dossche <dossche.ni...@gmail.com>
---
fs/gfs2/recovery.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/gfs2/recovery.c b/fs/gfs2/recovery.c
index 016ed1b2ca1d..19db755a6828 100644
--- a/fs/gfs2/recovery.c
+++ b/fs/gfs2/recovery.c
@@ -508,21 +508,21 @@ void gfs2_recover_func(struct work_struct *work)
/* We take the sd_log_flush_lock here primarily to prevent log
* flushes and simultaneous journal replays from stomping on
* each other wrt jd_log_bio. */
- down_read(&sdp->sd_log_flush_lock);
+ down_write(&sdp->sd_log_flush_lock);
for (pass = 0; pass < 2; pass++) {
lops_before_scan(jd, &head, pass);
error = foreach_descriptor(jd, head.lh_tail,
head.lh_blkno, pass);
lops_after_scan(jd, error, pass);
if (error) {
- up_read(&sdp->sd_log_flush_lock);
+ up_write(&sdp->sd_log_flush_lock);
goto fail_gunlock_thaw;
}
}
recover_local_statfs(jd, &head);
clean_journal(jd, &head);
- up_read(&sdp->sd_log_flush_lock);
+ up_write(&sdp->sd_log_flush_lock);
gfs2_freeze_unlock(&thaw_gh);
t_rep = ktime_get();
--
2.35.1
