linux-kernel - Re: ext3 on latest -git: BUG: unable to handle kernel NULL pointer dereference at 0000000c

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: <20080718105152.GB15844@unused.rdu.redhat.com>
Date:	Fri, 18 Jul 2008 06:51:52 -0400
From:	Josef Bacik <jbacik@...hat.com>
To:	Andreas Dilger <adilger@....com>
Cc:	Josef Bacik <jbacik@...hat.com>,
	Vegard Nossum <vegard.nossum@...il.com>,
	Josef Bacik <josef@...icpanda.com>, linux-ext4@...r.kernel.org,
	sct@...hat.com, akpm@...ux-foundation.org,
	Johannes Weiner <hannes@...urebad.de>,
	linux-kernel@...r.kernel.org
Subject: Re: ext3 on latest -git: BUG: unable to handle kernel NULL pointer
	dereference at 0000000c

On Thu, Jul 17, 2008 at 05:09:05PM -0600, Andreas Dilger wrote:
> On Jul 17, 2008  10:43 -0400, Josef Bacik wrote:
> > Yeah thats a hard to answer question, one that I will leave up to others
> > who have been doing this much longer than I.  My thought is remount-ro
> > is there to keep you from crashing, so if you have errors=continue then
> > you expect to live with the consequences.  Course if that bit gets flipped
> > via corruption thats not good either.
> 
> It shouldn't cause the kernel to crash, but it should definitely return
> an error to the application.  This is probably one of the code paths
> that the Coverity folks were reporting on in FAST this year where on-disk
> errors are not propagated to the application.

Ok, please revert the previous patch and apply this one.  On errors=continue we
will just abort the handle which should keep the NULL pointer dereference from
happening and return an error back to the application.  Please let me know how
this works Vegard, and thanks alot for testing all this.

Signed-off-by: Josef Bacik <jbacik@...hat.com>

Index: linux-2.6/fs/ext3/inode.c
===================================================================
--- linux-2.6.orig/fs/ext3/inode.c
+++ linux-2.6/fs/ext3/inode.c
@@ -2023,13 +2023,27 @@ static void ext3_clear_blocks(handle_t *
 		unsigned long count, __le32 *first, __le32 *last)
 {
 	__le32 *p;
+	int ret;
+
 	if (try_to_extend_transaction(handle, inode)) {
 		if (bh) {
 			BUFFER_TRACE(bh, "call ext3_journal_dirty_metadata");
-			ext3_journal_dirty_metadata(handle, bh);
+			ret = ext3_journal_dirty_metadata(handle, bh);
+			if (ret) {
+				ext3_std_error(inode->i_sb, ret);
+				return;
+			}
 		}
-		ext3_mark_inode_dirty(handle, inode);
-		ext3_journal_test_restart(handle, inode);
+		ret = ext3_mark_inode_dirty(handle, inode);
+		if (ret)
+			return;
+
+		ret = ext3_journal_test_restart(handle, inode);
+		if (ret) {
+			ext3_std_error(inode->i_sb, ret);
+			return;
+		}
+
 		if (bh) {
 			BUFFER_TRACE(bh, "retaking write access");
 			ext3_journal_get_write_access(handle, bh);
Index: linux-2.6/fs/ext3/balloc.c
===================================================================
--- linux-2.6.orig/fs/ext3/balloc.c
+++ linux-2.6/fs/ext3/balloc.c
@@ -498,6 +498,7 @@ void ext3_free_blocks_sb(handle_t *handl
 		ext3_error (sb, "ext3_free_blocks",
 			    "Freeing blocks not in datazone - "
 			    "block = "E3FSBLK", count = %lu", block, count);
+		err = -EIO;
 		goto error_return;
 	}
 
@@ -535,6 +536,7 @@ do_more:
 			    "Freeing blocks in system zones - "
 			    "Block = "E3FSBLK", count = %lu",
 			    block, count);
+		err = -EIO;
 		goto error_return;
 	}
 
Index: linux-2.6/fs/ext3/super.c
===================================================================
--- linux-2.6.orig/fs/ext3/super.c
+++ linux-2.6/fs/ext3/super.c
@@ -167,7 +167,15 @@ static void ext3_handle_error(struct sup
 		EXT3_SB(sb)->s_mount_opt |= EXT3_MOUNT_ABORT;
 		if (journal)
 			journal_abort(journal, -EIO);
+	} else {
+		handle_t *handle = current->journal_info;
+		if (handle && !is_handle_aborted(handle)) {
+			if (!handle->h_err)
+				handle->h_err = -EIO;
+			journal_abort_handle(handle);
+		}
 	}
+
 	if (test_opt (sb, ERRORS_RO)) {
 		printk (KERN_CRIT "Remounting filesystem read-only\n");
 		sb->s_flags |= MS_RDONLY;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/