Re: kernel NULL pointer dereference at rb_erase+0x1a3/0x370

Hugh Dickins
2012-10-02T20:28:40Z

On Tue, 2 Oct 2012, Shuah Khan wrote:
> I started seeing the following null pointer dereference on
> a linux-next sept 21 git and still seeing it on linux-next
> Sep 27th git.
>
> Can be reproduced easily. I have been able to reproduce every
> time I do a complete build of a kernel on fresh checkout or
> touch a header file that forces full build.
>
> Related to lib/rbtree.c commits that went into September 21 perhaps.

That's a sensible suggestion, but I think probably not.

> I didn't get a chance to investigate this yet, thought I would share
> just in case others have seen it.
>
>
> [ 2219.660124] BUG: unable to handle kernel NULL pointer dereference at (null)
> [ 2219.660182] IP: [<ffffffff81323c93>] rb_erase+0x1a3/0x370
> [ 2219.660209] PGD 73f2f067 PUD 67246067 PMD 0
> [ 2219.660235] Oops: 0000 [#1] SMP
> [ 2219.660632] CPU 0
> [ 2219.660644] Pid: 1660, comm: recordmcount Not tainted 3.6.0-rc6-next-20120921+ #4 Hewlett-Packard HP EliteBook 6930p/30DC
> [ 2219.664007] Process recordmcount (pid: 1660, threadinfo ffff880073f30000, task ffff88002fe9ada0)
> [ 2219.664007] Call Trace:
> [ 2219.664007] [<ffffffff8125af6e>] ext4_es_insert_extent+0x28e/0x2f0

ext4_es_insert_extent: this is probably something I posted a fix for on
the 27th (copied below). They're reworking that series more thoroughly,
and it has for the moment dropped out of linux-next, so I expect you
won't see this error at all with a newer next. But you may want to
continue with your Sep 27th tree, and try out this patch anyway...

[PATCH next/mmotm] ext4: fix cache_es after merge_left

Kernel build with CONFIG_DEBUG_SLAB or CONFIG_SLUB_DEBUG slub_debug=FPZ
gives me kernel BUG at fs/ext4/extents_status.c:142! That's the
BUG_ON(es->start + es->len < es->start) in extent_status_end() called
from ext4_es_insert_extent(). tree->cache_es has been freed and poisoned.

This comes from when ext4_es_try_to_merge_left() merges es into leftward
es1, but ext4_es_insert_extent()'s out then updates cache_es to the freed
extent_status. ext4_es_try_to_merge_right() does not pose a problem.

Change ext4_es_try_to_merge_left() to return whichever extent_status
should be recorded in tree->cache_es. Remove cache_es update from
both of them, leaving that to ext4_es_insert_extent()'s out label.

Signed-off-by: Hugh Dickins <hughd@google.com>
---
fs/ext4/extents_status.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)

--- mmotm/fs/ext4/extents_status.c 2012-09-26 10:15:29.340071552 -0700
+++ linux/fs/ext4/extents_status.c 2012-09-27 11:52:59.284937056 -0700
@@ -244,24 +244,24 @@ static void ext4_es_free_extent(struct e
kmem_cache_free(ext4_es_cachep, es);
}

-static void ext4_es_try_to_merge_left(struct ext4_es_tree *tree,
- struct extent_status *es)
+static struct extent_status *
+ext4_es_try_to_merge_left(struct ext4_es_tree *tree, struct extent_status *es)
{
struct extent_status *es1;
struct rb_node *node;

node = rb_prev(&es->rb_node);
if (!node)
- return;
+ return es;

es1 = rb_entry(node, struct extent_status, rb_node);
if (es->start == extent_status_end(es1) + 1) {
es1->len += es->len;
rb_erase(&es->rb_node, &tree->root);
- if (es == tree->cache_es)
- tree->cache_es = es1;
ext4_es_free_extent(es);
+ es = es1; /* Caller will update tree->cache_es to this */
}
+ return es;
}

static void ext4_es_try_to_merge_right(struct ext4_es_tree *tree,
@@ -278,9 +278,8 @@ static void ext4_es_try_to_merge_right(s
if (es1->start == extent_status_end(es) + 1) {
es->len += es1->len;
rb_erase(node, &tree->root);
- if (es1 == tree->cache_es)
- tree->cache_es = es;
ext4_es_free_extent(es1);
+ /* Caller will update tree->cache_es to es */
}
}

@@ -318,7 +317,7 @@ int ext4_es_insert_extent(struct inode *
es_debug("cached by [%u/%u)\n", es->start, es->len);
es->start = offset;
es->len += len;
- ext4_es_try_to_merge_left(tree, es);
+ es = ext4_es_try_to_merge_left(tree, es);
goto out;
} else if (es && in_range(offset, es->start, es->len)) {
es_debug("cached by [%u/%u)\n", es->start, es->len);