lib/iov_iter: initialize "flags" in new pipe_buffer

commit 9d2231c5d74e13b2a0546fee6737ee4446017903 upstream.

The functions copy_page_to_iter_pipe() and push_pipe() can both
allocate a new pipe_buf

lib/iov_iter: initialize "flags" in new pipe_buffer

commit 9d2231c5d74e13b2a0546fee6737ee4446017903 upstream.

The functions copy_page_to_iter_pipe() and push_pipe() can both
allocate a new pipe_buffer, but the "flags" member initializer is
missing.

Fixes: 241699cd72a8 ("new iov_iter flavour: pipe-backed")
To: Alexander Viro <viro@zeniv.linux.org.uk>
To: linux-fsdevel@vger.kernel.org
To: linux-kernel@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

show more ...

iov_iter: Fix iov_iter_get_pages{,_alloc} page fault return value

[ Upstream commit 814a66741b9ffb5e1ba119e368b178edb0b7322d ]

Both iov_iter_get_pages and iov_iter_get_pages_alloc return the number

iov_iter: Fix iov_iter_get_pages{,_alloc} page fault return value

[ Upstream commit 814a66741b9ffb5e1ba119e368b178edb0b7322d ]

Both iov_iter_get_pages and iov_iter_get_pages_alloc return the number
of bytes of the iovec they could get the pages for. When they cannot
get any pages, they're supposed to return 0, but when the start of the
iovec isn't page aligned, the calculation goes wrong and they return a
negative value. Fix both functions.

In addition, change iov_iter_get_pages_alloc to return NULL in that case
to prevent resource leaks.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>

show more ...

iov_iter: add helper to save iov_iter state

In an ideal world, when someone is passed an iov_iter and returns X bytes,
then X bytes would have been consumed/advanced from the iov_iter. But we
have u

iov_iter: add helper to save iov_iter state

In an ideal world, when someone is passed an iov_iter and returns X bytes,
then X bytes would have been consumed/advanced from the iov_iter. But we
have use cases that always consume the entire iterator, a few examples
of that are iomap and bdev O_DIRECT. This means we cannot rely on the
state of the iov_iter once we've called ->read_iter() or ->write_iter().

This would be easier if we didn't always have to deal with truncate of
the iov_iter, as rewinding would be trivial without that. We recently
added a commit to track the truncate state, but that grew the iov_iter
by 8 bytes and wasn't the best solution.

Implement a helper to save enough of the iov_iter state to sanely restore
it after we've called the read/write iterator helpers. This currently
only works for IOVEC/BVEC/KVEC as that's all we need, support for other
iterator types are left as an exercise for the reader.

Link: https://lore.kernel.org/linux-fsdevel/CAHk-=wiacKV4Gh-MYjteU0LwNBSGpWrK-Ov25HdqB1ewinrFPg@mail.gmail.com/
Signed-off-by: Jens Axboe <axboe@kernel.dk>

show more ...

lib/iov_iter.c: fix kernel-doc warnings

Fix all kernel-doc warnings in lib/iov_iter.c:

lib/iov_iter.c:695: warning: Function parameter or member 'i' not described in '_copy_mc_to_iter'
lib/iov_iter

lib/iov_iter.c: fix kernel-doc warnings

Fix all kernel-doc warnings in lib/iov_iter.c:

lib/iov_iter.c:695: warning: Function parameter or member 'i' not described in '_copy_mc_to_iter'
lib/iov_iter.c:695: warning: Excess function parameter 'iter' description in '_copy_mc_to_iter'
lib/iov_iter.c:695: warning: No description found for return value of '_copy_mc_to_iter'
lib/iov_iter.c:758: warning: Function parameter or member 'i' not described in '_copy_from_iter_flushcache'
lib/iov_iter.c:758: warning: Excess function parameter 'iter' description in '_copy_from_iter_flushcache'
lib/iov_iter.c:758: warning: No description found for return value of '_copy_from_iter_flushcache'

Link: https://lkml.kernel.org/r/20210809051053.6531-1-rdunlap@infradead.org
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

show more ...

iov_iter: remove uaccess_kernel() warning from iov_iter_init()

This warning was there to catch any architectures that still use
CONFIG_SET_FS, and that would mis-use iov_iter_init() for anything tha

iov_iter: remove uaccess_kernel() warning from iov_iter_init()

This warning was there to catch any architectures that still use
CONFIG_SET_FS, and that would mis-use iov_iter_init() for anything that
wasn't a proper user space pointer. So that

WARN_ON_ONCE(uaccess_kernel());

makes perfect conceptual sense: you really shouldn't use a kernel
pointer with set_fs(KERNEL_DS) and then pass it to iov_iter_init().

HOWEVER.

Guenter Roeck reports that this warning actually triggers in no-mmu
configurations of both ARM and m68k. And the reason isn't that they
pass in a kernel pointer under set_fs(KERNEL_DS) at all: the reason is
that in those configurations, "uaccess_kernel()" is simply not reliable.

Those no-mmu setups set USER_DS and KERNEL_DS to the same values, so you
can't test for the difference.

In particular, the no-mmu case for ARM does

#define USER_DS KERNEL_DS
#define uaccess_kernel() (true)

so USER_DS and KERNEL_DS have the same value, and uaccess_kernel() is
always trivially true.

The m68k case is slightly different and not quite as obvious. It does
(spread out over multiple header files just to be extra exciting:
asm/processor.h, asm/segment.h and asm-generic/uaccess.h):

#define TASK_SIZE (0xFFFFFFFFUL)
#define USER_DS MAKE_MM_SEG(TASK_SIZE)
#define KERNEL_DS MAKE_MM_SEG(~0UL)
#define get_fs() (current_thread_info()->addr_limit)
#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)

but the end result is the same: uaccess_kernel() will always be true,
because USER_DS and KERNEL_DS end up having the same value, even if that
value is defined differently.

This is very arguably a misfeature in those implementations, but in the
end we don't really care. All modern architectures have gotten rid of
set_fs() already, and generic kernel code never uses it. And while the
sanity check was a nice idea, an architecture would have to go the extra
mile to actually break this.

So this well-intentioned warning isn't really all that likely to find
anything but these known false positives, and as such just isn't worth
maintaining.

Reported-by: Guenter Roeck <linux@roeck-us.net>
Fixes: 8cd54c1c8480 ("iov_iter: separate direction from flavour")
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

show more ...

csum_and_copy_to_pipe_iter(): leave handling of csum_state to caller

... since all the logics is already there for use by iovec/kvec/etc.
cases.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

clean up copy_mc_pipe_to_iter()

... and we don't need kmap_atomic() there - kmap_local_page() is fine.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

pipe_zero(): we don't need no stinkin' kmap_atomic()...

FWIW, memcpy_to_page() itself almost certainly ought to
use kmap_local_page()...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

iov_iter: clean csum_and_copy_...() primitives up a bit

1) kmap_atomic() is not needed here, kmap_local_page() is enough.
2) No need to make sum = csum_block_add(sum, next, off); conditional
upon ne

iov_iter: clean csum_and_copy_...() primitives up a bit

1) kmap_atomic() is not needed here, kmap_local_page() is enough.
2) No need to make sum = csum_block_add(sum, next, off); conditional
upon next != 0 - adding 0 is a no-op as far as csum_block_add()
is concerned.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

copy_page_from_iter(): don't need kmap_atomic() for kvec/bvec cases

kmap_local_page() is enough.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

copy_page_to_iter(): don't bother with kmap_atomic() for bvec/kvec cases

kmap_local_page() is enough there. Moreover, we can use _copy_to_iter()
for actual copying in those cases - no useful extra

copy_page_to_iter(): don't bother with kmap_atomic() for bvec/kvec cases

kmap_local_page() is enough there. Moreover, we can use _copy_to_iter()
for actual copying in those cases - no useful extra checks on the
address we are copying from in that call.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

