[PATCH] block: initialize integrity buffer to zero before writing it to media

[PATCH] block: initialize integrity buffer to zero before writing it to media

[Christoph Hellwig]

Metadata added by bio_integrity_prep is using plain kmalloc, which leads
to random kernel memory being written media.  For PI metadata this is
limited to the app tag that isn't used by kernel generated metadata,
but for non-PI metadata the entire buffer leaks kernel memory.

Fix this by adding the __GFP_ZERO flag to allocations for writes.

Fixes: 7ba1ba12eeef ("block: Block layer data integrity support")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bio-integrity.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 2e3e8e04961eae..af7f71d16114de 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -432,6 +432,7 @@ bool bio_integrity_prep(struct bio *bio)
 	unsigned long start, end;
 	unsigned int len, nr_pages;
 	unsigned int bytes, offset, i;
+	gfp_t gfp = GFP_NOIO;
 
 	if (!bi)
 		return true;
@@ -454,11 +455,19 @@ bool bio_integrity_prep(struct bio *bio)
 		if (!bi->profile->generate_fn ||
 		    !(bi->flags & BLK_INTEGRITY_GENERATE))
 			return true;
+
+		/*
+		 * Zero the memory allocated to not leak uninitialized kernel
+		 * memory to disk.  For PI this only affects the app tag, but
+		 * for non-integrity metadata it affects the entire metadata
+		 * buffer.
+		 */
+		gfp |= __GFP_ZERO;
 	}
 
 	/* Allocate kernel buffer for protection data */
 	len = bio_integrity_bytes(bi, bio_sectors(bio));
-	buf = kmalloc(len, GFP_NOIO);
+	buf = kmalloc(len, gfp);
 	if (unlikely(buf == NULL)) {
 		printk(KERN_ERR "could not allocate integrity buffer\n");
 		goto err_end_io;
-- 
2.43.0

Re: [PATCH] block: initialize integrity buffer to zero before writing it to media

[Martin K. Petersen]

Christoph,

> Metadata added by bio_integrity_prep is using plain kmalloc, which
> leads to random kernel memory being written media. For PI metadata
> this is limited to the app tag that isn't used by kernel generated
> metadata, but for non-PI metadata the entire buffer leaks kernel
> memory.

We do explicitly set the app_tag to 0 for PI so it's only non-PI
metadata that's affected.

> Fix this by adding the __GFP_ZERO flag to allocations for writes.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] block: initialize integrity buffer to zero before writing it to media

[Christoph Hellwig]

On Thu, Jun 06, 2024 at 10:08:20AM -0400, Martin K. Petersen wrote:
> 
> Christoph,
> 
> > Metadata added by bio_integrity_prep is using plain kmalloc, which
> > leads to random kernel memory being written media. For PI metadata
> > this is limited to the app tag that isn't used by kernel generated
> > metadata, but for non-PI metadata the entire buffer leaks kernel
> > memory.
> 
> We do explicitly set the app_tag to 0 for PI so it's only non-PI
> metadata that's affected.

Ah, true.  I could switch to then just zeroing the buffer in
->generate_fn for non-PI metadata only.  That's actually the
first version I prototyped.

Re: [PATCH] block: initialize integrity buffer to zero before writing it to media

[Christoph Hellwig]

On Thu, Jun 06, 2024 at 04:10:17PM +0200, Christoph Hellwig wrote:
> > We do explicitly set the app_tag to 0 for PI so it's only non-PI
> > metadata that's affected.
> 
> Ah, true.  I could switch to then just zeroing the buffer in
> ->generate_fn for non-PI metadata only.  That's actually the
> first version I prototyped.

So that would cause a fair amout of conflicts with moving the integrity
information to the limits.  So unless someone objets I'd like to go
with this simple version, and with that series we can then easily
relax the zeroing check to only cover the non-PI case.

Re: [PATCH] block: initialize integrity buffer to zero before writing it to media

[Kanchan Joshi]

On 6/7/2024 11:10 AM, Christoph Hellwig wrote:
>   So unless someone objets I'd like to go
> with this simple version, and with that series we can then easily
> relax the zeroing check to only cover the non-PI case.

Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>

Re: [PATCH] block: initialize integrity buffer to zero before writing it to media

[Chaitanya Kulkarni]

On 6/5/2024 10:27 PM, Christoph Hellwig wrote:
> Metadata added by bio_integrity_prep is using plain kmalloc, which leads
> to random kernel memory being written media.  For PI metadata this is
> limited to the app tag that isn't used by kernel generated metadata,
> but for non-PI metadata the entire buffer leaks kernel memory.
> 
> Fix this by adding the __GFP_ZERO flag to allocations for writes.
> 
> Fixes: 7ba1ba12eeef ("block: Block layer data integrity support")
> Signed-off-by: Christoph Hellwig<hch@lst.de>

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck