Btrfs: avoid possible use-after-free in clear_extent_bit()
authorLi Zefan <lizf@cn.fujitsu.com>
Mon, 12 Mar 2012 08:39:48 +0000 (16:39 +0800)
committerDavid Sterba <dsterba@suse.cz>
Wed, 18 Apr 2012 17:22:18 +0000 (19:22 +0200)
clear_extent_bit()
{
    next_node = rb_next(&state->rb_node);
    ...
    clear_state_bit(state);  <-- this may free next_node
    if (next_node) {
        state = rb_entry(next_node);
        ...
    }
}

clear_state_bit() calls merge_state() which may free the next node
of the passing extent_state, so clear_extent_bit() may end up
referencing freed memory.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
fs/btrfs/extent_io.c

index 05951bdf72cc822e90a202d840a439e515e781c1..11eeb81fe69515e66cc01550438eba0ddd5c22dd 100644 (file)
@@ -402,6 +402,15 @@ static int split_state(struct extent_io_tree *tree, struct extent_state *orig,
        return 0;
 }
 
+static struct extent_state *next_state(struct extent_state *state)
+{
+       struct rb_node *next = rb_next(&state->rb_node);
+       if (next)
+               return rb_entry(next, struct extent_state, rb_node);
+       else
+               return NULL;
+}
+
 /*
  * utility function to clear some bits in an extent state struct.
  * it will optionally wake up any one waiting on this state (wake == 1)
@@ -409,10 +418,11 @@ static int split_state(struct extent_io_tree *tree, struct extent_state *orig,
  * If no bits are set on the state struct after clearing things, the
  * struct is freed and removed from the tree
  */
-static void clear_state_bit(struct extent_io_tree *tree,
-                           struct extent_state *state,
-                           int *bits, int wake)
+static struct extent_state *clear_state_bit(struct extent_io_tree *tree,
+                                           struct extent_state *state,
+                                           int *bits, int wake)
 {
+       struct extent_state *next;
        int bits_to_clear = *bits & ~EXTENT_CTLBITS;
 
        if ((bits_to_clear & EXTENT_DIRTY) && (state->state & EXTENT_DIRTY)) {
@@ -425,6 +435,7 @@ static void clear_state_bit(struct extent_io_tree *tree,
        if (wake)
                wake_up(&state->wq);
        if (state->state == 0) {
+               next = next_state(state);
                if (state->tree) {
                        rb_erase(&state->rb_node, &tree->state);
                        state->tree = NULL;
@@ -434,7 +445,9 @@ static void clear_state_bit(struct extent_io_tree *tree,
                }
        } else {
                merge_state(tree, state);
+               next = next_state(state);
        }
+       return next;
 }
 
 static struct extent_state *
@@ -473,7 +486,6 @@ int clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
        struct extent_state *state;
        struct extent_state *cached;
        struct extent_state *prealloc = NULL;
-       struct rb_node *next_node;
        struct rb_node *node;
        u64 last_end;
        int err;
@@ -525,14 +537,11 @@ hit_next:
        WARN_ON(state->end < start);
        last_end = state->end;
 
-       if (state->end < end && !need_resched())
-               next_node = rb_next(&state->rb_node);
-       else
-               next_node = NULL;
-
        /* the state doesn't have the wanted bits, go ahead */
-       if (!(state->state & bits))
+       if (!(state->state & bits)) {
+               state = next_state(state);
                goto next;
+       }
 
        /*
         *     | ---- desired range ---- |
@@ -590,16 +599,13 @@ hit_next:
                goto out;
        }
 
-       clear_state_bit(tree, state, &bits, wake);
+       state = clear_state_bit(tree, state, &bits, wake);
 next:
        if (last_end == (u64)-1)
                goto out;
        start = last_end + 1;
-       if (start <= end && next_node) {
-               state = rb_entry(next_node, struct extent_state,
-                                rb_node);
+       if (start <= end && state && !need_resched())
                goto hit_next;
-       }
        goto search_again;
 
 out: