Re: [bug report] dead loop in generic_perform_write() //Re: [PATCH v7 07/12] iov_iter: Convert iterate*() to inline funcs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 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.





[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux