linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
To: Pu Lehui <pulehui@huaweicloud.com>
Cc: mhiramat@kernel.org, oleg@redhat.com, peterz@infradead.org,
	akpm@linux-foundation.org, Liam.Howlett@oracle.com,
	vbabka@suse.cz, jannh@google.com, pfalcato@suse.de,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org, pulehui@huawei.com
Subject: Re: [PATCH v1 1/4] mm: Fix uprobe pte be overwritten when expanding vma
Date: Fri, 30 May 2025 10:28:11 +0100	[thread overview]
Message-ID: <2a13548c-afbb-40b2-a9b4-326b3958ba29@lucifer.local> (raw)
In-Reply-To: <20250529155650.4017699-2-pulehui@huaweicloud.com>

On Thu, May 29, 2025 at 03:56:47PM +0000, Pu Lehui wrote:
> From: Pu Lehui <pulehui@huawei.com>
>
> We encountered a BUG alert triggered by Syzkaller as follows:
>    BUG: Bad rss-counter state mm:00000000b4a60fca type:MM_ANONPAGES val:1
>
> And we can reproduce it with the following steps:
> 1. register uprobe on file at zero offset
> 2. mmap the file at zero offset:
>    addr1 = mmap(NULL, 2 * 4096, PROT_NONE, MAP_PRIVATE, fd, 0);
> 3. mremap part of vma1 to new vma2:
>    addr2 = mremap(addr1, 4096, 2 * 4096, MREMAP_MAYMOVE);
> 4. mremap back to orig addr1:
>    mremap(addr2, 4096, 4096, MREMAP_MAYMOVE | MREMAP_FIXED, addr1);
>
> In the step 3, the vma1 range [addr1, addr1 + 4096] will be remap to new
> vma2 with range [addr2, addr2 + 8192], and remap uprobe anon page from
> the vma1 to vma2, then unmap the vma1 range [addr1, addr1 + 4096]. In
> tht step 4, the vma2 range [addr2, addr2 + 4096] will be remap back to
> the addr range [addr1, addr1 + 4096]. Since the addr range
> [addr1 + 4096, addr1 + 8192] still maps the file, it will take
> vma_merge_new_range to expand the range, and then do uprobe_mmap in
> vma_complete. Since the merged vma pgoff is also zero offset, it will
> install uprobe anon page to the merged vma. However, the upcomming
> move_page_tables step, which use set_pte_at to remap the vma2 uprobe pte
> to the merged vma, will overwrite the newly uprobe pte in the merged
> vma, and lead that pte to be orphan.
>
> Since the uprobe pte will be remapped to the merged vma, we can
> remove the unnecessary uprobe_mmap upon merged vma.
>
> This problem was first find in linux-6.6.y and also exists in the
> community syzkaller:
> https://lore.kernel.org/all/000000000000ada39605a5e71711@google.com/T/
>
> CC: stable@vger.kernel.org
> Fixes: 2b1444983508 ("uprobes, mm, x86: Add the ability to install and remove uprobes breakpoints")
> Suggested-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Signed-off-by: Pu Lehui <pulehui@huawei.com>

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  mm/vma.c | 20 +++++++++++++++++---
>  mm/vma.h |  7 +++++++
>  2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/mm/vma.c b/mm/vma.c
> index 1c6595f282e5..b2d7c03d8aa4 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -169,6 +169,9 @@ static void init_multi_vma_prep(struct vma_prepare *vp,
>  	vp->file = vma->vm_file;
>  	if (vp->file)
>  		vp->mapping = vma->vm_file->f_mapping;
> +
> +	if (vmg && vmg->skip_vma_uprobe)
> +		vp->skip_vma_uprobe = true;
>  }
>
>  /*
> @@ -358,10 +361,13 @@ static void vma_complete(struct vma_prepare *vp, struct vma_iterator *vmi,
>
>  	if (vp->file) {
>  		i_mmap_unlock_write(vp->mapping);
> -		uprobe_mmap(vp->vma);
>
> -		if (vp->adj_next)
> -			uprobe_mmap(vp->adj_next);
> +		if (!vp->skip_vma_uprobe) {
> +			uprobe_mmap(vp->vma);
> +
> +			if (vp->adj_next)
> +				uprobe_mmap(vp->adj_next);
> +		}
>  	}
>
>  	if (vp->remove) {
> @@ -1823,6 +1829,14 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
>  		faulted_in_anon_vma = false;
>  	}
>
> +	/*
> +	 * If the VMA we are copying might contain a uprobe PTE, ensure
> +	 * that we do not establish one upon merge. Otherwise, when mremap()
> +	 * moves page tables, it will orphan the newly created PTE.
> +	 */
> +	if (vma->vm_file)
> +		vmg.skip_vma_uprobe = true;
> +
>  	new_vma = find_vma_prev(mm, addr, &vmg.prev);
>  	if (new_vma && new_vma->vm_start < addr + len)
>  		return NULL;	/* should never get here */
> diff --git a/mm/vma.h b/mm/vma.h
> index 9a8af9be29a8..0db066e7a45d 100644
> --- a/mm/vma.h
> +++ b/mm/vma.h
> @@ -19,6 +19,8 @@ struct vma_prepare {
>  	struct vm_area_struct *insert;
>  	struct vm_area_struct *remove;
>  	struct vm_area_struct *remove2;
> +
> +	bool skip_vma_uprobe :1;
>  };
>
>  struct unlink_vma_file_batch {
> @@ -120,6 +122,11 @@ struct vma_merge_struct {
>  	 */
>  	bool give_up_on_oom :1;
>
> +	/*
> +	 * If set, skip uprobe_mmap upon merged vma.
> +	 */
> +	bool skip_vma_uprobe :1;
> +
>  	/* Internal flags set during merge process: */
>
>  	/*
> --
> 2.34.1
>


  reply	other threads:[~2025-05-30  9:28 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-29 15:56 [PATCH v1 0/4] " Pu Lehui
2025-05-29 15:56 ` [PATCH v1 1/4] mm: " Pu Lehui
2025-05-30  9:28   ` Lorenzo Stoakes [this message]
2025-05-30 18:51   ` David Hildenbrand
2025-06-02 11:55     ` Lorenzo Stoakes
2025-06-02 12:26       ` David Hildenbrand
2025-06-02 13:26         ` Lorenzo Stoakes
2025-06-02 16:28           ` David Hildenbrand
2025-06-02 17:01             ` Lorenzo Stoakes
2025-06-03 12:16               ` David Hildenbrand
2025-06-04  1:51                 ` Andrew Morton
2025-05-29 15:56 ` [PATCH v1 2/4] mm: Expose abnormal new_pte during move_ptes Pu Lehui
2025-05-29 19:19   ` Andrew Morton
2025-05-30  1:24     ` Pu Lehui
2025-05-30  3:47       ` Andrew Morton
2025-05-30 10:21   ` Lorenzo Stoakes
2025-05-30 16:44     ` Oleg Nesterov
2025-05-29 15:56 ` [PATCH v1 3/4] selftests/mm: Extract read_sysfs and write_sysfs into vm_util Pu Lehui
2025-05-30 11:48   ` Lorenzo Stoakes
2025-06-03  7:17     ` Pu Lehui
2025-06-04  2:36       ` Andrew Morton
2025-06-04  8:21         ` Pu Lehui
2025-05-29 15:56 ` [PATCH v1 4/4] selftests/mm: Add test about uprobe pte be orphan during vma merge Pu Lehui
2025-05-30 11:32   ` Lorenzo Stoakes
2025-06-03  7:08     ` Pu Lehui
2025-06-03  9:56       ` Lorenzo Stoakes
2025-06-10 10:37   ` Aishwarya
2025-06-10 11:27     ` Pedro Falcato
2025-06-10 11:34       ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2a13548c-afbb-40b2-a9b4-326b3958ba29@lucifer.local \
    --to=lorenzo.stoakes@oracle.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=jannh@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhiramat@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=pfalcato@suse.de \
    --cc=pulehui@huawei.com \
    --cc=pulehui@huaweicloud.com \
    --cc=stable@vger.kernel.org \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox