在 2024/2/29 6:57, Linus Torvalds 写道:
On Wed, 28 Feb 2024 at 13:21, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
Hmm. If the copy doesn't succeed and make any progress at all, then
the code in generic_perform_write() after the "goto again"
//[4]
if (unlikely(fault_in_iov_iter_readable(i, bytes) ==
bytes)) {
should break out of the loop.
Ahh. I see the problem. Or at least part of it.
The iter is an ITER_BVEC.
And fault_in_iov_iter_readable() "knows" that an ITER_BVEC cannot
fail. Because obviously it's a kernel address, so no user page fault.
But for the machine check case, ITER_BVEC very much can fail.
This should never have worked in the first place.
What a crock.
Do we need to make iterate_bvec() always succeed fully, and make
copy_mc_to_kernel() zero out the end?
Linus
.
Hi Linus:
See the logic before this patch, always success (((void)(K),0)) is
returned for three types: ITER_BVEC, ITER_KVEC and ITER_XARRAY.
-------------------------------------------------------------------
-#define __iterate_and_advance(i, n, base, len, off, I, K) { \
- if (unlikely(i->count < n)) \
- n = i->count; \
- if (likely(n)) { \
- if (likely(iter_is_ubuf(i))) { \
[...] \
- iterate_buf(i, n, base, len, off, \
- i->ubuf, (I)) \
- } else if (likely(iter_is_iovec(i))) { \
[...] \
- iterate_iovec(i, n, base, len, off, \
- iov, (I)) \
- i->nr_segs -= iov - iter_iov(i); \
- i->__iov = iov; \
- } else if (iov_iter_is_bvec(i)) { \
[...] \
- iterate_bvec(i, n, base, len, off, \
- bvec, (K)) \
- i->nr_segs -= bvec - i->bvec; \
- i->bvec = bvec; \
- } else if (iov_iter_is_kvec(i)) { \
[...] \
- iterate_iovec(i, n, base, len, off, \
- kvec, (K)) \
[...] \
- } else if (iov_iter_is_xarray(i)) { \
[...] \
- iterate_xarray(i, n, base, len, off, \
- (K)) \
- } \
- i->count -= n; \
- } \
-}
-#define iterate_and_advance(i, n, base, len, off, I, K) \
- __iterate_and_advance(i, n, base, len, off, I, ((void)(K),0))
-------------------------------------------------------------------
Maybe we're all gonna fix it back? as follows：
-------------------------------------------------------------------
--- a/include/linux/iov_iter.h
+++ b/include/linux/iov_iter.h
@@ -246,11 +246,11 @@ size_t iterate_and_advance2(struct iov_iter
*iter, size_t len, void *priv,
if (likely(iter_is_iovec(iter)))
return iterate_iovec(iter, len, priv, priv2, ustep);
if (iov_iter_is_bvec(iter))
- return iterate_bvec(iter, len, priv, priv2, step);
+ return iterate_bvec(iter, len, priv, priv2, ((void
*)step, 0));
if (iov_iter_is_kvec(iter))
- return iterate_kvec(iter, len, priv, priv2, step);
+ return iterate_kvec(iter, len, priv, priv2, ((void
*)step, 0));
if (iov_iter_is_xarray(iter))
- return iterate_xarray(iter, len, priv, priv2, step);
+ return iterate_xarray(iter, len, priv, priv2, ((void
*)step, 0));
return iterate_discard(iter, len, priv, priv2, step);
}
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index e0aa6b440ca5..fabd5b1b97c7 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -257,7 +257,7 @@ static size_t __copy_from_iter_mc(void *addr,
size_t bytes, struct iov_iter *i)
bytes = i->count;
if (unlikely(!bytes))
return 0;
- return iterate_bvec(i, bytes, addr, NULL, memcpy_from_iter_mc);
+ return iterate_bvec(i, bytes, addr, NULL, ((void
*)memcpy_from_iter_mc, 0));
}
static __always_inline
-------------------------------------------------------------------
Hi, maintainer Alexander, what do you think ? :)
Thanks,
Tong.
