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: <Z1ENEwafY7g/rzeo@ly-workstation>
Date: Thu, 5 Dec 2024 10:16:51 +0800
From: "Lai, Yi" <yi1.lai@...ux.intel.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org, Matthew Wilcox <willy@...radead.org>,
	David Hildenbrand <david@...hat.com>, yi1.lai@...el.com
Subject: Re: [PATCH v2] perf: map pages in advance

On Fri, Nov 29, 2024 at 03:31:34PM +0000, Lorenzo Stoakes wrote:
> We are current refactoring struct page to make it smaller, removing
> unneeded fields that correctly belong to struct folio.
> 
> Two of those fields are page->index and page->mapping. Perf is currently
> making use of both of these, so this patch removes this usage as it turns
> out it is unnecessary.
> 
> Perf establishes its own internally controlled memory-mapped pages using
> vm_ops hooks. The first page in the mapping is the read/write user control
> page, and the rest of the mapping consists of read-only pages.
> 
> The VMA is backed by kernel memory either from the buddy allocator or
> vmalloc depending on configuration. It is intended to be mapped read/write,
> but because it has a page_mkwrite() hook, vma_wants_writenotify() indicaets
> that it should be mapped read-only.
> 
> When a write fault occurs, the provided page_mkwrite() hook,
> perf_mmap_fault() (doing double duty handing faults as well) uses the
> vmf->pgoff field to determine if this is the first page, allowing for the
> desired read/write first page, read-only rest mapping.
> 
> For this to work the implementation has to carefully work around faulting
> logic. When a page is write-faulted, the fault() hook is called first, then
> its page_mkwrite() hook is called (to allow for dirty tracking in file
> systems).
> 
> On fault we set the folio's mapping in perf_mmap_fault(), this is because
> when do_page_mkwrite() is subsequently invoked, it treats a missing mapping
> as an indicator that the fault should be retried.
> 
> We also set the folio's index so, given the folio is being treated as faux
> user memory, it correctly references its offset within the VMA.
> 
> This explains why the mapping and index fields are used - but it's not
> necessary.
> 
> We preallocate pages when perf_mmap() is called for the first time via
> rb_alloc(), and further allocate auxiliary pages via rb_aux_alloc() as
> needed if the mapping requires it.
> 
> This allocation is done in the f_ops->mmap() hook provided in perf_mmap(),
> and so we can instead simply map all the memory right away here - there's
> no point in handling (read) page faults when we don't demand page nor need
> to be notified about them (perf does not).
> 
> This patch therefore changes this logic to map everything when the mmap()
> hook is called, establishing a PFN map. It implements vm_ops->pfn_mkwrite()
> to provide the required read/write vs. read-only behaviour, which does not
> require the previously implemented workarounds.
> 
> While it is not ideal to use a VM_PFNMAP here, doing anything else will
> result in the page_mkwrite() hook need to be provided, which requires the
> same page->mapping hack this patch seeks to undo.
> 
> It will also result in the pages being treated as folios and placed on the
> rmap, which really does not make sense for these mappings.
> 
> Semantically it makes sense to establish this as some kind of special
> mapping, as the pages are managed by perf and are not strictly user pages,
> but currently the only means by which we can do so functionally while
> maintaining the required R/W and R/O bheaviour is a PFN map.
> 
> There should be no change to actual functionality as a result of this
> change.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> ---
> v2:
> * nommu fixup.
> * Add comment explaining why we are using a VM_PFNMAP as suggested by
>   David H.
> 
> v1:
> https://lore.kernel.org/all/20241128113714.492474-1-lorenzo.stoakes@oracle.com/
> 
>  kernel/events/core.c        | 116 ++++++++++++++++++++++++------------
>  kernel/events/ring_buffer.c |  19 +-----
>  2 files changed, 80 insertions(+), 55 deletions(-)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 5d4a54f50826..1bb5999d9d81 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6284,41 +6284,6 @@ void perf_event_update_userpage(struct perf_event *event)
>  }
>  EXPORT_SYMBOL_GPL(perf_event_update_userpage);
> 
> -static vm_fault_t perf_mmap_fault(struct vm_fault *vmf)
> -{
> -	struct perf_event *event = vmf->vma->vm_file->private_data;
> -	struct perf_buffer *rb;
> -	vm_fault_t ret = VM_FAULT_SIGBUS;
> -
> -	if (vmf->flags & FAULT_FLAG_MKWRITE) {
> -		if (vmf->pgoff == 0)
> -			ret = 0;
> -		return ret;
> -	}
> -
> -	rcu_read_lock();
> -	rb = rcu_dereference(event->rb);
> -	if (!rb)
> -		goto unlock;
> -
> -	if (vmf->pgoff && (vmf->flags & FAULT_FLAG_WRITE))
> -		goto unlock;
> -
> -	vmf->page = perf_mmap_to_page(rb, vmf->pgoff);
> -	if (!vmf->page)
> -		goto unlock;
> -
> -	get_page(vmf->page);
> -	vmf->page->mapping = vmf->vma->vm_file->f_mapping;
> -	vmf->page->index   = vmf->pgoff;
> -
> -	ret = 0;
> -unlock:
> -	rcu_read_unlock();
> -
> -	return ret;
> -}
> -
>  static void ring_buffer_attach(struct perf_event *event,
>  			       struct perf_buffer *rb)
>  {
> @@ -6558,13 +6523,87 @@ static void perf_mmap_close(struct vm_area_struct *vma)
>  	ring_buffer_put(rb); /* could be last */
>  }
> 
> +static vm_fault_t perf_mmap_pfn_mkwrite(struct vm_fault *vmf)
> +{
> +	/* The first page is the user control page, others are read-only. */
> +	return vmf->pgoff == 0 ? 0 : VM_FAULT_SIGBUS;
> +}
> +
>  static const struct vm_operations_struct perf_mmap_vmops = {
>  	.open		= perf_mmap_open,
>  	.close		= perf_mmap_close, /* non mergeable */
> -	.fault		= perf_mmap_fault,
> -	.page_mkwrite	= perf_mmap_fault,
> +	.pfn_mkwrite	= perf_mmap_pfn_mkwrite,
>  };
> 
> +static int map_range(struct perf_buffer *rb, struct vm_area_struct *vma)
> +{
> +	unsigned long nr_pages = vma_pages(vma);
> +	int err = 0;
> +	unsigned long pgoff;
> +
> +	/*
> +	 * We map this as a VM_PFNMAP VMA.
> +	 *
> +	 * This is not ideal as this is designed broadly for mappings of PFNs
> +	 * referencing memory-mapped I/O ranges or non-system RAM i.e. for which
> +	 * !pfn_valid(pfn).
> +	 *
> +	 * We are mapping kernel-allocated memory (memory we manage ourselves)
> +	 * which would more ideally be mapped using vm_insert_page() or a
> +	 * similar mechanism, that is as a VM_MIXEDMAP mapping.
> +	 *
> +	 * However this won't work here, because:
> +	 *
> +	 * 1. It uses vma->vm_page_prot, but this field has not been completely
> +	 *    setup at the point of the f_op->mmp() hook, so we are unable to
> +	 *    indicate that this should be mapped CoW in order that the
> +	 *    mkwrite() hook can be invoked to make the first page R/W and the
> +	 *    rest R/O as desired.
> +	 *
> +	 * 2. Anything other than a VM_PFNMAP of valid PFNs will result in
> +	 *    vm_normal_page() returning a struct page * pointer, which means
> +	 *    vm_ops->page_mkwrite() will be invoked rather than
> +	 *    vm_ops->pfn_mkwrite(), and this means we have to set page->mapping
> +	 *    to work around retry logic in the fault handler, however this
> +	 *    field is no longer allowed to be used within struct page.
> +	 *
> +	 * 3. Having a struct page * made available in the fault logic also
> +	 *    means that the page gets put on the rmap and becomes
> +	 *    inappropriately accessible and subject to map and ref counting.
> +	 *
> +	 * Ideally we would have a mechanism that could explicitly express our
> +	 * desires, but this is not currently the case, so we instead use
> +	 * VM_PFNMAP.
> +	 *
> +	 * We manage the lifetime of these mappings with internal refcounts (see
> +	 * perf_mmap_open() and perf_mmap_close()) so we ensure the lifetime of
> +	 * this mapping is maintained correctly.
> +	 */
> +	for (pgoff = 0; pgoff < nr_pages; pgoff++) {
> +		unsigned long va = vma->vm_start + PAGE_SIZE * pgoff;
> +		struct page *page = perf_mmap_to_page(rb, pgoff);
> +
> +		if (page == NULL) {
> +			err = -EINVAL;
> +			break;
> +		}
> +
> +		/* Map readonly, perf_mmap_pfn_mkwrite() called on write fault. */
> +		err = remap_pfn_range(vma, va, page_to_pfn(page), PAGE_SIZE,
> +				      vm_get_page_prot(vma->vm_flags & ~VM_SHARED));
> +		if (err)
> +			break;
> +	}
> +
> +#ifdef CONFIG_MMU
> +	/* Clear any partial mappings on error. */
> +	if (err)
> +		zap_page_range_single(vma, vma->vm_start, nr_pages * PAGE_SIZE, NULL);
> +#endif
> +
> +	return err;
> +}
> +
>  static int perf_mmap(struct file *file, struct vm_area_struct *vma)
>  {
>  	struct perf_event *event = file->private_data;
> @@ -6783,6 +6822,9 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
>  	vm_flags_set(vma, VM_DONTCOPY | VM_DONTEXPAND | VM_DONTDUMP);
>  	vma->vm_ops = &perf_mmap_vmops;
> 
> +	if (!ret)
> +		ret = map_range(rb, vma);
> +
>  	if (event->pmu->event_mapped)
>  		event->pmu->event_mapped(event, vma->vm_mm);
> 
> diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> index 4f46f688d0d4..180509132d4b 100644
> --- a/kernel/events/ring_buffer.c
> +++ b/kernel/events/ring_buffer.c
> @@ -643,7 +643,6 @@ static void rb_free_aux_page(struct perf_buffer *rb, int idx)
>  	struct page *page = virt_to_page(rb->aux_pages[idx]);
> 
>  	ClearPagePrivate(page);
> -	page->mapping = NULL;
>  	__free_page(page);
>  }
> 
> @@ -819,7 +818,6 @@ static void perf_mmap_free_page(void *addr)
>  {
>  	struct page *page = virt_to_page(addr);
> 
> -	page->mapping = NULL;
>  	__free_page(page);
>  }
> 
> @@ -890,28 +888,13 @@ __perf_mmap_to_page(struct perf_buffer *rb, unsigned long pgoff)
>  	return vmalloc_to_page((void *)rb->user_page + pgoff * PAGE_SIZE);
>  }
> 
> -static void perf_mmap_unmark_page(void *addr)
> -{
> -	struct page *page = vmalloc_to_page(addr);
> -
> -	page->mapping = NULL;
> -}
> -
>  static void rb_free_work(struct work_struct *work)
>  {
>  	struct perf_buffer *rb;
> -	void *base;
> -	int i, nr;
> 
>  	rb = container_of(work, struct perf_buffer, work);
> -	nr = data_page_nr(rb);
> -
> -	base = rb->user_page;
> -	/* The '<=' counts in the user page. */
> -	for (i = 0; i <= nr; i++)
> -		perf_mmap_unmark_page(base + (i * PAGE_SIZE));
> 
> -	vfree(base);
> +	vfree(rb->user_page);
>  	kfree(rb);
>  }
> 
> --
> 2.47.1

