summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLukas Czerner <lczerner@redhat.com>2016-10-28 17:29:18 +0200
committerTasayco Loarte, VictorX <victorx.tasayco.loarte@intel.com>2016-11-30 08:36:08 -0800
commit0a20e44b6d497ba394d52627676bfe5a3a034d95 (patch)
tree6762527eeef3a72f4898afeeb1f7d55334c3f151
parent8ecedf4cce6dba914c946d9cde0c9917ecc0c316 (diff)
downloadx86-0a20e44b6d497ba394d52627676bfe5a3a034d95.tar.gz
ext4: fix NULL pointer dereference when journal restart fails
Currently when journal restart fails, we'll have the h_transaction of the handle set to NULL to indicate that the handle has been effectively aborted. We handle this situation quietly in the jbd2_journal_stop() and just free the handle and exit because everything else has been done before we attempted (and failed) to restart the journal. Unfortunately there are a number of problems with that approach introduced with commit 41a5b913197c "jbd2: invalidate handle if jbd2_journal_restart() fails" First of all in ext4 jbd2_journal_stop() will be called through __ext4_journal_stop() where we would try to get a hold of the superblock by dereferencing h_transaction which in this case would lead to NULL pointer dereference and crash. In addition we're going to free the handle regardless of the refcount which is bad as well, because others up the call chain will still reference the handle so we might potentially reference already freed memory. Moreover it's expected that we'll get aborted handle as well as detached handle in some of the journalling function as the error propagates up the stack, so it's unnecessary to call WARN_ON every time we get detached handle. And finally we might leak some memory by forgetting to free reserved handle in jbd2_journal_stop() in the case where handle was detached from the transaction (h_transaction is NULL). Fix the NULL pointer dereference in __ext4_journal_stop() by just calling jbd2_journal_stop() quietly as suggested by Jan Kara. Also fix the potential memory leak in jbd2_journal_stop() and use proper handle refcounting before we attempt to free it to avoid use-after-free issues. And finally remove all WARN_ON(!transaction) from the code so that we do not get random traces when something goes wrong because when journal restart fails we will get to some of those functions. Change-Id: I29d9ceb812c72caf2f6b9ef0e6a9a79b4e52c69b Tracked-On: https://jira01.devtools.intel.com/browse/AW-2637 Cc: stable@vger.kernel.org Signed-off-by: Lukas Czerner <lczerner@redhat.com> Signed-off-by: Theodore Ts'o <tytso@mit.edu> Reviewed-by: Jan Kara <jack@suse.cz> Reviewed-on: https://android.intel.com/549191 Reviewed-by: Tasayco Loarte, VictorX <victorx.tasayco.loarte@intel.com> Tested-by: Tasayco Loarte, VictorX <victorx.tasayco.loarte@intel.com> Reviewed-by: Maalem, Saadi <saadi.maalem@intel.com>
-rw-r--r--fs/ext4/ext4_jbd2.c6
1 files changed, 6 insertions, 0 deletions
diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c
index 1be3996b5942..fa41043cb5c8 100644
--- a/fs/ext4/ext4_jbd2.c
+++ b/fs/ext4/ext4_jbd2.c
@@ -75,6 +75,12 @@ int __ext4_journal_stop(const char *where, unsigned int line, handle_t *handle)
ext4_put_nojournal(handle);
return 0;
}
+
+ if (!handle->h_transaction) {
+ err = jbd2_journal_stop(handle);
+ return handle->h_err ? handle->h_err : err;
+ }
+
sb = handle->h_transaction->t_journal->j_private;
err = handle->h_err;
rc = jbd2_journal_stop(handle);