lib/iov_iter.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
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
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?
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
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. Sure.
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.
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
On Sat, Sep 05, 2026 at 05:26:40PM +0100, David Howells wrote: > 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). The file position is totally irrelevant, the memory address, or in case of IOMMUs the IOVA (which keeps the lower bits of the offset aligned) matter. Most hardware, bother storage and networking (and lots of others) require at least 4-byte aka DWORD alignment. But if we want to support event less, we'll need to adjust the interface, which shouldn't be too horrible. > > David ---end quoted text---
I forgot to add: Signed-off-by: David Howells <dhowells@redhat.com>
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
>
© 2016 - 2026 Red Hat, Inc.