* [PATCH RESEND 5/5] block: validate user space vectors during extraction [not found] <20260720201057.1862857-1-kbusch@meta.com> @ 2026-07-20 20:10 ` Keith Busch 2026-08-05 12:46 ` Shin'ichiro Kawasaki 0 siblings, 1 reply; 5+ messages in thread From: Keith Busch @ 2026-07-20 20:10 UTC (permalink / raw) To: linux-block, linux-fsdevel Cc: dm-devel, hch, axboe, brauner, djwong, viro, Keith Busch, stable, Hannes Reinecke From: Keith Busch <kbusch@kernel.org> The bio-based drivers don't necessarily check the alignment split, and stacking block drivers don't always handle a misalignment detected after submitting the bio. Validate user vectors against the device's dma_alignment as the bio is built from the iov_iter, rejecting misaligned early with -EINVAL. Cc: stable@vger.kernel.org Fixes: 5ff3f74e145a ("block: simplify direct io validity check") Fixes: 7eac33186957 ("iomap: simplify direct io validity check") Reviewed-by: Hannes Reinecke <hare@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Keith Busch <kbusch@kernel.org> --- block/bio.c | 56 +++++++++++++++++++++++++++++++++++++++++--- block/blk-map.c | 2 +- block/fops.c | 2 +- fs/iomap/direct-io.c | 1 + include/linux/bio.h | 2 +- include/linux/uio.h | 10 +++++++- lib/iov_iter.c | 9 ++++++- 7 files changed, 74 insertions(+), 8 deletions(-) diff --git a/block/bio.c b/block/bio.c index f2a5f4d0a9672..faad41a72ac77 100644 --- a/block/bio.c +++ b/block/bio.c @@ -1220,10 +1220,45 @@ static int bio_iov_iter_align_down(struct bio *bio, struct iov_iter *iter, return 0; } +#ifdef CONFIG_DEBUG_KERNEL +static inline bool bio_iov_bvec_aligned(const struct bio *bio, + unsigned mem_align_mask) +{ + struct bvec_iter iter; + struct bio_vec bv; + + /* + * Correct callers never break the alignment requirements, so this + * exhaustive check is only paid for in debug builds. + */ + for_each_mp_bvec(bv, bio->bi_io_vec, iter, bio->bi_iter) + if ((bv.bv_offset | bv.bv_len) & mem_align_mask) + return false; + return true; +} +#else +static inline bool bio_iov_bvec_aligned(const struct bio *bio, + unsigned mem_align_mask) +{ + /* + * We forward the bio_vec as-is, so ITER_BVEC callers must provide + * segments already aligned to the device's DMA alignment. The only + * unchecked user-controllable offset that reaches here is an io_uring + * registered buffer where just the first segment can be unaligned + * (the rest is virtually contiguous), so checking only that one is + * sufficient to know if the entire vector is valid. + */ + return !(mp_bvec_iter_offset(bio->bi_io_vec, bio->bi_iter) & + mem_align_mask); +} +#endif + /** * bio_iov_iter_get_pages - add user or kernel pages to a bio * @bio: bio to add pages to * @iter: iov iterator describing the region to be added + * @mem_align_mask: the mask the source address and length must be aligned to, + * 0 for no requirement * @len_align_mask: the mask to align the total size to, 0 for any length * * This takes either an iterator pointing to user memory, or one pointing to @@ -1242,7 +1277,7 @@ static int bio_iov_iter_align_down(struct bio *bio, struct iov_iter *iter, * is returned only if 0 pages could be pinned. */ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter, - unsigned len_align_mask) + unsigned mem_align_mask, unsigned len_align_mask) { iov_iter_extraction_t flags = 0; @@ -1251,6 +1286,10 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter, if (iov_iter_is_bvec(iter)) { bio_iov_bvec_set(bio, iter); + + if (!bio_iov_bvec_aligned(bio, mem_align_mask)) + return -EINVAL; + iov_iter_advance(iter, bio->bi_iter.bi_size); return 0; } @@ -1265,8 +1304,19 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter, ret = iov_iter_extract_bvecs(iter, bio->bi_io_vec, BIO_MAX_SIZE - bio->bi_iter.bi_size, - &bio->bi_vcnt, bio->bi_max_vecs, flags); + &bio->bi_vcnt, bio->bi_max_vecs, + mem_align_mask, flags); if (ret <= 0) { + /* + * A misaligned vector fails the whole I/O. Release any + * pages pinned by earlier iterations before returning + * since this bio won't be submitted to release them. + */ + if (ret == -EINVAL) { + bio_release_pages(bio, false); + bio_clear_flag(bio, BIO_PAGE_PINNED); + bio->bi_vcnt = 0; + } if (!bio->bi_vcnt) return ret; break; @@ -1377,7 +1427,7 @@ static int bio_iov_iter_bounce_read(struct bio *bio, struct iov_iter *iter, ssize_t ret; ret = iov_iter_extract_bvecs(iter, bio->bi_io_vec + 1, len, - &bio->bi_vcnt, bio->bi_max_vecs - 1, 0); + &bio->bi_vcnt, bio->bi_max_vecs - 1, 0, 0); if (ret <= 0) { if (!bio->bi_vcnt) { folio_put(folio); diff --git a/block/blk-map.c b/block/blk-map.c index 768549f19f97e..c9535efe1a913 100644 --- a/block/blk-map.c +++ b/block/blk-map.c @@ -274,7 +274,7 @@ static int bio_map_user_iov(struct request *rq, struct iov_iter *iter, * No alignment requirements on our part to support arbitrary * passthrough commands. */ - ret = bio_iov_iter_get_pages(bio, iter, 0); + ret = bio_iov_iter_get_pages(bio, iter, 0, 0); if (ret) goto out_put; ret = blk_rq_append_bio(rq, bio); diff --git a/block/fops.c b/block/fops.c index 0098a90a956e1..e519d7f43b310 100644 --- a/block/fops.c +++ b/block/fops.c @@ -46,7 +46,7 @@ static bool blkdev_dio_invalid(struct block_device *bdev, struct kiocb *iocb, static inline int blkdev_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter, struct block_device *bdev) { - return bio_iov_iter_get_pages(bio, iter, + return bio_iov_iter_get_pages(bio, iter, bdev_dma_alignment(bdev), bdev_logical_block_size(bdev) - 1); } diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index b485e3b191daf..ff458aa12ae29 100644 --- a/fs/iomap/direct-io.c +++ b/fs/iomap/direct-io.c @@ -358,6 +358,7 @@ static ssize_t iomap_dio_bio_iter_one(struct iomap_iter *iter, iomap_max_bio_size(&iter->iomap), alignment); else ret = bio_iov_iter_get_pages(bio, dio->submit.iter, + bdev_dma_alignment(bio->bi_bdev), alignment - 1); if (unlikely(ret)) goto out_put_bio; diff --git a/include/linux/bio.h b/include/linux/bio.h index 8f33f717b14f5..ce34ea49ef358 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -477,7 +477,7 @@ int bdev_rw_virt(struct block_device *bdev, sector_t sector, void *data, size_t len, enum req_op op); int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter, - unsigned len_align_mask); + unsigned mem_align_mask, unsigned len_align_mask); void bio_iov_bvec_set(struct bio *bio, const struct iov_iter *iter); void __bio_release_pages(struct bio *bio, bool mark_dirty); diff --git a/include/linux/uio.h b/include/linux/uio.h index a9bc5b3067e32..fe2e985d74d24 100644 --- a/include/linux/uio.h +++ b/include/linux/uio.h @@ -389,9 +389,17 @@ ssize_t iov_iter_extract_pages(struct iov_iter *i, struct page ***pages, size_t maxsize, unsigned int maxpages, iov_iter_extraction_t extraction_flags, size_t *offset0); +/* + * Block-layer consumers (e.g. bio_iov_iter_get_pages()) require that the + * segments of an ITER_BVEC iterator are already aligned to the target device's + * DMA alignment, and forward them as-is. In-kernel users that build their own + * bvecs must not create sub-aligned segments; iov_iter_extract_bvecs() enforces + * the same for the segments it extracts via @mem_align_mask. + */ ssize_t iov_iter_extract_bvecs(struct iov_iter *iter, struct bio_vec *bv, size_t max_size, unsigned short *nr_vecs, - unsigned short max_vecs, iov_iter_extraction_t extraction_flags); + unsigned short max_vecs, unsigned mem_align_mask, + iov_iter_extraction_t extraction_flags); /** * iov_iter_extract_will_pin - Indicate how pages from the iterator will be retained diff --git a/lib/iov_iter.c b/lib/iov_iter.c index 273919b161617..c343075951ded 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -1886,6 +1886,8 @@ static unsigned int get_contig_folio_len(struct page **pages, * @max_size: maximum size to extract from @iter * @nr_vecs: number of vectors in @bv (on in and output) * @max_vecs: maximum vectors in @bv, including those filled before calling + * @mem_align_mask: reject with -EINVAL if the source address or + * length is not aligned to this mask * @extraction_flags: flags to qualify request * * Like iov_iter_extract_pages(), but returns physically contiguous ranges @@ -1897,14 +1899,19 @@ static unsigned int get_contig_folio_len(struct page **pages, */ ssize_t iov_iter_extract_bvecs(struct iov_iter *iter, struct bio_vec *bv, size_t max_size, unsigned short *nr_vecs, - unsigned short max_vecs, iov_iter_extraction_t extraction_flags) + unsigned short max_vecs, unsigned mem_align_mask, + iov_iter_extraction_t extraction_flags) { + unsigned long start = (unsigned long)iter_iov_addr(iter); unsigned short entries_left = max_vecs - *nr_vecs; unsigned short nr_pages, i = 0; size_t left, offset, len; struct page **pages; ssize_t size; + if ((start | iter_iov_len(iter)) & mem_align_mask) + return -EINVAL; + /* * Move page array up in the allocated memory for the bio vecs as far as * possible so that we can start filling biovecs from the beginning -- 2.52.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH RESEND 5/5] block: validate user space vectors during extraction 2026-07-20 20:10 ` [PATCH RESEND 5/5] block: validate user space vectors during extraction Keith Busch @ 2026-08-05 12:46 ` Shin'ichiro Kawasaki 2026-08-06 8:03 ` Shin'ichiro Kawasaki 0 siblings, 1 reply; 5+ messages in thread From: Shin'ichiro Kawasaki @ 2026-08-05 12:46 UTC (permalink / raw) To: Keith Busch Cc: linux-block, linux-fsdevel, dm-devel, hch, axboe, brauner, djwong, viro, Keith Busch, stable, Hannes Reinecke, 0wnerD1ed Cc+: 0wnerD1ed On Jul 20, 2026 / 13:10, Keith Busch wrote: > From: Keith Busch <kbusch@kernel.org> > > The bio-based drivers don't necessarily check the alignment split, and > stacking block drivers don't always handle a misalignment detected after > submitting the bio. Validate user vectors against the device's > dma_alignment as the bio is built from the iov_iter, rejecting > misaligned early with -EINVAL. Recently, the new test case block/045 was added to blkests through a GitHub PR [1]. The test case passes with the Linux master branch tags v7.2-rcX. Today, I found that it fails with block/for-next branch tip [2]. I bisected and found that this patch triggers the failure. 0wnerDied, Keith, may I ask your help to resolve the failure? I'm guessing the test case needs to care device's dma alignment, but not so sure. [1] https://github.com/linux-blktests/blktests/pull/251 [2] failure log of block/045 block/045 (test direct I/O full-trim bvec alignment) [failed] runtime 0.550s ... 0.630s --- tests/block/045.out 2026-08-03 19:26:40.407053612 +0900 +++ /home/shin/Blktests/blktests/results/nodev/block/045.out.bad 2026-08-05 20:48:33.366932845 +0900 @@ -1,2 +1,4 @@ Running block/045 +bio-full-trim: pread returned -1 (errno 22) +bio-full-trim helper failed Test complete block/045 (zoned) (test direct I/O full-trim bvec alignment) [failed] runtime 0.578s ... 0.636s --- tests/block/045.out 2026-08-03 19:26:40.407053612 +0900 +++ /home/shin/Blktests/blktests/results/nodev_zoned/block/045.out.bad 2026-08-05 20:48:34.550929436 +0900 @@ -1,2 +1,4 @@ Running block/045 +bio-full-trim: pread returned -1 (errno 22) +bio-full-trim helper failed Test complete ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RESEND 5/5] block: validate user space vectors during extraction 2026-08-05 12:46 ` Shin'ichiro Kawasaki @ 2026-08-06 8:03 ` Shin'ichiro Kawasaki 2026-08-06 8:29 ` 0wnerD1ed 0 siblings, 1 reply; 5+ messages in thread From: Shin'ichiro Kawasaki @ 2026-08-06 8:03 UTC (permalink / raw) To: Keith Busch Cc: linux-block, linux-fsdevel, dm-devel, hch, axboe, brauner, djwong, viro, Keith Busch, stable, Hannes Reinecke, 0wnerD1ed On Aug 05, 2026 / 21:46, Shin'ichiro Kawasaki wrote: > Cc+: 0wnerD1ed > > On Jul 20, 2026 / 13:10, Keith Busch wrote: > > From: Keith Busch <kbusch@kernel.org> > > > > The bio-based drivers don't necessarily check the alignment split, and > > stacking block drivers don't always handle a misalignment detected after > > submitting the bio. Validate user vectors against the device's > > dma_alignment as the bio is built from the iov_iter, rejecting > > misaligned early with -EINVAL. > > Recently, the new test case block/045 was added to blkests through a GitHub PR > [1]. The test case passes with the Linux master branch tags v7.2-rcX. Today, I > found that it fails with block/for-next branch tip [2]. I bisected and found > that this patch triggers the failure. > > 0wnerDied, Keith, may I ask your help to resolve the failure? I'm guessing > the test case needs to care device's dma alignment, but not so sure. I took a closer look. I modified the test case to respect the dma alignment [*], and now the test case passes. This approach looks working. Will post the change as a formal patch for review. [*] fix trial patch for blktests diff --git a/src/bio-full-trim.c b/src/bio-full-trim.c index e304b2c..7cea3f9 100644 --- a/src/bio-full-trim.c +++ b/src/bio-full-trim.c @@ -12,20 +12,38 @@ #include <sys/mman.h> #include <unistd.h> +int msb(unsigned int v) +{ + unsigned int b = 0; + + while (v >>= 1) + b++; + + return b; +} + int main(int argc, char **argv) { unsigned int block_size; unsigned char *p; long page_size; + unsigned int dma_alignment; + unsigned int dma_aligned_offset; ssize_t ret; int fd; - if (argc != 2) + if (argc != 3) return EXIT_FAILURE; + page_size = sysconf(_SC_PAGESIZE); if (page_size <= 0) errx(EXIT_FAILURE, "invalid page size"); + dma_alignment = atoi(argv[2]); + dma_aligned_offset = 1 << (msb(dma_alignment) + 1); + if (dma_aligned_offset >= page_size) + err(EXIT_FAILURE, "unexpected dma_alignment"); + fd = open(argv[1], O_RDONLY | O_DIRECT); if (fd < 0) err(EXIT_FAILURE, "open %s", argv[1]); @@ -40,7 +58,7 @@ int main(int argc, char **argv) err(EXIT_FAILURE, "mprotect"); errno = 0; - ret = pread(fd, p + page_size - 1, block_size, 0); + ret = pread(fd, p + page_size - dma_aligned_offset, block_size, 0); if (ret == -1 && errno == EFAULT) return EXIT_SUCCESS; errx(EXIT_FAILURE, "pread returned %zd (errno %d)", ret, errno); diff --git a/tests/block/045 b/tests/block/045 index 65bfcb9..6e112b6 100755 --- a/tests/block/045 +++ b/tests/block/045 @@ -25,7 +25,8 @@ test() { return 1 fi - if ! src/bio-full-trim /dev/nullb1; then + if ! src/bio-full-trim /dev/nullb1 \ + $(< /sys/block/nullb1/queue/dma_alignment); then echo "bio-full-trim helper failed" fi -- 2.55.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH RESEND 5/5] block: validate user space vectors during extraction 2026-08-06 8:03 ` Shin'ichiro Kawasaki @ 2026-08-06 8:29 ` 0wnerD1ed 2026-08-06 9:54 ` Shin'ichiro Kawasaki 0 siblings, 1 reply; 5+ messages in thread From: 0wnerD1ed @ 2026-08-06 8:29 UTC (permalink / raw) To: Shin'ichiro Kawasaki Cc: Keith Busch, linux-block, linux-fsdevel, dm-devel, hch, axboe, brauner, djwong, viro, Keith Busch, stable, Hannes Reinecke On Aug 06, 2026 / 16:04, Shin'ichiro Kawasaki wrote: > I took a closer look. I modified the test case to respect the dma alignment [*], > and now the test case passes. This approach looks working. Will post the change > as a formal patch for review. Sorry for the slow reply, I have been busy lately. Thanks for chasing this down. The test is mine and its assumption is wrong, not the kernel, src/bio-full-trim puts the destination at p + page_size - 1, and null_blk advertises dma_alignment = 1 since commit 3451cf34f51b ("null_blk: allow byte aligned memory offsets"), so iov_iter_extract_bvecs() now rejects that odd address before anything is extracted and bio_iov_iter_align_down() is never reached. That restores the behaviour we had until 5ff3f74e145a ("block: simplify direct io validity check"), so the test is what has to respect dma_alignment. Your approach is the right one. One problem in the trial patch, the guard has to be against the logical block size, not the page size. What the test needs is a fragment that bio_iov_iter_align_down() removes completely, i.e. one smaller than the block size, and the read has to reach the PROT_NONE page. With dma_alignment = 511 and a 512 byte block size (the block layer default for drivers that do not set dma_alignment), dma_aligned_offset becomes 512, which is below page_size and passes the check, but the read then lies entirely within the first page, succeeds, and the helper reports pread returned 512 (errno 0) instead of taking the "unexpected dma_alignment" exit. block/045 only uses null_blk so this cannot bite today, but the check does not do what it intends. block_size comes from BLKSSZGET, so it also has to move below that ioctl: if (dma_aligned_offset >= block_size) errx(EXIT_FAILURE, "unexpected dma_alignment %u for block size %u", dma_alignment, block_size); While there, err() should be errx(), errno is not meaningful at that point. I am short on time at the moment, so please go ahead with the formal posting. I will send a Tested-by once I get a chance to run it. Regards, 0wnerD1ed l7z@0b1t.tech ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RESEND 5/5] block: validate user space vectors during extraction 2026-08-06 8:29 ` 0wnerD1ed @ 2026-08-06 9:54 ` Shin'ichiro Kawasaki 0 siblings, 0 replies; 5+ messages in thread From: Shin'ichiro Kawasaki @ 2026-08-06 9:54 UTC (permalink / raw) To: 0wnerD1ed Cc: Keith Busch, linux-block, linux-fsdevel, dm-devel, hch, axboe, brauner, djwong, viro, Keith Busch, stable, Hannes Reinecke On Aug 06, 2026 / 16:29, 0wnerD1ed wrote: > On Aug 06, 2026 / 16:04, Shin'ichiro Kawasaki wrote: > > I took a closer look. I modified the test case to respect the dma alignment [*], > > and now the test case passes. This approach looks working. Will post the change > > as a formal patch for review. > > Sorry for the slow reply, I have been busy lately. > > Thanks for chasing this down. The test is mine and its assumption is > wrong, not the kernel, src/bio-full-trim puts the destination at > p + page_size - 1, and null_blk advertises dma_alignment = 1 since > commit 3451cf34f51b ("null_blk: allow byte aligned memory offsets"), > so iov_iter_extract_bvecs() now rejects that odd address before > anything is extracted and bio_iov_iter_align_down() is never reached. > That restores the behaviour we had until 5ff3f74e145a ("block: > simplify direct io validity check"), so the test is what has to > respect dma_alignment. Your approach is the right one. Good, thanks for the confirmation. > > One problem in the trial patch, the guard has to be against the > logical block size, not the page size. What the test needs is a > fragment that bio_iov_iter_align_down() removes completely, i.e. one > smaller than the block size, and the read has to reach the PROT_NONE > page. With dma_alignment = 511 and a 512 byte block size (the block > layer default for drivers that do not set dma_alignment), > dma_aligned_offset becomes 512, which is below page_size and passes > the check, but the read then lies entirely within the first page, > succeeds, and the helper reports > > pread returned 512 (errno 0) > > instead of taking the "unexpected dma_alignment" exit. block/045 only > uses null_blk so this cannot bite today, but the check does not do > what it intends. block_size comes from BLKSSZGET, so it also has to > move below that ioctl: > > if (dma_aligned_offset >= block_size) > errx(EXIT_FAILURE, > "unexpected dma_alignment %u for block size %u", > dma_alignment, block_size); > > While there, err() should be errx(), errno is not meaningful at that > point. Yep, I reflected this change to the patch. > > I am short on time at the moment, so please go ahead with the formal > posting. I will send a Tested-by once I get a chance to run it. Sure, I have posted the patch. Thanks. https://lore.kernel.org/linux-block/20260806094218.2077413-1-shinichiro.kawasaki@wdc.com/ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-06 9:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260720201057.1862857-1-kbusch@meta.com>
2026-07-20 20:10 ` [PATCH RESEND 5/5] block: validate user space vectors during extraction Keith Busch
2026-08-05 12:46 ` Shin'ichiro Kawasaki
2026-08-06 8:03 ` Shin'ichiro Kawasaki
2026-08-06 8:29 ` 0wnerD1ed
2026-08-06 9:54 ` Shin'ichiro Kawasaki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox