All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@kernel.org>
To: linux-nfs@vger.kernel.org, linux-block@vger.kernel.org
Cc: dm-devel@lists.linux.dev, axboe@kernel.dk, cel@kernel.org,
	jlayton@kernel.org, david.flynn@hammerspace.com
Subject: [PATCH 1/4] brd: iterate the bio by byte position, not bi_sector
Date: Tue,  8 Sep 2026 12:34:36 -0400	[thread overview]
Message-ID: <20260908163448.30841-2-snitzer@kernel.org> (raw)
In-Reply-To: <20260908163448.30841-1-snitzer@kernel.org>

brd_rw_bvec() takes the device position from bio->bi_iter.bi_sector,
which bio_advance_iter_single() advances by bytes >> SECTOR_SHIFT. For
a bvec whose length is not a multiple of the sector size the sector
cursor silently loses the sub-sector residue while the data cursor
(bi_bvec_done/bi_size) consumes the full length -- from that segment
on, data is written at a device offset short of where it belongs, and
every subsequent byte lands shifted with no error reported anywhere.

Such bvec geometry is legal at the submitter: ITER_BVEC direct I/O
passes the caller's bio_vec array through as-is (bio_iov_bvec_set()),
so e.g. NFSD's NFSD_IO_DIRECT write path hands XFS/iomap a payload
whose first fragment starts mid-page (the RPC header precedes it in
the receive buffer) and whose fragment lengths are not sector
multiples. A 1 MiB write arriving as bv0=(160,16224) + 63x(0,16384) +
(0,160) reproduces on brd as: first 15872 = ALIGN_DOWN(16224, 512)
bytes correct, everything after shifted forward by 352 = 16224 - 15872
bytes -- while the write completes successfully. Any NFSD_IO_DIRECT
(or other kernel bvec direct I/O) write to a brd-backed filesystem is
exposed; request-based drivers are unaffected because nothing in the
request path does per-bvec sector arithmetic.

Track the device position as a byte offset owned by the submit loop
and advanced by the number of bytes each segment actually processed,
instead of re-deriving it from the skewed bi_sector. Verified with a
synthetic-bio reproducer over brd directly and through nvme-loop:
mid-page-offset geometries and the page-aligned control now all read
back byte-identical, and 20 fresh NFS connections x 16 MiB of O_DIRECT
writes over an XFS-on-nvme-loop-on-brd export complete with zero data
mismatches (previously most connections corrupted).

Fixes: 3185444f0504 ("brd: split I/O at page boundaries")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 drivers/block/brd.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 00cc8122068f..4011538cecaf 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -134,12 +134,24 @@ static void brd_free_pages(struct brd_device *brd)
 /*
  * Process a single segment.  The segment is capped to not cross page boundaries
  * in both the bio and the brd backing memory.
+ *
+ * The device position is @pos, a byte offset maintained by the caller --
+ * not bio->bi_iter.bi_sector: bio_advance_iter_single() advances bi_sector
+ * by bytes >> SECTOR_SHIFT, so a bvec whose length is not a multiple of the
+ * sector size silently skews bi_sector against the bytes actually consumed
+ * and corrupts everything that follows.  Byte-granular bvec boundaries
+ * reach us from ITER_BVEC direct I/O submitters whose caller's bio_vec
+ * array is passed through as-is (bio_iov_bvec_set()).
+ *
+ * Returns the number of bytes processed, or 0 on error (the bio has then
+ * been completed).
  */
-static bool brd_rw_bvec(struct brd_device *brd, struct bio *bio)
+static unsigned int brd_rw_bvec(struct brd_device *brd, struct bio *bio,
+				loff_t pos)
 {
 	struct bio_vec bv = bio_iter_iovec(bio, bio->bi_iter);
-	sector_t sector = bio->bi_iter.bi_sector;
-	u32 offset = (sector & (PAGE_SECTORS - 1)) << SECTOR_SHIFT;
+	sector_t sector = pos >> SECTOR_SHIFT;
+	u32 offset = pos & (PAGE_SIZE - 1);
 	blk_opf_t opf = bio->bi_opf;
 	struct page *page;
 	void *kaddr;
@@ -167,14 +179,14 @@ static bool brd_rw_bvec(struct brd_device *brd, struct bio *bio)
 	bio_advance_iter_single(bio, &bio->bi_iter, bv.bv_len);
 	if (page)
 		put_page(page);
-	return true;
+	return bv.bv_len;
 
 out_error:
 	if (PTR_ERR(page) == -ENOMEM && (opf & REQ_NOWAIT))
 		bio_wouldblock_error(bio);
 	else
 		bio_io_error(bio);
-	return false;
+	return 0;
 }
 
 static void brd_do_discard(struct brd_device *brd, sector_t sector, u32 size)
