linux-ext4 - Re: [PATCH 1/2] jbd2: jbd2_journal_stop needs an exclusive control to synchronize around t

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120105031240.GC24494@thunk.org>
Date:	Wed, 4 Jan 2012 22:12:40 -0500
From:	Ted Ts'o <tytso@....edu>
To:	Jan Kara <jack@...e.cz>
Cc:	Toshiyuki Okajima <toshi.okajima@...fujitsu.com>,
	adilger.kernel@...ger.ca, Yongqiang Yang <xiaoqiangnk@...il.com>,
	linux-ext4@...r.kernel.org
Subject: Re: [PATCH 1/2] jbd2: jbd2_journal_stop needs an exclusive control
 to synchronize around t_update operations

On Tue, Jan 03, 2012 at 04:32:45PM +0100, Jan Kara wrote:
>   Thanks for the analysis. Actually, you fix adds unnecessary overhead.
> The problem really is the wrong ordering of prepare_to_wait() and t_updates
> check. So attached patch should fix the issue as well without introducing
> the overhead.

Thanks, applied.

						- Ted


> From 1cd5b8178893f3f186ce93eb1f47664a1a3e81fc Mon Sep 17 00:00:00 2001
> From: Jan Kara <jack@...e.cz>
> Date: Tue, 3 Jan 2012 16:13:29 +0100
> Subject: [PATCH] jbd2: Fix hung processes in jbd2_journal_lock_updates()
> 
> Toshiyuki Okajima found out that when running
> 
> for ((i=0; i < 100000; i++)); do
>         if ((i%2 == 0)); then
>                 chattr +j /mnt/file
>         else
>                 chattr -j /mnt/file
>         fi
>         echo "0" >> /mnt/file
> done
> 
> process sometimes hangs indefinitely in jbd2_journal_lock_updates().
> 
> Toshiyuki identified that the following race happens:
> 
> jbd2_journal_lock_updates()            |jbd2_journal_stop()
> ---------------------------------------+---------------------------------------
>  write_lock(&journal->j_state_lock)    |    .
>  ++journal->j_barrier_count            |    .
>  spin_lock(&tran->t_handle_lock)       |    .
>  atomic_read(&tran->t_updates) //not 0 |
>                                        | atomic_dec_and_test(&tran->t_updates)
>                                        |    // t_updates = 0
>                                        | wake_up(&journal->j_wait_updates)
>  prepare_to_wait()                     |    // no process is woken up.
>  spin_unlock(&tran->t_handle_lock)     |
>  write_unlock(&journal->j_state_lock)  |
>  schedule() // never return            |
> 
> We fix the problem by first calling prepare_to_wait() and only after that
> checking t_updates in jbd2_journal_lock_updates().
> 
> Reported-and-analyzed-by: Toshiyuki Okajima <toshi.okajima@...fujitsu.com>
> Signed-off-by: Jan Kara <jack@...e.cz>
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html