linux-kernel - Re: [PATCH] mm: Fix __wp_page_copy_user fallback path for remote mm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c00226ea-6e29-4432-a1c4-a25e9e05df9c@redhat.com>
Date: Tue, 5 Nov 2024 13:03:51 +0100
From: David Hildenbrand <david@...hat.com>
To: Asahi Lina <lina@...hilina.net>, Andrew Morton <akpm@...ux-foundation.org>
Cc: Sergio Lopez Pascual <slp@...hat.com>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, asahi@...ts.linux.dev
Subject: Re: [PATCH] mm: Fix __wp_page_copy_user fallback path for remote mm

On 01.11.24 13:08, Asahi Lina 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
> [  143.683793] CPU: 1 PID: 350 Comm: gdb Not tainted 6.6.52 #1
> [  143.683794] Hardware name: linux,dummy-virt (DT)
> [  143.683795] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> [  143.683796] pc : __wp_page_copy_user+0x120/0x2bc
> [  143.683798] lr : __wp_page_copy_user+0x254/0x2bc
> [  143.683799] sp : ffff80008272b8b0
> [  143.683799] x29: ffff80008272b8b0 x28: 0000000000000000 x27: ffff000083bad580
> [  143.683801] x26: 0000000000000000 x25: 0000fffff7fd5000 x24: ffff000081db04c0
> [  143.683802] x23: ffff00014f24b000 x22: fffffc00053c92c0 x21: ffff000083502150
> [  143.683803] x20: 0000fffff7fd5000 x19: ffff80008272b9d0 x18: 0000000000000000
> [  143.683804] x17: ffff000081db0500 x16: ffff800080fe52a0 x15: 0000fffff7fd5000
> [  143.683804] x14: 0000000000bb1845 x13: 0000000000000080 x12: ffff80008272b880
> [  143.683805] x11: ffff000081d13600 x10: ffff000081d13608 x9 : ffff000081d1360c
> [  143.683806] x8 : ffff000083a16f00 x7 : 0000000000000010 x6 : ffff00014f24b000
> [  143.683807] x5 : ffff00014f24c000 x4 : 0000000000000000 x3 : ffff000083582000
> [  143.683807] x2 : 0000000000000f80 x1 : 0000fffff7fd5000 x0 : 0000000000001000
> [  143.683808] Call trace:
> [  143.683809]  __wp_page_copy_user+0x120/0x2bc
> [  143.683810]  wp_page_copy+0x98/0x5c0
> [  143.683813]  do_wp_page+0x250/0x530
> [  143.683814]  __handle_mm_fault+0x278/0x284
> [  143.683817]  handle_mm_fault+0x64/0x1e8
> [  143.683819]  faultin_page+0x5c/0x110
> [  143.683820]  __get_user_pages+0xc8/0x2f4
> [  143.683821]  get_user_pages_remote+0xac/0x30c
> [  143.683823]  __access_remote_vm+0xb4/0x368
> [  143.683824]  access_remote_vm+0x10/0x1c
> [  143.683826]  mem_rw.isra.0+0xc4/0x218
> [  143.683831]  mem_write+0x18/0x24
> [  143.683831]  vfs_write+0xa0/0x37c
> [  143.683834]  ksys_pwrite64+0x7c/0xc0
> [  143.683834]  __arm64_sys_pwrite64+0x20/0x2c
> [  143.683835]  invoke_syscall+0x48/0x10c
> [  143.683837]  el0_svc_common.constprop.0+0x40/0xe0
> [  143.683839]  do_el0_svc+0x1c/0x28
> [  143.683841]  el0_svc+0x3c/0xdc
> [  143.683846]  el0t_64_sync_handler+0x120/0x12c
> [  143.683848]  el0t_64_sync+0x194/0x198
> [  143.683849] ---[ end trace 0000000000000000 ]---
> 
> Signed-off-by: Asahi Lina <lina@...hilina.net>
> ---
>   mm/memory.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 3ccee51adfbbd007b24331fe6874265f231a877b..dba25d9734063ac02cdaeb0a5cd5432473f6372e 100644
> --- 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)

access_remote_vm() will do a mmap_read_lock_killable() and then call 
into get_user_page_vma_remote() -- fortunately read-access, otherwise 
we'd be in trouble :) .

So we should already be holding the mmap read lock from the previous 
access_remote_vm() users (who we end up here) ... doesn't this complain 
with lockdep about recursive locking?

I keep forgetting locking rules, so I might just be wrong.

-- 
Cheers,

David / dhildenb