[PATCH] block: Fix start and length check added to iov_iter_extract_bvecs()

[PATCH] block: Fix start and length check added to iov_iter_extract_bvecs()

[David Howells]

Commit 14b007e17881 added an address check using iter_iov_addr() and a
length check using iter_iov_len() to iov_iter_extract_bvecs(), but these
cannot be used so and are unsafe in this circumstance as the functions have
hardwired assumptions about the iterator type.  They should only be used
with ITER_UBUF or ITER_IOVEC-type iterators; they should not be used with
ITER_BVEC, ITER_KVEC, ITER_FOLIOQ, ITER_XARRAY or ITER_DISCARD iterators.

This change proved to be a problem for cachefiles as an iterator of type
ITER_FOLIOQ is passed and iter_iov_addr() and iter_iov_len() both
malfunction because iter->__iov in iter_iov() is not pointing to an iovec
array.

Fix this by using iov_iter_alignment() instead.

Fixes: 14b007e17881 ("block: validate user space vectors during extraction")
cc: Keith Busch <kbusch@kernel.org>
cc: Jens Axboe <axboe@kernel.dk>
cc: Hannes Reinecke <hare@kernel.org>
cc: Christoph Hellwig <hch@lst.de>
cc: Alexander Viro <viro@zeniv.linux.org.uk>
cc: Paulo Alcantara <pc@manguebit.org>
cc: netfs@lists.linux.dev
cc: linux-block@vger.kernel.org
cc: linux-fsdevel@vger.kernel.org
---

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 6665372ecf71..6df716e19247 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1921,15 +1921,19 @@ ssize_t iov_iter_extract_bvecs(struct iov_iter *iter, struct bio_vec *bv,
 		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)
