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 <[email protected]> --- 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