Hi Lorenzo Stoakes,

Greetings!

I used Syzkaller and found that there is general protection fault in perf_mmap_to_page in linux-next next-20241203.

After bisection and the first bad commit is:
"
eca51ce01d49 perf: Map pages in advance
"

All detailed into can be found at:
https://github.com/laifryiee/syzkaller_logs/tree/main/241204_084442_perf_mmap_to_page
Syzkaller repro code:
https://github.com/laifryiee/syzkaller_logs/tree/main/241204_084442_perf_mmap_to_page/repro.c
Syzkaller repro syscall steps:
https://github.com/laifryiee/syzkaller_logs/tree/main/241204_084442_perf_mmap_to_page/repro.prog
Syzkaller report:
https://github.com/laifryiee/syzkaller_logs/tree/main/241204_084442_perf_mmap_to_page/repro.report
Kconfig(make olddefconfig):
https://github.com/laifryiee/syzkaller_logs/tree/main/241204_084442_perf_mmap_to_page/kconfig_origin
Bisect info:
https://github.com/laifryiee/syzkaller_logs/tree/main/241204_084442_perf_mmap_to_page/bisect_info.log
bzImage:
https://github.com/laifryiee/syzkaller_logs/raw/refs/heads/main/241204_084442_perf_mmap_to_page/bzImage_c245a7a79602ccbee780c004c1e4abcda66aec32
Issue dmesg:
https://github.com/laifryiee/syzkaller_logs/blob/main/241204_084442_perf_mmap_to_page/c245a7a79602ccbee780c004c1e4abcda66aec32_dmesg.log

