jbd2: fix use after free in kjournald2() [Linux 3.18.107]

This Linux kernel change "jbd2: fix use after free in kjournald2()" is included in the Linux 3.18.107 release. This change is authored by Sahitya Tummala <stummala [at] codeaurora.org> on Wed Feb 1 20:49:35 2017 -0500. The commit for this change in Linux stable tree is f47bd1b (patch) which is from upstream commit dbfcef6. The same Linux upstream change may have been applied to various maintained Linux releases and you can find all Linux releases containing changes from upstream dbfcef6.

jbd2: fix use after free in kjournald2()

commit dbfcef6b0f4012c57bc0b6e0e660d5ed12a5eaed upstream.

Below is the synchronization issue between unmount and kjournald2
contexts, which results into use after free issue in kjournald2().
Fix this issue by using journal->j_state_lock to synchronize the
wait_event() done in journal_kill_thread() and the wake_up() done
in kjournald2().

TASK 1:
umount cmd:
   |--jbd2_journal_destroy() {
       |--journal_kill_thread() {
            write_lock(&journal->j_state_lock);
        journal->j_flags |= JBD2_UNMOUNT;
        ...
        write_unlock(&journal->j_state_lock);
        wake_up(&journal->j_wait_commit);      TASK 2 wakes up here:
                               kjournald2() {
                             ...
                             checks JBD2_UNMOUNT flag and calls goto end-loop;
                             ...
                             end_loop:
                               write_unlock(&journal->j_state_lock);
                               journal->j_task = NULL; --> If this thread gets
                               pre-empted here, then TASK 1 wait_event will
                               exit even before this thread is completely
                               done.
        wait_event(journal->j_wait_done_commit, journal->j_task == NULL);
        ...
        write_lock(&journal->j_state_lock);
        write_unlock(&journal->j_state_lock);
      }
       |--kfree(journal);
     }
}
                               wake_up(&journal->j_wait_done_commit); --> this step
                               now results into use after free issue.
                           }

Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: Amit Pundir <amit.pundir@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

There are 2 lines of Linux source code added/deleted in this change. Code changes to Linux kernel are as follows.

 fs/jbd2/journal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 2dc76fe..e4f3965 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -275,11 +275,11 @@ static int kjournald2(void *arg)
    goto loop;

 end_loop:
-   write_unlock(&journal->j_state_lock);
    del_timer_sync(&journal->j_commit_timer);
    journal->j_task = NULL;
    wake_up(&journal->j_wait_done_commit);
    jbd_debug(1, "Journal thread exiting.n");
+   write_unlock(&journal->j_state_lock);
    return 0;
 }

Leave a Reply

Your email address will not be published. Required fields are marked *