On Fri, 01 Nov 2024 21:08:02 +0900 Asahi Lina <lina@xxxxxxxxxxxxx> wrote:
> If the source page is a PFN mapping, we copy back from userspace.
> However, if this fault is a remote access, we cannot use
> __copy_from_user_inatomic. Instead, use access_remote_vm() in this case.
>
> Fixes WARN and incorrect zero-filling when writing to CoW mappings in
> a remote process, such as when using gdb on a binary present on a DAX
> filesystem.
>
> [ 143.683782] ------------[ cut here ]------------
> [ 143.683784] WARNING: CPU: 1 PID: 350 at mm/memory.c:2904 __wp_page_copy_user+0x120/0x2bc
>
> ...
>
Thanks. I assume we should backport this into earlier kernels?
If so, a Fixes: target is desired, to tell people how far back in time
it should be ported. I think it's
83d116c53058 ("mm: fix double page fault on arm64 if PTE_AF is cleared").
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3081,13 +3081,18 @@ static inline int __wp_page_copy_user(struct page *dst, struct page *src,
> update_mmu_cache_range(vmf, vma, addr, vmf->pte, 1);
> }
>
> + /* If the mm is a remote mm, copy in the page using access_remote_vm() */
> + if (current->mm != mm) {
> + if (access_remote_vm(mm, (unsigned long)uaddr, kaddr, PAGE_SIZE, 0) != PAGE_SIZE)
> + goto warn;
> + }
> /*
> * This really shouldn't fail, because the page is there
> * in the page tables. But it might just be unreadable,
> * in which case we just give up and fill the result with
> * zeroes.
> */
> - if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE)) {
> + else if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE)) {
> if (vmf->pte)
> goto warn;
>
The coding style ends up being unconventional. I made these changes:
--- a/mm/memory.c~mm-fix-__wp_page_copy_user-fallback-path-for-remote-mm-fix
+++ a/mm/memory.c
@@ -3081,18 +3081,20 @@ static inline int __wp_page_copy_user(st
update_mmu_cache_range(vmf, vma, addr, vmf->pte, 1);
}
- /* If the mm is a remote mm, copy in the page using access_remote_vm() */
- if (current->mm != mm) {
- if (access_remote_vm(mm, (unsigned long)uaddr, kaddr, PAGE_SIZE, 0) != PAGE_SIZE)
- goto warn;
- }
/*
- * This really shouldn't fail, because the page is there
- * in the page tables. But it might just be unreadable,
- * in which case we just give up and fill the result with
- * zeroes.
+ * If the mm is a remote mm, copy in the page using access_remote_vm()
*/
- else if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE)) {
+ if (current->mm != mm) {
+ if (access_remote_vm(mm, (unsigned long)uaddr, kaddr,
+ PAGE_SIZE, 0) != PAGE_SIZE)
+ goto warn;
+ } else if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE)) {
+ /*
+ * This really shouldn't fail, because the page is there
+ * in the page tables. But it might just be unreadable,
+ * in which case we just give up and fill the result with
+ * zeroes.
+ */
if (vmf->pte)
goto warn;
_
I'll queue this for testing and shall await further review.