"
[   22.133358] KASAN: null-ptr-deref in range [0x0000000000000178-0x000000000000017f]
[   22.133907] CPU: 0 UID: 0 PID: 727 Comm: repro Not tainted 6.13.0-rc1-next-20241203-c245a7a79602 #1
[   22.134557] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
[   22.135371] RIP: 0010:perf_mmap_to_page+0x39/0x500
[   22.135763] Code: 41 56 41 55 41 54 49 89 f4 53 48 89 fb e8 3f 5f c2 ff 48 8d bb 78 01 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 3c 03 0f 8e e9 03 00 00 4c 63 ab 78 01 00
[   22.137075] RSP: 0018:ffff888020f0f798 EFLAGS: 00010202
[   22.137465] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: dffffc0000000000
[   22.137980] RDX: 000000000000002f RSI: ffffffff81a5ccf1 RDI: 0000000000000178
[   22.138495] RBP: ffff888020f0f7c0 R08: 0000000000000001 R09: ffffed10025fbdb0
[   22.139012] R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000000
[   22.139530] R13: 0000000000000000 R14: 0000000020002000 R15: ffff888011cce3c0
[   22.140047] FS:  00007f7f57f30600(0000) GS:ffff88806c400000(0000) knlGS:0000000000000000
[   22.140630] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   22.141052] CR2: 00000000200000c0 CR3: 0000000014e10004 CR4: 0000000000770ef0
[   22.141570] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   22.142088] DR3: 0000000000000000 DR6: 00000000ffff07f0 DR7: 0000000000000400
[   22.142606] PKRU: 55555554
[   22.142815] Call Trace:
[   22.143005]  <TASK>
[   22.143173]  ? show_regs+0x6d/0x80
[   22.143455]  ? die_addr+0x45/0xb0
[   22.143720]  ? exc_general_protection+0x1ae/0x340
[   22.144102]  ? asm_exc_general_protection+0x2b/0x30
[   22.144486]  ? perf_mmap_to_page+0x21/0x500
[   22.144810]  ? perf_mmap_to_page+0x39/0x500
[   22.145130]  ? perf_mmap_to_page+0x21/0x500
[   22.145448]  perf_mmap+0xbd9/0x1ce0
[   22.145729]  __mmap_region+0x10e7/0x25a0
[   22.146038]  ? __pfx___mmap_region+0x10/0x10
[   22.146376]  ? mark_lock.part.0+0xf3/0x17b0
[   22.146712]  ? __pfx_mark_lock.part.0+0x10/0x10
[   22.147071]  ? __kasan_check_read+0x15/0x20
[   22.147403]  ? mark_lock.part.0+0xf3/0x17b0
[   22.147744]  ? __sanitizer_cov_trace_const_cmp1+0x1e/0x30
[   22.148162]  ? trace_cap_capable+0x78/0x1e0
[   22.148500]  ? cap_capable+0xa4/0x250
[   22.148792]  mmap_region+0x248/0x2f0
[   22.149086]  do_mmap+0xb29/0x12a0
[   22.149355]  ? __pfx_do_mmap+0x10/0x10
[   22.149651]  ? __pfx_down_write_killable+0x10/0x10
[   22.150027]  ? __this_cpu_preempt_check+0x21/0x30
[   22.150393]  vm_mmap_pgoff+0x235/0x3e0
[   22.150699]  ? __pfx_vm_mmap_pgoff+0x10/0x10
[   22.151037]  ? __fget_files+0x1fb/0x3a0
[   22.151352]  ksys_mmap_pgoff+0x3dc/0x520
[   22.151664]  __x64_sys_mmap+0x139/0x1d0
[   22.151975]  x64_sys_call+0x2001/0x2140
[   22.152283]  do_syscall_64+0x6d/0x140
[   22.152572]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[   22.152960] RIP: 0033:0x7f7f57c3ee5d
[   22.153251] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 93 af 1b 00 f7 d8 64 89 01 48
[   22.154593] RSP: 002b:00007ffd805489f8 EFLAGS: 00000212 ORIG_RAX: 0000000000000009
[   22.155156] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f7f57c3ee5d
[   22.155683] RDX: 0000000000000000 RSI: 0000000000001000 RDI: 0000000020002000
[   22.156210] RBP: 00007ffd80548a20 R08: 0000000000000003 R09: 0000000000000000
[   22.156739] R10: 0000000000006053 R11: 0000000000000212 R12: 00007ffd80548b38
[   22.157263] R13: 0000000000401126 R14: 0000000000403e08 R15: 00007f7f57f77000
[   22.157799]  </TASK>
[   22.157975] Modules linked in:
[   22.158322] ---[ end trace 0000000000000000 ]---
[   22.158694] RIP: 0010:perf_mmap_to_page+0x39/0x500
[   22.159061] Code: 41 56 41 55 41 54 49 89 f4 53 48 89 fb e8 3f 5f c2 ff 48 8d bb 78 01 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 3c 03 0f 8e e9 03 00 00 4c 63 ab 78 01 00
[   22.160388] RSP: 0018:ffff888020f0f798 EFLAGS: 00010202
[   22.160782] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: dffffc0000000000
[   22.161304] RDX: 000000000000002f RSI: ffffffff81a5ccf1 RDI: 0000000000000178
[   22.161824] RBP: ffff888020f0f7c0 R08: 0000000000000001 R09: ffffed10025fbdb0
[   22.162344] R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000000
[   22.162877] R13: 0000000000000000 R14: 0000000020002000 R15: ffff888011cce3c0
[   22.163403] FS:  00007f7f57f30600(0000) GS:ffff88806c400000(0000) knlGS:0000000000000000
[   22.163988] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   22.164417] CR2: 00000000200000c0 CR3: 0000000014e10004 CR4: 0000000000770ef0
[   22.165409] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   22.165956] DR3: 0000000000000000 DR6: 00000000ffff07f0 DR7: 0000000000000400
[   22.166918] PKRU: 55555554
"

I hope you find it useful.

Regards,
Yi Lai

---

If you don't need the following environment to reproduce the problem or if you
already have one reproduced environment, please ignore the following information.

How to reproduce:
git clone https://gitlab.com/xupengfe/repro_vm_env.git
cd repro_vm_env
tar -xvf repro_vm_env.tar.gz
cd repro_vm_env; ./start3.sh  // it needs qemu-system-x86_64 and I used v7.1.0
  // start3.sh will load bzImage_2241ab53cbb5cdb08a6b2d4688feb13971058f65 v6.2-rc5 kernel
  // You could change the bzImage_xxx as you want
  // Maybe you need to remove line "-drive if=pflash,format=raw,readonly=on,file=./OVMF_CODE.fd \" for different qemu version
You could use below command to log in, there is no password for root.
ssh -p 10023 root@...alhost

After login vm(virtual machine) successfully, you could transfer reproduced
binary to the vm by below way, and reproduce the problem in vm:
gcc -pthread -o repro repro.c
scp -P 10023 repro root@...alhost:/root/

Get the bzImage for target kernel:
Please use target kconfig and copy it to kernel_src/.config
make olddefconfig
make -jx bzImage           //x should equal or less than cpu num your pc has

Fill the bzImage file into above start3.sh to load the target kernel in vm.


Tips:
If you already have qemu-system-x86_64, please ignore below info.
If you want to install qemu v7.1.0 version:
git clone https://github.com/qemu/qemu.git
cd qemu
git checkout -f v7.1.0
mkdir build
cd build
yum install -y ninja-build.x86_64
yum -y install libslirp-devel.x86_64
../configure --target-list=x86_64-softmmu --enable-kvm --enable-vnc --enable-gtk --enable-sdl --enable-usb-redir --enable-slirp
make
make install


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