* [PATCH 4.9-4.19] jbd2: fix use-after-free of transaction_t race
@ 2022-04-26 18:27 Samuel Mendoza-Jonas
2022-04-26 18:27 ` [PATCH 5.4-5.16] " Samuel Mendoza-Jonas
2022-04-27 16:31 ` [PATCH 4.9-4.19] " Samuel Mendoza-Jonas
0 siblings, 2 replies; 4+ messages in thread
From: Samuel Mendoza-Jonas @ 2022-04-26 18:27 UTC (permalink / raw)
To: stable
From: Ritesh Harjani <riteshh@linux.ibm.com>
commit cc16eecae687912238ee6efbff71ad31e2bc414e upstream.
jbd2_journal_wait_updates() is called with j_state_lock held. But if
there is a commit in progress, then this transaction might get committed
and freed via jbd2_journal_commit_transaction() ->
jbd2_journal_free_transaction(), when we release j_state_lock.
So check for journal->j_running_transaction everytime we release and
acquire j_state_lock to avoid use-after-free issue.
Link: https://lore.kernel.org/r/948c2fed518ae739db6a8f7f83f1d58b504f87d0.1644497105.git.ritesh.list@gmail.com
Fixes: 4f98186848707f53 ("jbd2: refactor wait logic for transaction updates into a common function")
Cc: stable@kernel.org
Reported-and-tested-by: syzbot+afa2ca5171d93e44b348@syzkaller.appspotmail.com
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
[backport to 4.9-4.19 in original jbd2_journal_commit_transaction()
location before the refactor in
4f9818684870 "jbd2: refactor wait logic for transaction updates into a
common function"]
Signed-off-by: Samuel Mendoza-Jonas <samjonas@amazon.com>
Fixes: 1da177e4c3f41524
Cc: stable@kernel.org # 4.9.x - 4.19.x
---
While marked for 5.17 stable, it looks like this fix also applies to the
original location in jbd2_journal_commit_transaction() before it was
refactored to use jbd2_journal_wait_updates(). This applies the same
change there.
fs/jbd2/commit.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index 97760cb9bcd7..66e776eb5ea7 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -382,6 +382,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
int csum_size = 0;
LIST_HEAD(io_bufs);
LIST_HEAD(log_bufs);
+ DEFINE_WAIT(wait);
if (jbd2_journal_has_csum_v2or3(journal))
csum_size = sizeof(struct jbd2_journal_block_tail);
@@ -434,22 +435,25 @@ void jbd2_journal_commit_transaction(journal_t *journal)
stats.run.rs_running = jbd2_time_diff(commit_transaction->t_start,
stats.run.rs_locked);
- spin_lock(&commit_transaction->t_handle_lock);
- while (atomic_read(&commit_transaction->t_updates)) {
- DEFINE_WAIT(wait);
+ while (1) {
+ commit_transaction = journal->j_running_transaction;
+ if (!commit_transaction)
+ break;
+ spin_lock(&commit_transaction->t_handle_lock);
prepare_to_wait(&journal->j_wait_updates, &wait,
TASK_UNINTERRUPTIBLE);
- if (atomic_read(&commit_transaction->t_updates)) {
+ if (!atomic_read(&commit_transaction->t_updates)) {
spin_unlock(&commit_transaction->t_handle_lock);
- write_unlock(&journal->j_state_lock);
- schedule();
- write_lock(&journal->j_state_lock);
- spin_lock(&commit_transaction->t_handle_lock);
+ finish_wait(&journal->j_wait_updates, &wait);
+ break;
}
+ spin_unlock(&commit_transaction->t_handle_lock);
+ write_unlock(&journal->j_state_lock);
+ schedule();
finish_wait(&journal->j_wait_updates, &wait);
+ write_lock(&journal->j_state_lock);
}
- spin_unlock(&commit_transaction->t_handle_lock);
J_ASSERT (atomic_read(&commit_transaction->t_outstanding_credits) <=
journal->j_max_transaction_buffers);
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 5.4-5.16] jbd2: fix use-after-free of transaction_t race
2022-04-26 18:27 [PATCH 4.9-4.19] jbd2: fix use-after-free of transaction_t race Samuel Mendoza-Jonas
@ 2022-04-26 18:27 ` Samuel Mendoza-Jonas
2022-04-27 16:31 ` [PATCH 4.9-4.19] " Samuel Mendoza-Jonas
1 sibling, 0 replies; 4+ messages in thread
From: Samuel Mendoza-Jonas @ 2022-04-26 18:27 UTC (permalink / raw)
To: stable
From: Ritesh Harjani <riteshh@linux.ibm.com>
commit cc16eecae687912238ee6efbff71ad31e2bc414e upstream.
jbd2_journal_wait_updates() is called with j_state_lock held. But if
there is a commit in progress, then this transaction might get committed
and freed via jbd2_journal_commit_transaction() ->
jbd2_journal_free_transaction(), when we release j_state_lock.
So check for journal->j_running_transaction everytime we release and
acquire j_state_lock to avoid use-after-free issue.
Link: https://lore.kernel.org/r/948c2fed518ae739db6a8f7f83f1d58b504f87d0.1644497105.git.ritesh.list@gmail.com
Fixes: 4f98186848707f53 ("jbd2: refactor wait logic for transaction updates into a common function")
Cc: stable@kernel.org
Reported-and-tested-by: syzbot+afa2ca5171d93e44b348@syzkaller.appspotmail.com
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
[backport to 5.4-5.16 in original jbd2_journal_commit_transaction()
location before the refactor in
4f9818684870 "jbd2: refactor wait logic for transaction updates into a
common function"]
Signed-off-by: Samuel Mendoza-Jonas <samjonas@amazon.com>
Fixes: 1da177e4c3f41524
Cc: stable@kernel.org # 5.4.x - 5.16.x
---
While marked for 5.17 stable, it looks like this fix also applies to the
original location in jbd2_journal_commit_transaction() before it was
refactored to use jbd2_journal_wait_updates(). This applies the same
change there.
fs/jbd2/commit.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index d188fa913a07..e9ed8a71ff25 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -408,6 +408,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
int csum_size = 0;
LIST_HEAD(io_bufs);
LIST_HEAD(log_bufs);
+ DEFINE_WAIT(wait);
if (jbd2_journal_has_csum_v2or3(journal))
csum_size = sizeof(struct jbd2_journal_block_tail);
@@ -484,22 +485,25 @@ void jbd2_journal_commit_transaction(journal_t *journal)
stats.run.rs_running = jbd2_time_diff(commit_transaction->t_start,
stats.run.rs_locked);
- spin_lock(&commit_transaction->t_handle_lock);
- while (atomic_read(&commit_transaction->t_updates)) {
- DEFINE_WAIT(wait);
+ while (1) {
+ commit_transaction = journal->j_running_transaction;
+ if (!commit_transaction)
+ break;
+ spin_lock(&commit_transaction->t_handle_lock);
prepare_to_wait(&journal->j_wait_updates, &wait,
TASK_UNINTERRUPTIBLE);
- if (atomic_read(&commit_transaction->t_updates)) {
+ if (!atomic_read(&commit_transaction->t_updates)) {
spin_unlock(&commit_transaction->t_handle_lock);
- write_unlock(&journal->j_state_lock);
- schedule();
- write_lock(&journal->j_state_lock);
- spin_lock(&commit_transaction->t_handle_lock);
+ finish_wait(&journal->j_wait_updates, &wait);
+ break;
}
+ spin_unlock(&commit_transaction->t_handle_lock);
+ write_unlock(&journal->j_state_lock);
+ schedule();
finish_wait(&journal->j_wait_updates, &wait);
+ write_lock(&journal->j_state_lock);
}
- spin_unlock(&commit_transaction->t_handle_lock);
commit_transaction->t_state = T_SWITCH;
write_unlock(&journal->j_state_lock);
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 4.9-4.19] jbd2: fix use-after-free of transaction_t race
2022-04-26 18:27 [PATCH 4.9-4.19] jbd2: fix use-after-free of transaction_t race Samuel Mendoza-Jonas
2022-04-26 18:27 ` [PATCH 5.4-5.16] " Samuel Mendoza-Jonas
@ 2022-04-27 16:31 ` Samuel Mendoza-Jonas
2022-04-29 8:59 ` Greg KH
1 sibling, 1 reply; 4+ messages in thread
From: Samuel Mendoza-Jonas @ 2022-04-27 16:31 UTC (permalink / raw)
To: stable
On Tue, Apr 26, 2022 at 11:27:01AM -0700, Samuel Mendoza-Jonas wrote:
> From: Ritesh Harjani <riteshh@linux.ibm.com>
>
> commit cc16eecae687912238ee6efbff71ad31e2bc414e upstream.
>
> jbd2_journal_wait_updates() is called with j_state_lock held. But if
> there is a commit in progress, then this transaction might get committed
> and freed via jbd2_journal_commit_transaction() ->
> jbd2_journal_free_transaction(), when we release j_state_lock.
> So check for journal->j_running_transaction everytime we release and
> acquire j_state_lock to avoid use-after-free issue.
>
> Link: https://lore.kernel.org/r/948c2fed518ae739db6a8f7f83f1d58b504f87d0.1644497105.git.ritesh.list@gmail.com
> Fixes: 4f98186848707f53 ("jbd2: refactor wait logic for transaction updates into a common function")
> Cc: stable@kernel.org
> Reported-and-tested-by: syzbot+afa2ca5171d93e44b348@syzkaller.appspotmail.com
> Reviewed-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> [backport to 4.9-4.19 in original jbd2_journal_commit_transaction()
> location before the refactor in
> 4f9818684870 "jbd2: refactor wait logic for transaction updates into a
> common function"]
> Signed-off-by: Samuel Mendoza-Jonas <samjonas@amazon.com>
> Fixes: 1da177e4c3f41524
> Cc: stable@kernel.org # 4.9.x - 4.19.x
> ---
> While marked for 5.17 stable, it looks like this fix also applies to the
> original location in jbd2_journal_commit_transaction() before it was
> refactored to use jbd2_journal_wait_updates(). This applies the same
> change there.
Jan kindly pointed out this was a false alarm:
https://lore.kernel.org/all/20220427111726.3wdyxbqoxs7skdzf@quack3.lan/
So the existing patch is fine and these can be ignored!
Cheers,
Sam
>
> fs/jbd2/commit.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index 97760cb9bcd7..66e776eb5ea7 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -382,6 +382,7 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> int csum_size = 0;
> LIST_HEAD(io_bufs);
> LIST_HEAD(log_bufs);
> + DEFINE_WAIT(wait);
>
> if (jbd2_journal_has_csum_v2or3(journal))
> csum_size = sizeof(struct jbd2_journal_block_tail);
> @@ -434,22 +435,25 @@ void jbd2_journal_commit_transaction(journal_t *journal)
> stats.run.rs_running = jbd2_time_diff(commit_transaction->t_start,
> stats.run.rs_locked);
>
> - spin_lock(&commit_transaction->t_handle_lock);
> - while (atomic_read(&commit_transaction->t_updates)) {
> - DEFINE_WAIT(wait);
> + while (1) {
> + commit_transaction = journal->j_running_transaction;
> + if (!commit_transaction)
> + break;
>
> + spin_lock(&commit_transaction->t_handle_lock);
> prepare_to_wait(&journal->j_wait_updates, &wait,
> TASK_UNINTERRUPTIBLE);
> - if (atomic_read(&commit_transaction->t_updates)) {
> + if (!atomic_read(&commit_transaction->t_updates)) {
> spin_unlock(&commit_transaction->t_handle_lock);
> - write_unlock(&journal->j_state_lock);
> - schedule();
> - write_lock(&journal->j_state_lock);
> - spin_lock(&commit_transaction->t_handle_lock);
> + finish_wait(&journal->j_wait_updates, &wait);
> + break;
> }
> + spin_unlock(&commit_transaction->t_handle_lock);
> + write_unlock(&journal->j_state_lock);
> + schedule();
> finish_wait(&journal->j_wait_updates, &wait);
> + write_lock(&journal->j_state_lock);
> }
> - spin_unlock(&commit_transaction->t_handle_lock);
>
> J_ASSERT (atomic_read(&commit_transaction->t_outstanding_credits) <=
> journal->j_max_transaction_buffers);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 4.9-4.19] jbd2: fix use-after-free of transaction_t race
2022-04-27 16:31 ` [PATCH 4.9-4.19] " Samuel Mendoza-Jonas
@ 2022-04-29 8:59 ` Greg KH
0 siblings, 0 replies; 4+ messages in thread
From: Greg KH @ 2022-04-29 8:59 UTC (permalink / raw)
To: Samuel Mendoza-Jonas; +Cc: stable
On Wed, Apr 27, 2022 at 09:31:50AM -0700, Samuel Mendoza-Jonas wrote:
> On Tue, Apr 26, 2022 at 11:27:01AM -0700, Samuel Mendoza-Jonas wrote:
> > From: Ritesh Harjani <riteshh@linux.ibm.com>
> >
> > commit cc16eecae687912238ee6efbff71ad31e2bc414e upstream.
> >
> > jbd2_journal_wait_updates() is called with j_state_lock held. But if
> > there is a commit in progress, then this transaction might get committed
> > and freed via jbd2_journal_commit_transaction() ->
> > jbd2_journal_free_transaction(), when we release j_state_lock.
> > So check for journal->j_running_transaction everytime we release and
> > acquire j_state_lock to avoid use-after-free issue.
> >
> > Link: https://lore.kernel.org/r/948c2fed518ae739db6a8f7f83f1d58b504f87d0.1644497105.git.ritesh.list@gmail.com
> > Fixes: 4f98186848707f53 ("jbd2: refactor wait logic for transaction updates into a common function")
> > Cc: stable@kernel.org
> > Reported-and-tested-by: syzbot+afa2ca5171d93e44b348@syzkaller.appspotmail.com
> > Reviewed-by: Jan Kara <jack@suse.cz>
> > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> > Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> > [backport to 4.9-4.19 in original jbd2_journal_commit_transaction()
> > location before the refactor in
> > 4f9818684870 "jbd2: refactor wait logic for transaction updates into a
> > common function"]
> > Signed-off-by: Samuel Mendoza-Jonas <samjonas@amazon.com>
> > Fixes: 1da177e4c3f41524
> > Cc: stable@kernel.org # 4.9.x - 4.19.x
> > ---
> > While marked for 5.17 stable, it looks like this fix also applies to the
> > original location in jbd2_journal_commit_transaction() before it was
> > refactored to use jbd2_journal_wait_updates(). This applies the same
> > change there.
>
> Jan kindly pointed out this was a false alarm:
> https://lore.kernel.org/all/20220427111726.3wdyxbqoxs7skdzf@quack3.lan/
>
> So the existing patch is fine and these can be ignored!
Thanks for the update, now all dropped from my queue.
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-04-29 8:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-26 18:27 [PATCH 4.9-4.19] jbd2: fix use-after-free of transaction_t race Samuel Mendoza-Jonas
2022-04-26 18:27 ` [PATCH 5.4-5.16] " Samuel Mendoza-Jonas
2022-04-27 16:31 ` [PATCH 4.9-4.19] " Samuel Mendoza-Jonas
2022-04-29 8:59 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox