Re: [PATCH v1] proc/kcore: advance the vmalloc retry by the last copy, not the total

[Omar Sandoval]

On Fri, Oct 16, 2026 at 11:47:39PM -0400, Nickolai Zeldovich wrote:
> A readv() or preadv() of /proc/kcore fails with EFAULT, although every
> byte of the destination is writable, when the page of a later iovec is
> not present at the moment the copy of a KCORE_VMALLOC chunk is retried:
> swapped out, migrated by compaction or NUMA balancing, write-protected
> by KSM, or unmapped by MADV_DONTNEED from another thread. Only root can
> read /proc/kcore, and the read must use more than one iovec. On an
> x86_64 defconfig kernel (v7.3-rc7) a preadv() of one vmalloc page into
> two 2 KiB iovecs on separate pages, with a second thread repeatedly
> unmapping the second page, failed 77701 of 200000 calls with EFAULT;
> with the fix all 200000 succeed. Affected since v6.4.
> 
> read_kcore_iter() copies a KCORE_VMALLOC chunk with vread_iter(), which
> writes to the user buffer with page faults disabled and stops at the
> first iovec whose page is absent, after the earlier iovecs were
> written; the loop then faults the rest of the buffer in and calls
> vread_iter() again for what is left. It keeps the running total in
> `read` and advances the source address and the remaining length by that
> total:
> 
> 	read += vread_iter(iter, src, left);
> 	if (read == tsz)
> 		break;
> 	src += read;
> 	left -= read;
> 
> In the reproducer the first call copies the 2048 bytes of the first
> iovec and stops at the second: read is 2048, and src and left move by
> 2048, which is right. The page that fault_in_iov_iter_writeable() then
> faults in is unmapped again before the second call, which copies 0
> bytes: read is still 2048, but src moves by another 2048 to the end of
> the chunk and left drops to 0. The third call has nothing to copy, src
> moves by 2048 once more, left wraps around, and
> fault_in_iov_iter_writeable() fails on that length, so the read returns
> -EFAULT. When both short copies are non-zero (three or more iovecs) the
> extra call also copies bytes from the wrong offset of the page into the
> buffer before the failure. A single iovec is not affected: its nofault
> copy is all or nothing, so a short copy is always empty and the advance
> by the total is still the advance by the last copy.
> 
> Keep the return value of each vread_iter() call and advance src and
> left by that; `read` stays the running total compared with tsz.
> 
> Found while formally verifying vread_iter() against the kernel binary.
> 
> Fixes: 4c91c07c93bb ("mm: vmalloc: convert vread() to vread_iter()")
> Assisted-by: LLM
> Signed-off-by: Nickolai Zeldovich <nickolai@xxxxxxxxxxxxx>
> ---

Thanks for the fix!

Reviewed-by: Omar Sandoval <osandov@xxxxxxxxxxx>

Christian, do you mind taking this through the VFS tree?

Thanks,
Omar