iterate_xarray(): only of the first iteration we might get offset != 0

recalculating offset on each iteration is pointless - on all subsequent
passes through the loop it will be zero anyway.

Signed

iterate_xarray(): only of the first iteration we might get offset != 0

recalculating offset on each iteration is pointless - on all subsequent
passes through the loop it will be zero anyway.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

pull handling of ->iov_offset into iterate_{iovec,bvec,xarray}

fewer arguments (by one, but still...) for iterate_...() macros

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

iov_iter: make iterator callbacks use base and len instead of iovec

Iterator macros used to provide the arguments for step callbacks in
a structure matching the flavour - iovec for ITER_IOVEC, kvec

iov_iter: make iterator callbacks use base and len instead of iovec

Iterator macros used to provide the arguments for step callbacks in
a structure matching the flavour - iovec for ITER_IOVEC, kvec for
ITER_KVEC and bio_vec for ITER_BVEC. That already broke down for
ITER_XARRAY (bio_vec there); now that we are using kvec callback
for bvec and xarray cases, we are always passing a pointer + length
(void __user * + size_t for ITER_IOVEC callback, void * + size_t
for everything else).

Note that the original reason for bio_vec (page + offset + len) in
case of ITER_BVEC used to be that we did *not* want to kmap a
page when all we wanted was e.g. to find the alignment of its
subrange. Now all such users are gone and the ones that are left
want the page mapped anyway for actually copying the data.

So in all cases we have pointer + length, and there's no good
reason for keeping those in struct iovec or struct kvec - we
can just pass them to callback separately.

Again, less boilerplate in callbacks...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

iov_iter: make the amount already copied available to iterator callbacks

Making iterator macros keep track of the amount of data copied is pretty
easy and it has several benefits:
1) we no longer n

iov_iter: make the amount already copied available to iterator callbacks

Making iterator macros keep track of the amount of data copied is pretty
easy and it has several benefits:
1) we no longer need the mess like (from += v.iov_len) - v.iov_len
in the callbacks - initial value + total amount copied so far would do
just fine.
2) less obviously, we no longer need to remember the initial amount
of data we wanted to copy; the loops in iterator macros are along the lines
of
wanted = bytes;
while (bytes) {
copy some
bytes -= copied
if short copy
break
}
bytes = wanted - bytes;
Replacement is
offs = 0;
while (bytes) {
copy some
offs += copied
bytes -= copied
if short copy
break
}
bytes = offs;
That wouldn't be a win per se, but unlike the initial value of bytes, the amount
copied so far *is* useful in callbacks.
3) in some cases (csum_and_copy_..._iter()) we already had offs manually
maintained by the callbacks. With that change we can drop that.

Less boilerplate and more readable code...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

iov_iter: get rid of separate bvec and xarray callbacks

After the previous commit we have
* xarray and bvec callbacks idential in all cases
* both equivalent to kvec callback wrapped into
kmap_loc

iov_iter: get rid of separate bvec and xarray callbacks

After the previous commit we have
* xarray and bvec callbacks idential in all cases
* both equivalent to kvec callback wrapped into
kmap_local_page()/kunmap_local() pair.

So we can pass only two (iovec and kvec) callbacks to
iterate_and_advance() and let iterate_{bvec,xarray} wrap
it into kmap_local_page()/kunmap_local_page().

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

iov_iter: teach iterate_{bvec,xarray}() about possible short copies

... and now we finally can sort out the mess in _copy_mc_to_iter().
Provide a variant of iterate_and_advance() that does *NOT* ign

iov_iter: teach iterate_{bvec,xarray}() about possible short copies

... and now we finally can sort out the mess in _copy_mc_to_iter().
Provide a variant of iterate_and_advance() that does *NOT* ignore
the return values of bvec, xarray and kvec callbacks, use that in
_copy_mc_to_iter(). That gets rid of magic in those callbacks -
we used to need it so we'd get at least the right return value in
case of failure halfway through.

