| 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
| ||
|
Message-ID: <e3aca3b7-4f38-410d-91d6-5a2521dedb6c@oracle.com>
Date: Tue, 12 Aug 2025 10:12:14 -0700
From: Anthony Yznaga <anthony.yznaga@...cle.com>
To: Yongting Lin <linyongting@...edance.com>
Cc: akpm@...ux-foundation.org, andreyknvl@...il.com, arnd@...db.de,
brauner@...nel.org, catalin.marinas@....com, dave.hansen@...el.com,
david@...hat.com, ebiederm@...ssion.com, khalid@...nel.org,
linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, luto@...nel.org, markhemm@...glemail.com,
maz@...nel.org, mhiramat@...nel.org, neilb@...e.de, pcc@...gle.com,
rostedt@...dmis.org, vasily.averin@...ux.dev, viro@...iv.linux.org.uk,
willy@...radead.org, xhao@...ux.alibaba.com
Subject: Re: [PATCH v2 13/20] x86/mm: enable page table sharing
On 8/12/25 6:46 AM, Yongting Lin wrote:
> Hi,
>
> On 4/4/25 10:18 AM, Anthony Yznaga wrote:
>> Enable x86 support for handling page faults in an mshare region by
>> redirecting page faults to operate on the mshare mm_struct and vmas
>> contained in it.
>> Some permissions checks are done using vma flags in architecture-specfic
>> fault handling code so the actual vma needed to complete the handling
>> is acquired before calling handle_mm_fault(). Because of this an
>> ARCH_SUPPORTS_MSHARE config option is added.
>>
>> Signed-off-by: Anthony Yznaga <anthony.yznaga@...cle.com>
>> ---
>> arch/Kconfig | 3 +++
>> arch/x86/Kconfig | 1 +
>> arch/x86/mm/fault.c | 37 ++++++++++++++++++++++++++++++++++++-
>> mm/Kconfig | 2 +-
>> 4 files changed, 41 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/Kconfig b/arch/Kconfig
>> index 9f6eb09ef12d..2e000fefe9b3 100644
>> --- a/arch/Kconfig
>> +++ b/arch/Kconfig
>> @@ -1652,6 +1652,9 @@ config HAVE_ARCH_PFN_VALID
>> config ARCH_SUPPORTS_DEBUG_PAGEALLOC
>> bool
>>
>> +config ARCH_SUPPORTS_MSHARE
>> + bool
>> +
>> config ARCH_SUPPORTS_PAGE_TABLE_CHECK
>> bool
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 1502fd0c3c06..1f1779decb44 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -125,6 +125,7 @@ config X86
>> select ARCH_SUPPORTS_ACPI
>> select ARCH_SUPPORTS_ATOMIC_RMW
>> select ARCH_SUPPORTS_DEBUG_PAGEALLOC
>> + select ARCH_SUPPORTS_MSHARE if X86_64
>> select ARCH_SUPPORTS_PAGE_TABLE_CHECK if X86_64
>> select ARCH_SUPPORTS_NUMA_BALANCING if X86_64
>> select ARCH_SUPPORTS_KMAP_LOCAL_FORCE_MAP if NR_CPUS <= 4096
>> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
>> index 296d294142c8..49659d2f9316 100644
>> --- a/arch/x86/mm/fault.c
>> +++ b/arch/x86/mm/fault.c
>> @@ -1216,6 +1216,8 @@ void do_user_addr_fault(struct pt_regs *regs,
>> struct mm_struct *mm;
>> vm_fault_t fault;
>> unsigned int flags = FAULT_FLAG_DEFAULT;
>> + bool is_shared_vma;
>> + unsigned long addr;
>>
>> tsk = current;
>> mm = tsk->mm;
>> @@ -1329,6 +1331,12 @@ void do_user_addr_fault(struct pt_regs *regs,
>> if (!vma)
>> goto lock_mmap;
>>
>> + /* mshare does not support per-VMA locks yet */
>> + if (vma_is_mshare(vma)) {
>> + vma_end_read(vma);
>> + goto lock_mmap;
>> + }
>> +
>> if (unlikely(access_error(error_code, vma))) {
>> bad_area_access_error(regs, error_code, address, NULL, vma);
>> count_vm_vma_lock_event(VMA_LOCK_SUCCESS);
>> @@ -1357,17 +1365,38 @@ void do_user_addr_fault(struct pt_regs *regs,
>> lock_mmap:
>>
>> retry:
>> + addr = address;
>> + is_shared_vma = false;
>> vma = lock_mm_and_find_vma(mm, address, regs);
>> if (unlikely(!vma)) {
>> bad_area_nosemaphore(regs, error_code, address);
>> return;
>> }
>>
>> + if (unlikely(vma_is_mshare(vma))) {
>> + fault = find_shared_vma(&vma, &addr);
>> +
>> + if (fault) {
>> + mmap_read_unlock(mm);
>> + goto done;
>> + }
>> +
>> + if (!vma) {
>> + mmap_read_unlock(mm);
>> + bad_area_nosemaphore(regs, error_code, address);
>> + return;
>> + }
>> +
>> + is_shared_vma = true;
>> + }
>> +
>> /*
>> * Ok, we have a good vm_area for this memory access, so
>> * we can handle it..
>> */
>> if (unlikely(access_error(error_code, vma))) {
>> + if (unlikely(is_shared_vma))
>> + mmap_read_unlock(vma->vm_mm);
>> bad_area_access_error(regs, error_code, address, mm, vma);
>> return;
>> }
>> @@ -1385,7 +1414,11 @@ void do_user_addr_fault(struct pt_regs *regs,
>> * userland). The return to userland is identified whenever
>> * FAULT_FLAG_USER|FAULT_FLAG_KILLABLE are both set in flags.
>> */
>> - fault = handle_mm_fault(vma, address, flags, regs);
>> + fault = handle_mm_fault(vma, addr, flags, regs);
>> +
>> + if (unlikely(is_shared_vma) && ((fault & VM_FAULT_COMPLETED) ||
>> + (fault & VM_FAULT_RETRY) || fault_signal_pending(fault, regs)))
>> + mmap_read_unlock(mm);
>
> I was backporting these patches of mshare to 5.15 kernel and trying to do some
> basic tests. Then found a potential issue.
>
> Reaching here means find_shared_vma function has been executed successfully
> and host_mm->mmap_lock has got locked.
>
> When returned fault variable has VM_FAULT_COMPLETED or VM_FAULT_RETRY flags,
> or fault_signal_pending(fault, regs) takes true, there is not chance to release
> locks of both mm and host_mm(i.e. vma->vm_mm) in the following Snippet of Code.
If VM_FAULT_COMPLETED or VM_FAULT_RETRY are returned then the
host_mm->mmap_lock will already have been released. See
fault_dirty_shared_page(), filemap_fault(), and other callers of
maybe_unlock_mmap_for_io(). I will add a comment to help make this
clearer. I also realized that fault_signal_pending() does not need to be
called here because it can only return true if VM_FAULT_RETRY is set so
I'll change that, too.
The checks in a 5.15 kernel will be different. You probably want
something like:
if (unlikely(is_shared_vma) && ((fault & VM_FAULT_RETRY) &&
(flags & FAULT_FLAG_ALLOW_RETRY)) ||
fault_signal_pending(fault, regs))
mmap_read_unlock(mm);
Anthony
>
> As a result, needs to release vma->vm_mm.mmap_lock as well.
>
> So it is supposed to be like below:
>
> - fault = handle_mm_fault(vma, address, flags, regs);
> + fault = handle_mm_fault(vma, addr, flags, regs);
> +
> + if (unlikely(is_shared_vma) && ((fault & VM_FAULT_COMPLETED) ||
> + (fault & VM_FAULT_RETRY) || fault_signal_pending(fault, regs))) {
> + mmap_read_unlock(vma->vm_mm);
> + mmap_read_unlock(mm);
> + }
>
>>
>> if (fault_signal_pending(fault, regs)) {
>> /*
>> @@ -1413,6 +1446,8 @@ void do_user_addr_fault(struct pt_regs *regs,
>> goto retry;
>> }
>>
>> + if (unlikely(is_shared_vma))
>> + mmap_read_unlock(vma->vm_mm);
>> mmap_read_unlock(mm);
>> done:
>> if (likely(!(fault & VM_FAULT_ERROR)))
>> diff --git a/mm/Kconfig b/mm/Kconfig
>> index e6c90db83d01..8a5a159457f2 100644
>> --- a/mm/Kconfig
>> +++ b/mm/Kconfig
>> @@ -1344,7 +1344,7 @@ config PT_RECLAIM
>>
>> config MSHARE
>> bool "Mshare"
>> - depends on MMU
>> + depends on MMU && ARCH_SUPPORTS_MSHARE
>> help
>> Enable msharefs: A ram-based filesystem that allows multiple
>> processes to share page table entries for shared pages. A file
>
> Yongting Lin.
Powered by blists - more mailing lists