@@ -202,6 +214,7 @@ static void brd_do_discard(struct brd_device *brd, sector_t sector, u32 size)
 static void brd_submit_bio(struct bio *bio)
 {
 	struct brd_device *brd = bio->bi_bdev->bd_disk->private_data;
+	loff_t pos;
 
 	if (unlikely(op_is_discard(bio->bi_opf))) {
 		brd_do_discard(brd, bio->bi_iter.bi_sector,
@@ -210,9 +223,13 @@ static void brd_submit_bio(struct bio *bio)
 		return;
 	}
 
+	pos = (loff_t)bio->bi_iter.bi_sector << SECTOR_SHIFT;
 	do {
-		if (!brd_rw_bvec(brd, bio))
+		unsigned int len = brd_rw_bvec(brd, bio, pos);
+
+		if (!len)
 			return;
+		pos += len;
 	} while (bio->bi_iter.bi_size);
 
 	bio_endio(bio);
-- 
2.52.0


  reply	other threads:[~2026-09-08 16:34 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-08 16:32 [PATCH 0/4] block, nfsd: fixes for sub-sector bvec direct I/O Mike Snitzer
2026-09-08 16:32 ` [PATCH 1/4] brd: iterate the bio by byte position, not bi_sector Mike Snitzer
2026-09-08 16:34 ` [PATCH 0/4] block, nfsd: fixes for sub-sector bvec direct I/O Mike Snitzer
2026-09-08 16:34   ` Mike Snitzer [this message]
2026-09-08 16:34   ` [PATCH 2/4] zram: handle sub-page bvec segments without corrupting data Mike Snitzer
2026-09-08 16:34   ` [PATCH 3/4] nfsd: fetch direct I/O alignment for files handed to the filecache Mike Snitzer
2026-09-09 14:11     ` Chuck Lever
2026-09-08 16:34   ` [PATCH 4/4] nfsd: fall back to buffered I/O when a direct write gets -EINVAL Mike Snitzer
2026-09-08 18:25     ` Chuck Lever
     [not found]       ` <B3A1EA3A-00AA-4A56-A644-9AC77FF50CAF@hammerspace.com>
2026-09-09 13:44         ` Chuck Lever
2026-09-09 16:40       ` Mike Snitzer
2026-09-10  9:53       ` Christoph Hellwig
2026-09-08 16:34   ` [PATCH 0/4] block, nfsd: fixes for sub-sector bvec direct I/O Mike Snitzer
2026-09-08 16:34   ` [PATCH 1/4] brd: iterate the bio by byte position, not bi_sector Mike Snitzer
2026-09-08 16:34   ` [PATCH 2/4] zram: handle sub-page bvec segments without corrupting data Mike Snitzer
2026-09-08 16:34   ` [PATCH 3/4] nfsd: fetch direct I/O alignment for files handed to the filecache Mike Snitzer
2026-09-08 16:34   ` [PATCH 4/4] nfsd: fall back to buffered I/O when a direct write gets -EINVAL Mike Snitzer
2026-09-08 16:34   ` [PATCH 2/4] zram: handle sub-page bvec segments without corrupting data Mike Snitzer
2026-09-08 16:34   ` [PATCH 3/4] nfsd: fetch direct I/O alignment for files handed to the filecache Mike Snitzer
2026-09-08 16:34   ` [PATCH 4/4] nfsd: fall back to buffered I/O when a direct write gets -EINVAL Mike Snitzer
2026-09-08 16:36   ` [PATCH 0/4] block, nfsd: fixes for sub-sector bvec direct I/O Mike Snitzer
2026-09-08 17:48     ` Chuck Lever
2026-09-08 18:06       ` Mike Snitzer
2026-09-10  7:14     ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260908163448.30841-2-snitzer@kernel.org \
    --to=snitzer@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=cel@kernel.org \
    --cc=david.flynn@hammerspace.com \
    --cc=dm-devel@lists.linux.dev \
    --cc=jlayton@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.