The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH RFC] blk-integrity: fix slab-out-of-bounds in t10_pi_verify on namespace revalidation
@ 2026-05-31 22:45 Samin Y. Chowdhury via B4 Relay
  2026-06-01  7:40 ` Christoph Hellwig
  0 siblings, 1 reply; 3+ messages in thread
From: Samin Y. Chowdhury via B4 Relay @ 2026-05-31 22:45 UTC (permalink / raw)
  To: Jens Axboe, Martin K. Petersen, Keith Busch
  Cc: linux-block, linux-kernel, Samin Y. Chowdhury, Sungwoo Kim,
	Dave Tian, Weidong Zhu, Ruimin Sun

From: "Samin Y. Chowdhury" <samin_c@outlook.com>

Abort early with BLK_STS_PROTECTION if a namespace revalidation changed
bi->metadata_size after bio_integrity_prep() sized the allocation.

Found by FuzzNvme (Syzkaller with FEMU fuzzing framework).

When a namespace is revalidated between bio_integrity_prep() and
bio_integrity_verify_fn(), the integrity profile's metadata_size may
change under the in-flight bio. bio_integrity_verify_fn() re-reads the
live blk_integrity via blk_get_integrity(), so blk_integrity_iterate()
uses the new metadata_size as the per-interval step size against a
buffer sized for the old one, advancing iter->prot_buf past the end of
the allocation.

task 1:
  bio_integrity_prep()
    bio_integrity_alloc_buf()
      len = bio_integrity_bytes(bi, bio_sectors(bio))   ...(1)
      bip->bip_iter.bi_size = len

task 2:
  nvme_update_ns_info_block()
    blk_mq_freeze_queue()
    nvme_init_integrity()
      bi->metadata_size = head->ms                      ...(2)
    blk_mq_unfreeze_queue()

task 3:
  bio_integrity_verify_fn()
    bio_integrity_verify()
      blk_integrity_iterate()
        bi = blk_get_integrity()                        ...(3)
        iter->interval_remaining = 1 << bi->interval_exp
        iter->prot_buf += bi->metadata_size per interval
        /* step size from (3), buffer sized at (1): overrun */

Fixes: 8098514bd5ca ("block: always allocate integrity buffer when required")
Signed-off-by: Samin Y. Chowdhury <samin_c@outlook.com>
Acked-by: Sungwoo Kim <iam@sung-woo.kim>
Acked-by: Dave Tian <daveti@purdue.edu>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Acked-by: Ruimin Sun <rsun@fiu.edu>
---
When a namespace is revalidated between bio_integrity_prep() and
bio_integrity_verify_fn(), the integrity profile's metadata_size may
change under the in-flight bio. bio_integrity_verify_fn() re-reads the
live blk_integrity via blk_get_integrity(), so blk_integrity_iterate()
uses the new metadata_size as the per-interval step size against a
buffer sized for the old one, advancing iter->prot_buf past the end of
the allocation.

task 1:
  bio_integrity_prep()
    bio_integrity_alloc_buf()
      len = bio_integrity_bytes(bi, bio_sectors(bio))   ...(1)
      bip->bip_iter.bi_size = len

task 2:
  nvme_update_ns_info_block()
    blk_mq_freeze_queue()
    nvme_init_integrity()
      bi->metadata_size = head->ms                      ...(2)
    blk_mq_unfreeze_queue()

task 3:
  bio_integrity_verify_fn()
    bio_integrity_verify()
      blk_integrity_iterate()
        bi = blk_get_integrity()                        ...(3)
        iter->interval_remaining = 1 << bi->interval_exp
        iter->prot_buf += bi->metadata_size per interval
        /* step size from (3), buffer sized at (1): overrun */
---
 block/bio-integrity-auto.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/block/bio-integrity-auto.c b/block/bio-integrity-auto.c
index 353eed632fc..b404dbaa9f8 100644
--- a/block/bio-integrity-auto.c
+++ b/block/bio-integrity-auto.c
@@ -38,6 +38,18 @@ static void bio_integrity_verify_fn(struct work_struct *work)
 	struct bio_integrity_data *bid =
 		container_of(work, struct bio_integrity_data, work);
 	struct bio *bio = bid->bio;
+	struct blk_integrity *bi = blk_get_integrity(bio->bi_bdev->bd_disk);
+
+	if (bi) {
+		unsigned int required = bio_integrity_bytes(bi, bio_sectors(bio));
+
+		if (unlikely(required > bid->bip.bip_iter.bi_size)) {
+			bio->bi_status = BLK_STS_PROTECTION;
+			bio_integrity_finish(bid);
+			bio_endio(bio);
+			return;
+		}
+	}
 
 	bio->bi_status = bio_integrity_verify(bio, &bid->saved_bio_iter);
 	bio_integrity_finish(bid);

---
base-commit: 174914ea551314c52a61713b9c4bde9e42d48073
change-id: 20260531-blk-integrity-fix-e8109008af54

Best regards,
--  
Samin Y. Chowdhury <samin_c@outlook.com>



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH RFC] blk-integrity: fix slab-out-of-bounds in t10_pi_verify on namespace revalidation
  2026-05-31 22:45 [PATCH RFC] blk-integrity: fix slab-out-of-bounds in t10_pi_verify on namespace revalidation Samin Y. Chowdhury via B4 Relay
@ 2026-06-01  7:40 ` Christoph Hellwig
  2026-08-11  4:04   ` Chao Shi
  0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2026-06-01  7:40 UTC (permalink / raw)
  To: samin_c
  Cc: Jens Axboe, Martin K. Petersen, Keith Busch, linux-block,
	linux-kernel, Sungwoo Kim, Dave Tian, Weidong Zhu, Ruimin Sun

On Sun, May 31, 2026 at 06:45:07PM -0400, Samin Y. Chowdhury via B4 Relay wrote:
> When a namespace is revalidated between bio_integrity_prep() and
> bio_integrity_verify_fn(), the integrity profile's metadata_size may
> change under the in-flight bio. bio_integrity_verify_fn() re-reads the
> live blk_integrity via blk_get_integrity(), so blk_integrity_iterate()
> uses the new metadata_size as the per-interval step size against a
> buffer sized for the old one, advancing iter->prot_buf past the end of
> the allocation.

I don't think changing fundamental device properies such as the LBA
or integrity tag size under a live device is a good model.  So instead
of coming up with bandaids like this, we should probably just fail
any such revalidation when there are openers instead of trying to deal
with the fallout.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH RFC] blk-integrity: fix slab-out-of-bounds in t10_pi_verify on namespace revalidation
  2026-06-01  7:40 ` Christoph Hellwig
