[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:

> +	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

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.

[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>