Re: Fw: [Bugme-new] [Bug 9849] New: NULL pointer deref in journal_wait_on_commit_record
On Wed, Jan 30, 2008 at 03:17:57PM -0800, Mingming Cao wrote: > > The buufer head pointer passed to journal_wait_on_commit_record() could > be NULL if the previous journal_submit_commit_record() failed or journal > has already aborted. > > Looking at the jbd2 debug messages, before the oops happen, the jbd2 is > aborted due to trying to access the next log block beyond the end of > device. This might be caused by using a corrupted image. > > We need to check the error returns from journal_submit_commit_record() > and avoid calling journal_wait_on_commit_record() in the failure case. > > Signed-off-by: Mingming Cao <[EMAIL PROTECTED]> > The buufer head pointer passed to journal_wait_on_commit_record() > could be NULL if the previous journal_submit_commit_record() failed > or journal has already aborted. > > We need to check the error returns from journal_submit_commit_record() > and avoid calling journal_wait_on_commit_record() in the failure case. > > Signed-off-by: Mingming Cao <[EMAIL PROTECTED]> > --- > fs/jbd2/commit.c |3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > Index: linux-2.6.24-rc8/fs/jbd2/commit.c > === > --- linux-2.6.24-rc8.orig/fs/jbd2/commit.c2008-01-30 14:12:10.0 > -0800 > +++ linux-2.6.24-rc8/fs/jbd2/commit.c 2008-01-30 15:09:50.0 -0800 > @@ -872,7 +872,8 @@ wait_for_iobuf: > if (err) > __jbd2_journal_abort_hard(journal); > } > - err = journal_wait_on_commit_record(cbh); > + if (!err && !is_journal_aborted(journal)) > + err = journal_wait_on_commit_record(cbh); > > if (err) > jbd2_journal_abort(journal, err); > > Needs the below small change also. I don't see this patch in the patch queue. So i guess we can add the below diff to the same. The change was suggested by Girish. Before journal checksum changes sync_dirty_buffer did the get_bh. Signed-off-by: Aneesh Kumar K.V <[EMAIL PROTECTED]> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c index da8d0eb..2b88ab0 100644 --- a/fs/jbd2/commit.c +++ b/fs/jbd2/commit.c @@ -136,7 +136,7 @@ static int journal_submit_commit_record(journal_t *journal, JBUFFER_TRACE(descriptor, "submit commit block"); lock_buffer(bh); - + get_bh(bh); set_buffer_dirty(bh); set_buffer_uptodate(bh); bh->b_end_io = journal_end_buffer_io_sync; - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fw: [Bugme-new] [Bug 9849] New: NULL pointer deref in journal_wait_on_commit_record
* Mingming Cao ([EMAIL PROTECTED]) wrote: > We need to check the error returns from journal_submit_commit_record() > and avoid calling journal_wait_on_commit_record() in the failure case. > > Signed-off-by: Mingming Cao <[EMAIL PROTECTED]> thanks, the patch works for me, i closed the bugzilla entry - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fw: [Bugme-new] [Bug 9849] New: NULL pointer deref in journal_wait_on_commit_record
On Wed, 30 Jan 2008 15:17:57 -0800 Mingming Cao <[EMAIL PROTECTED]> wrote: > The buufer head pointer passed to journal_wait_on_commit_record() > could be NULL if the previous journal_submit_commit_record() failed > or journal has already aborted. > > We need to check the error returns from journal_submit_commit_record() > and avoid calling journal_wait_on_commit_record() in the failure case. > > Signed-off-by: Mingming Cao <[EMAIL PROTECTED]> > --- > fs/jbd2/commit.c |3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > Index: linux-2.6.24-rc8/fs/jbd2/commit.c > === > --- linux-2.6.24-rc8.orig/fs/jbd2/commit.c2008-01-30 14:12:10.0 > -0800 > +++ linux-2.6.24-rc8/fs/jbd2/commit.c 2008-01-30 15:09:50.0 -0800 > @@ -872,7 +872,8 @@ wait_for_iobuf: > if (err) > __jbd2_journal_abort_hard(journal); > } > - err = journal_wait_on_commit_record(cbh); > + if (!err && !is_journal_aborted(journal)) > + err = journal_wait_on_commit_record(cbh); > > if (err) > jbd2_journal_abort(journal, err); Thanks. Please note that I Cc'ed [EMAIL PROTECTED] on this, for a 2.6.24.x backport. - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fw: [Bugme-new] [Bug 9849] New: NULL pointer deref in journal_wait_on_commit_record
On Wed, 2008-01-30 at 12:00 -0800, Andrew Morton wrote: > > Begin forwarded message: > > Date: Wed, 30 Jan 2008 03:24:08 -0800 (PST) > From: [EMAIL PROTECTED] > To: [EMAIL PROTECTED] > Subject: [Bugme-new] [Bug 9849] New: NULL pointer deref in > journal_wait_on_commit_record > > > http://bugzilla.kernel.org/show_bug.cgi?id=9849 > >Summary: NULL pointer deref in journal_wait_on_commit_record >Product: File System >Version: 2.5 > KernelVersion: 2.6.24-03997-g85004cc > Platform: All > OS/Version: Linux > Tree: Mainline > Status: NEW > Severity: normal > Priority: P1 > Component: ext4 > AssignedTo: [EMAIL PROTECTED] > ReportedBy: [EMAIL PROTECTED] > > > Latest working kernel version: - > Earliest failing kernel version: 2.6.24-03863-g0ba6c33 > Distribution: Ubuntu > Problem Description: > > using a corrupted image causes an oops in unmount, seems as if > journal_wait_on_commit_record() gets passed a NULL pointer > The buufer head pointer passed to journal_wait_on_commit_record() could be NULL if the previous journal_submit_commit_record() failed or journal has already aborted. Looking at the jbd2 debug messages, before the oops happen, the jbd2 is aborted due to trying to access the next log block beyond the end of device. This might be caused by using a corrupted image. We need to check the error returns from journal_submit_commit_record() and avoid calling journal_wait_on_commit_record() in the failure case. Signed-off-by: Mingming Cao <[EMAIL PROTECTED]> The buufer head pointer passed to journal_wait_on_commit_record() could be NULL if the previous journal_submit_commit_record() failed or journal has already aborted. We need to check the error returns from journal_submit_commit_record() and avoid calling journal_wait_on_commit_record() in the failure case. Signed-off-by: Mingming Cao <[EMAIL PROTECTED]> --- fs/jbd2/commit.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Index: linux-2.6.24-rc8/fs/jbd2/commit.c === --- linux-2.6.24-rc8.orig/fs/jbd2/commit.c 2008-01-30 14:12:10.0 -0800 +++ linux-2.6.24-rc8/fs/jbd2/commit.c 2008-01-30 15:09:50.0 -0800 @@ -872,7 +872,8 @@ wait_for_iobuf: if (err) __jbd2_journal_abort_hard(journal); } - err = journal_wait_on_commit_record(cbh); + if (!err && !is_journal_aborted(journal)) + err = journal_wait_on_commit_record(cbh); if (err) jbd2_journal_abort(journal, err); - To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