As a bonus, now iterator is advanced by the amount actually copied
for all flavours. That's what the callers expect and it used to do that
correctly in iovec and xarray cases. However, in kvec and bvec cases
the iterator had not been advanced on such failures, breaking the users.
Fixed now...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

iterate_bvec(): expand bvec.h macro forest, massage a bit

... incidentally, using pointer instead of index in an array
(the only change here) trims half-kilobyte of .text...

Signed-off-by: Al Viro

iterate_bvec(): expand bvec.h macro forest, massage a bit

... incidentally, using pointer instead of index in an array
(the only change here) trims half-kilobyte of .text...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

iov_iter: unify iterate_iovec and iterate_kvec

The differences between iterate_iovec and iterate_kvec are minor:
* kvec callback is treated as if it returned 0
* initialization of __p is with i->i

iov_iter: unify iterate_iovec and iterate_kvec

The differences between iterate_iovec and iterate_kvec are minor:
* kvec callback is treated as if it returned 0
* initialization of __p is with i->iov and i->kvec resp.
which is trivially dealt with.

No code generation changes - compiler is quite capable of turning
left = ((void)(STEP), 0);
__v.iov_len -= left;
(with no accesses to left downstream) and
(void)(STEP);
into the same code.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

iov_iter: massage iterate_iovec and iterate_kvec to logics similar to iterate_bvec

Premature optimization is the root of all evil... Trying
to unroll the first pass through the loop makes it harder

iov_iter: massage iterate_iovec and iterate_kvec to logics similar to iterate_bvec

Premature optimization is the root of all evil... Trying
to unroll the first pass through the loop makes it harder
to follow and not just for readers - compiler ends up
generating worse code than it would on a "non-optimized"
loop.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

iterate_and_advance(): get rid of magic in case when n is 0

iov_iter_advance() needs to do some non-trivial work when it's given
0 as argument (skip all empty iovecs, mostly). We used to implement

iterate_and_advance(): get rid of magic in case when n is 0

iov_iter_advance() needs to do some non-trivial work when it's given
0 as argument (skip all empty iovecs, mostly). We used to implement
it via iterate_and_advance(); we no longer do so and for all other
users of iterate_and_advance() zero length is a no-op.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

csum_and_copy_to_iter(): massage into form closer to csum_and_copy_from_iter()

Namely, have off counted starting from 0 rather than from csstate->off.
To compensate we need to shift the initial valu

csum_and_copy_to_iter(): massage into form closer to csum_and_copy_from_iter()

Namely, have off counted starting from 0 rather than from csstate->off.
To compensate we need to shift the initial value (csstate->sum) (rotate
by 8 bits, as usual for csum) and do the same after we are finished adding
the pieces up.

What we get out of that is a bit more redundancy in our variables - from
is always equal to addr + off, which will be useful several commits down
the road.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

iov_iter: replace iov_iter_copy_from_user_atomic() with iterator-advancing variant

Replacement is called copy_page_from_iter_atomic(); unlike the old primitive the
callers do *not* need to do iov_it

iov_iter: replace iov_iter_copy_from_user_atomic() with iterator-advancing variant

Replacement is called copy_page_from_iter_atomic(); unlike the old primitive the
callers do *not* need to do iov_iter_advance() after it. In case when they end
up consuming less than they'd been given they need to do iov_iter_revert() on
everything they had not consumed. That, however, needs to be done only on slow
paths.

All in-tree callers converted. And that kills the last user of iterate_all_kinds()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...

[xarray] iov_iter_npages(): just use DIV_ROUND_UP()

Compiler is capable of recognizing division by power of 2 and turning
it into shifts.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

iov_iter_npages(): don't bother with iterate_all_kinds()

note that in bvec case pages can be compound ones - we can't just assume
that each segment is covered by one (sub)page

Signed-off-by: Al Vir

iov_iter_npages(): don't bother with iterate_all_kinds()

note that in bvec case pages can be compound ones - we can't just assume
that each segment is covered by one (sub)page

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

show more ...