+	if (iov_iter_alignment(iter) & mem_align_mask) {
+		pr_warn("%u %zx %x\n",
+			iter->iter_type,
+			iov_iter_alignment(iter),
+			mem_align_mask);
 		return -EINVAL;
+	}
 
 	/*
 	 * Move page array up in the allocated memory for the bio vecs as far as

Re: [PATCH] block: Fix start and length check added to iov_iter_extract_bvecs()

[Keith Busch]

On Wed, Aug 26, 2026 at 07:18:37PM +0100, David Howells wrote:
> @@ -1921,15 +1921,19 @@ ssize_t iov_iter_extract_bvecs(struct iov_iter *iter, struct bio_vec *bv,
>  		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)
> +	if (iov_iter_alignment(iter) & mem_align_mask) {
> +		pr_warn("%u %zx %x\n",
> +			iter->iter_type,
> +			iov_iter_alignment(iter),
> +			mem_align_mask);
>  		return -EINVAL;
> +	}

iov_iter_alignment loops over all the vectors when we only need to
examine the current one here.

Re: [PATCH] block: Fix start and length check added to iov_iter_extract_bvecs()

[David Howells]

Keith Busch <kbusch@kernel.org> wrote:

> iov_iter_alignment loops over all the vectors when we only need to
> examine the current one here.

Can the check be done earlier, then?

David

Re: [PATCH] block: Fix start and length check added to iov_iter_extract_bvecs()

[Keith Busch]

On Wed, Aug 26, 2026 at 08:24:05PM +0100, David Howells wrote:
> Keith Busch <kbusch@kernel.org> wrote:
> 
> > iov_iter_alignment loops over all the vectors when we only need to
> > examine the current one here.
> 
> Can the check be done earlier, then?

It used to be earlier, but the point was to reduce repeated iter
looping. It adds up, so I trying to co-locate validity checks with
places that have to iterate.

Would it be okay to special case the ubuf, iovec, and kvec types for the
simple check?

---
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 81e5c5e5121f7..b6b1e75352b0b 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1920,15 +1920,22 @@ ssize_t iov_iter_extract_bvecs(struct iov_iter *iter, struct bio_vec *bv,
 		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)
+	if (likely(iter_is_ubuf(iter) ||
+		   iter_is_iovec(iter) ||
+		   iov_iter_is_kvec(iter))) {
+		unsigned long start = (unsigned long)iter_iov_addr(iter);
+
+		if ((start | iter_iov_len(iter)) & mem_align_mask)
+			return -EINVAL;
+	} else if (iov_iter_alignment(iter) & mem_align_mask) {
 		return -EINVAL;
+	}
 
 	/*
 	 * Move page array up in the allocated memory for the bio vecs as far as
--

Re: [PATCH] block: Fix start and length check added to iov_iter_extract_bvecs()

[David Howells]

Keith Busch <kbusch@kernel.org> wrote:

> iov_iter_alignment loops over all the vectors when we only need to
> examine the current one here.

Actually, I don't think that's true.  iov_iter_extract_pages() is allowed to
pull from multiple bio_vecs in an ITER_BVEC, for example - and if, say, the
page in the second bio_vec is contiguous with the first, then
iov_iter_extract_bvecs() will use it - so you still need to check bv_len on
it.

David

Re: [PATCH] block: Fix start and length check added to iov_iter_extract_bvecs()

[Keith Busch]

On Wed, Aug 26, 2026 at 08:49:00PM +0100, David Howells wrote:
> Keith Busch <kbusch@kernel.org> wrote:
> 
> > iov_iter_alignment loops over all the vectors when we only need to
> > examine the current one here.
> 
> Actually, I don't think that's true.  iov_iter_extract_pages() is allowed to
> pull from multiple bio_vecs in an ITER_BVEC, for example - and if, say, the
> page in the second bio_vec is contiguous with the first, then
> iov_iter_extract_bvecs() will use it - so you still need to check bv_len on
> it.

I don't think we should be extracting bvecs for the ITER_BVEC type.
bio_iov_iter_get_pages() already doesn't. I'll look more into the
recently introduced bio_iov_iter_bounce_read() usage, as there may be an
optimization there.

But in general, yeah, it should be safe for any type. The proposal I
sent a bit ago will handle the ITER_BVEC as you've desribed.

Re: [PATCH] block: Fix start and length check added to iov_iter_extract_bvecs()

[David Howells]

Keith Busch <kbusch@kernel.org> wrote:

> +	if (likely(iter_is_ubuf(iter) ||
> +		   iter_is_iovec(iter) ||
> +		   iov_iter_is_kvec(iter))) {

I wonder if it's worth making an exception for ITER_KVEC.  I know the structs
align, as it were, but do we use ITER_KVEC often enough?

Anyway, it seems to work, so you can add:

	Reviewed-by: David Howells <dhowells@redhat.com>

if you want to submit the patch yourself.

David

[PATCH v2] block: Fix start and length check added to iov_iter_extract_bvecs()

[David Howells]

Commit 14b007e17881 added an address check using iter_iov_addr() and a
length check using iter_iov_len() to iov_iter_extract_bvecs(), but these
cannot be used so and are unsafe in this circumstance as the functions have
hardwired assumptions about the iterator type.  They should only be used
with ITER_UBUF or ITER_IOVEC-type iterators and work with ITER_KVEC; they
should not be used with ITER_BVEC, ITER_FOLIOQ, ITER_XARRAY or ITER_DISCARD
iterators.

This change proved to be a problem for cachefiles as an iterator of type
ITER_FOLIOQ is passed and iter_iov_addr() and iter_iov_len() both
malfunction because iter->__iov in iter_iov() is not pointing to an iovec
array.

Fix this by using iov_iter_alignment() instead for anything other than
ITER_UBUF, ITER_IOVEC or ITER_KVEC.

Fixes: 14b007e17881 ("block: validate user space vectors during extraction")
Suggested-by: Keith Busch <kbusch@kernel.org>
cc: Keith Busch <kbusch@kernel.org>
cc: Jens Axboe <axboe@kernel.dk>
cc: Hannes Reinecke <hare@kernel.org>
cc: Christoph Hellwig <hch@lst.de>
cc: Alexander Viro <viro@zeniv.linux.org.uk>
cc: Paulo Alcantara <pc@manguebit.org>
cc: netfs@lists.linux.dev
cc: linux-block@vger.kernel.org
cc: linux-fsdevel@vger.kernel.org
---
 lib/iov_iter.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 6665372ecf71..c489569a5815 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1921,15 +1921,22 @@ ssize_t iov_iter_extract_bvecs(struct iov_iter *iter, struct bio_vec *bv,
 		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)
+	if (likely(iter_is_ubuf(iter) ||
+		   iter_is_iovec(iter) ||
+		   iov_iter_is_kvec(iter))) {
+		unsigned long start = (unsigned long)iter_iov_addr(iter);
+
+		if ((start | iter_iov_len(iter)) & mem_align_mask)
+			return -EINVAL;
+	} else if (iov_iter_alignment(iter) & mem_align_mask) {
 		return -EINVAL;
+	}
 
 	/*
 	 * Move page array up in the allocated memory for the bio vecs as far as

Re: [PATCH v2] block: Fix start and length check added to iov_iter_extract_bvecs()

[Keith Busch]

On Wed, Aug 26, 2026 at 09:46:32PM +0100, David Howells wrote:
> Fixes: 14b007e17881 ("block: validate user space vectors during extraction")
> Suggested-by: Keith Busch <kbusch@kernel.org>

Thanks, looks good.

Reviewed-by: Keith Busch <kbusch@kernel.org>

Missing your Signed-off-by?
 
> cc: Keith Busch <kbusch@kernel.org>
> cc: Jens Axboe <axboe@kernel.dk>
> cc: Hannes Reinecke <hare@kernel.org>
> cc: Christoph Hellwig <hch@lst.de>
> cc: Alexander Viro <viro@zeniv.linux.org.uk>
> cc: Paulo Alcantara <pc@manguebit.org>
> cc: netfs@lists.linux.dev
> cc: linux-block@vger.kernel.org
> cc: linux-fsdevel@vger.kernel.org
> ---
>  lib/iov_iter.c |   11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index 6665372ecf71..c489569a5815 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -1921,15 +1921,22 @@ ssize_t iov_iter_extract_bvecs(struct iov_iter *iter, struct bio_vec *bv,
>  		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)
> +	if (likely(iter_is_ubuf(iter) ||
> +		   iter_is_iovec(iter) ||
> +		   iov_iter_is_kvec(iter))) {
> +		unsigned long start = (unsigned long)iter_iov_addr(iter);
> +
> +		if ((start | iter_iov_len(iter)) & mem_align_mask)
> +			return -EINVAL;
> +	} else if (iov_iter_alignment(iter) & mem_align_mask) {
>  		return -EINVAL;
> +	}
>  
>  	/*
>  	 * Move page array up in the allocated memory for the bio vecs as far as
> 

Re: [PATCH v2] block: Fix start and length check added to iov_iter_extract_bvecs()

[David Howells]

I forgot to add:

Signed-off-by: David Howells <dhowells@redhat.com>

Re: [PATCH v2] block: Fix start and length check added to iov_iter_extract_bvecs()

[Christoph Hellwig]

Should this grow a comment explaining why for non-iov/kvec/ubuf
we check the alignment of every sector instead of just the
start alignment?

Re: [PATCH v2] block: Fix start and length check added to iov_iter_extract_bvecs()

[David Howells]

Christoph Hellwig <hch@lst.de> wrote:

> Should this grow a comment explaining why for non-iov/kvec/ubuf
> we check the alignment of every sector instead of just the
> start alignment?

There should possibly be a comment explaining the need for an address
alignment at all.  Is it mandatory for all devices?  Or are we being a bit
over restrictive?  I don't think NICs capable of RDMA are so restricted, for
example.

Note that iov_iter_alignment() for ITER_XARRAY and ITER_FOLIOQ basically just
check iov_offset and count anyway as they refer to whole pages/folios only,
with just the start and end reduced.

David

Re: [PATCH v2] block: Fix start and length check added to iov_iter_extract_bvecs()

[Keith Busch]

On Fri, Sep 04, 2026 at 10:42:17AM +0100, David Howells wrote:
> Christoph Hellwig <hch@lst.de> wrote:
> 
> > Should this grow a comment explaining why for non-iov/kvec/ubuf
> > we check the alignment of every sector instead of just the
> > start alignment?
> 
> There should possibly be a comment explaining the need for an address
> alignment at all.  Is it mandatory for all devices?  Or are we being a bit
> over restrictive?  I don't think NICs capable of RDMA are so restricted, for
> example.

If a block device behind an RDMA NIC really wants to DMA a single byte
at a time, we can't describe that capability today. That would require a
0 mask, which is currently treated as "unset" and overridden with a
default 512b mask.

Re: [PATCH v2] block: Fix start and length check added to iov_iter_extract_bvecs()

[David Howells]

Keith Busch <kbusch@kernel.org> wrote:

> If a block device behind an RDMA NIC really wants to DMA a single byte
> at a time, we can't describe that capability today. That would require a
> 0 mask, which is currently treated as "unset" and overridden with a
> default 512b mask.

The thing is, Christoph said:

    Massage __bio_iov_iter_get_pages so that it doesn't need the bio, and
    move it to lib/iov_iter.c so that it can be used by block code for
    other things than filling a bio and by other subsystems like netfs.

but unless it can have a 1-byte alignment, it's not actually much use for
netfslib (I have to be able to support "unbuffered writes", which are almost
exactly like DIO writes but without alignment restrictions).

Do block devices really need an aligned "start memory address" or just an
aligned "start file position" (whatever that means for a blockdev).

David