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]
Date:	Mon, 1 Dec 2014 11:23:24 -0500 (EST)
From:	Mikulas Patocka <mpatocka@...hat.com>
To:	device-mapper development <dm-devel@...hat.com>
cc:	Alasdair Kergon <agk@...hat.com>,
	Mike Snitzer <snitzer@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [dm-devel] [PATCH] dm-bufio: fix memleak when using a dm_buffer's
 inline bio



On Tue, 25 Nov 2014, Darrick J. Wong wrote:

> When dm-bufio sets out to use the bio built into a struct dm_buffer to
> issue an IO, it needs to call bio_reset after it's done with the bio
> so that we can free things attached to the bio such as the integrity
> payload.  Therefore, inject our own endio callback to take care of
> the bio_reset after calling submit_io's end_io callback.
> 
> Test case:
> 1. modprobe scsi_debug delay=0 dif=1 dix=199 ato=1 dev_size_mb=300
> 2. Set up a dm-bufio client, e.g. dm-verity, on the scsi_debug device
> 3. Repeatedly read metadata and watch kmalloc-192 leak!
> 
> Fix is against 3.18-rc6.
> 
> +/* Reset the bio to free attached bio integrity profiles when we're done */
> +static void inline_endio(struct bio *bio, int error)
> +{
> +	bio_end_io_t *end_fn;
> +
> +	end_fn = bio->bi_private;
> +	end_fn(bio, error);
> +	bio_reset(bio);
> +}

This is wrong - when end_fn clears the B_READING or B_WRITING flag, the 
buffer may be freed by the background cleanup - so bio_reset may be 
modifying freed memory here. We need to call bio_reset before end_fn.



From: Mikulas Patocka <mpatocka@...hat.com>

When dm-bufio sets out to use the bio built into a struct dm_buffer to
issue an IO, it needs to call bio_reset after it's done with the bio
so that we can free things attached to the bio such as the integrity
payload.  Therefore, inject our own endio callback to take care of
the bio_reset after calling submit_io's end_io callback.

Test case:
1. modprobe scsi_debug delay=0 dif=1 dix=199 ato=1 dev_size_mb=300
2. Set up a dm-bufio client, e.g. dm-verity, on the scsi_debug device
3. Repeatedly read metadata and watch kmalloc-192 leak!

Signed-off-by: Darrick J. Wong <darrick.wong@...cle.com>
Signed-off-by: Mike Snitzer <snitzer@...hat.com>
Signed-off-by: Mikulas Patocka <mpatocka@...hat.com>
Cc: stable@...r.kernel.org

---
 drivers/md/dm-bufio.c |   19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Index: linux-3.18-rc6/drivers/md/dm-bufio.c
===================================================================
--- linux-3.18-rc6.orig/drivers/md/dm-bufio.c	2014-12-01 14:52:35.000000000 +0100
+++ linux-3.18-rc6/drivers/md/dm-bufio.c	2014-12-01 14:52:37.000000000 +0100
@@ -565,6 +565,18 @@ static void use_dmio(struct dm_buffer *b
 		end_io(&b->bio, r);
 }
 
+static void inline_endio(struct bio *bio, int error)
+{
+	bio_end_io_t *end_fn = bio->bi_private;
+	/*
+	 * Reset the bio to free any attached resources
+	 * (e.g. bio integrity profiles).
+	 */
+	bio_reset(bio);
+
+	end_fn(bio, error);
+}
+
 static void use_inline_bio(struct dm_buffer *b, int rw, sector_t block,
 			   bio_end_io_t *end_io)
 {
@@ -576,7 +588,12 @@ static void use_inline_bio(struct dm_buf
 	b->bio.bi_max_vecs = DM_BUFIO_INLINE_VECS;
 	b->bio.bi_iter.bi_sector = block << b->c->sectors_per_block_bits;
 	b->bio.bi_bdev = b->c->bdev;
-	b->bio.bi_end_io = end_io;
+	b->bio.bi_end_io = inline_endio;
+	/*
+	 * Use of .bi_private isn't a problem here because
+	 * the dm_buffer's inline bio is local to bufio.
+	 */
+	b->bio.bi_private = end_io;
 
 	/*
 	 * We assume that if len >= PAGE_SIZE ptr is page-aligned.
--
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/

Powered by blists - more mailing lists