@ 2026-08-11  4:04   ` Chao Shi
  0 siblings, 0 replies; 3+ messages in thread
From: Chao Shi @ 2026-08-11  4:04 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Martin K. Petersen, Keith Busch, linux-block,
	linux-kernel, Samin Y. Chowdhury, Sungwoo Kim, Dave Tian,
	Weidong Zhu, Ruimin Sun

On Mon, Jun 01, 2026 at 09:40:29AM +0200, Christoph Hellwig wrote:
> I don't think changing fundamental device properies such as the LBA
> or integrity tag size under a live device is a good model.  So instead
> of coming up with bandaids like this, we should probably just fail
> any such revalidation when there are openers instead of trying to deal
> with the fallout.

Agreed, and sorry for the long silence.  Samin and I are in the same
group and I am picking this up from him.  We are dropping the
blk-integrity patch rather than respinning it.

Before writing anything on the nvme side I would like to get the shape
right, because "fail the revalidation" can mean a few things and one of
them looks worse than the splat.

If a namespace really was reformatted out of band and we refuse to adopt
the new geometry, the host keeps issuing I/O with the old LBA format and
metadata size to a device that has already changed underneath it.  That
is silent corruption rather than a KASAN report.  So I assume refusing
the update has to be paired with taking the namespace out of service.
Did you have in mind

  (a) refuse the update and mark the namespace dead, like a surprise
      removal,
  (b) refuse only while the disk has openers and redo the revalidation
      on last close, or
  (c) something else?

Chao

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-11  4:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-31 22:45 [PATCH RFC] blk-integrity: fix slab-out-of-bounds in t10_pi_verify on namespace revalidation Samin Y. Chowdhury via B4 Relay
2026-06-01  7:40 ` Christoph Hellwig
2026-08-11  4:04   ` Chao Shi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox