Thread (29 messages) 29 messages, 8 authors, 2012-09-17

Re: NULL pointer dereference in ext4_ext_remove_space on 3.5.1

From: Maciej Żenczykowski <hidden>
Date: 2012-08-16 20:21:15
Also in: lkml

This would probably be much more readable code if the 'i=0' init was
before path=kzalloc.

On Thu, Aug 16, 2012 at 8:25 AM, Theodore Ts'o [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On Thu, Aug 16, 2012 at 07:10:51PM +0800, Fengguang Wu wrote:
quoted
Here is the dmesg. BTW, it seems 3.5.0 don't have this issue.
Fengguang,

It sounds like you have a (at least fairly) reliable reproduction for
this problem?  Is it something you can share?  It would be good to get
this into our test suites, since it was _not_ something that was
caught by xfstests, apparently.

Can you see if this patch addresses it?  (The first two patch hunks
are the same debugging additions I had posted before.)

It looks like the responsible commit is 968dee7722: "ext4: fix hole
punch failure when depth is greater than 0".  I had thought this patch
was low risk if you weren't using the new punch ioctl, but it turns
out it did make a critical change in the non-punch (i.e., truncate)
code path, which is what the addition of "i = 0;" in the patch below
addresses.

Regards,

                                          - Ted
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 769151d..fa829dc 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2432,6 +2432,10 @@ ext4_ext_rm_leaf(handle_t *handle, struct inode *inode,

        /* the header must be checked already in ext4_ext_remove_space() */
        ext_debug("truncate since %u in leaf to %u\n", start, end);
+       if (!path[depth].p_hdr && !path[depth].p_bh) {
+               EXT4_ERROR_INODE(inode, "depth %d", depth);
+               BUG_ON(1);
+       }
        if (!path[depth].p_hdr)
                path[depth].p_hdr = ext_block_hdr(path[depth].p_bh);
        eh = path[depth].p_hdr;
@@ -2730,6 +2734,10 @@ cont:
                /* this is index block */
                if (!path[i].p_hdr) {
                        ext_debug("initialize header\n");
+                       if (!path[i].p_hdr && !path[i].p_bh) {
+                               EXT4_ERROR_INODE(inode, "i=%d", i);
+                               BUG_ON(1);
+                       }
                        path[i].p_hdr = ext_block_hdr(path[i].p_bh);
                }
@@ -2828,6 +2836,7 @@ out:
        kfree(path);
        if (err == -EAGAIN) {
                path = NULL;
+               i = 0;
                goto again;
        }
        ext4_journal_stop(handle);


-- 
Maciej A. Żenczykowski
Kernel Networking Developer @ Google
1600 Amphitheatre Parkway, Mountain View, CA 94043
tel: +1 (650) 253-0062
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help