Re: Bio & Biovec-1 increasing cache size, never freed during disk IO

[Neil Brown]

On Tuesday February 28, mbrancaleoni@xxxxxxxxx wrote:
> Hi Neil,
> 
> seems that the patch that leads to the error is the one signed up by you:
> commit 3795bb0fc52fe2af2749f3ad2185cb9c90871ef8
> Author: NeilBrown <neilb@xxxxxxx>
> Date:   Mon Dec 12 02:39:16 2005 -0800
> 
>     [PATCH] md: fix a use-after-free bug in raid1
> 
>     Who would submit code with a FIXME like that in it !!!!
> 
>     Signed-off-by: Neil Brown <neilb@xxxxxxx>
>     Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
>     Signed-off-by: Linus Torvalds <torvalds@xxxxxxxx>

Thanks for finding this.

There are two bugs here.
One is that if BIO_RW_BARRIER is rejected by one drive but not the
other, then we forget to release a bio that we should have released.

The other is that the test for "should we do barrier IO" was wrong so
that even though one device doesn't support it, raid1 keeps trying it
(but only on read-ahead requests....)

It would seem that the devices in your array are not all on the same
controller.  Is that correct?  There isn't a problem with that, but I
just want to check my understanding of what is happening.

Could you try this patch please and see if it fixes the problem?

Thanks again,
NeilBrown

Signed-off-by: Neil Brown <neilb@xxxxxxx>

### Diffstat output
 ./drivers/md/raid1.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff ./drivers/md/raid1.c~current~ ./drivers/md/raid1.c
--- ./drivers/md/raid1.c~current~	2006-02-27 11:52:18.000000000 +1100
+++ ./drivers/md/raid1.c	2006-03-01 10:30:43.000000000 +1100
@@ -375,7 +375,7 @@ static int raid1_end_write_request(struc
 			/* Don't dec_pending yet, we want to hold
 			 * the reference over the retry
 			 */
-			return 0;
+			goto out;
 		}
 		if (test_bit(R1BIO_BehindIO, &r1_bio->state)) {
 			/* free extra copy of the data pages */
@@ -392,10 +392,11 @@ static int raid1_end_write_request(struc
 		raid_end_bio_io(r1_bio);
 	}
 
+	rdev_dec_pending(conf->mirrors[mirror].rdev, conf->mddev);
+ out:
 	if (r1_bio->bios[mirror]==NULL)
 		bio_put(bio);
 
-	rdev_dec_pending(conf->mirrors[mirror].rdev, conf->mddev);
 	return 0;
 }
 
@@ -857,7 +858,7 @@ static int make_request(request_queue_t 
 	atomic_set(&r1_bio->remaining, 0);
 	atomic_set(&r1_bio->behind_remaining, 0);
 
-	do_barriers = bio->bi_rw & BIO_RW_BARRIER;
+	do_barriers = bio_barrier(bio);
 	if (do_barriers)
 		set_bit(R1BIO_Barrier, &r1_bio->state);
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/