From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CC7F6188736 for ; Mon, 27 Jan 2025 17:08:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737997706; cv=none; b=sf36S3NLyOs4ad1LLpsHy8XXsrJ6bPQHk8nkzAeQ3suLhOLOE9VUufiPVsHJMH6ksz/asm6edHVXzw6tA8ZfIEduNdplRXPrKcBVBnPrnCRUyjkbtRBQ0j94DeGWBA9csVSB/6xsCb3cYwYM9YLhEMndrkkTuLuA2a3aB9pDdCs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737997706; c=relaxed/simple; bh=Kx6L5ne46GmL1a1lMEw14+eshv6c27PQsLZBsXrQjx4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=hQJMSzDyt3vXR4n+gdNERix1VVcmRsjDjwgmbPUxZXU+OvWgj5kHQlI2Ycn6jzMTWtYAMiz/Y3x4dZCQHrApI2gLrsgj5OMZNIelaOiB7PXXycBxbO6G8e4kUWAr2qYlOHNzMzXYZ0KZf+/LVS9VuUY7507ftzLPBTsjQAnTVW4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=nQ69Vy/A; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="nQ69Vy/A" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=2dPWRjFb8yCXJpAGicb+5JCs45LQ7/H/zcxH8kFVcAU=; b=nQ69Vy/A2wCPbl/pRTrrin/I/k BjrN2xZk5QIyA1xklnpys3slA38hO/gXybvXl14vu/xFfBFHg2D3ly0swjxAWc8sVUQEFG+WPoG21 O1avVBq7P86Fbh6YqLywIsfksnKEFj0S40jTKReU7u5IqBCCrG7ZMFKyrsXJr54j5bntLpGHPpIsJ 5ijc94VDyf+AztLCzy307ihHif8KxEEVufglF40RAa8Xr9vaY0lWQvK6VjvEA6kqe7ZJyjmqYX1It S2Th4tbrG0zySUGxA18DIXgrc9+R1Hxtg55zf/ZnQxGL9PW+wiC8c7kAbg5m0uclCp1EWRvcY6wLz 1YW+4IcA==; Received: from willy by casper.infradead.org with local (Exim 4.98 #2 (Red Hat Linux)) id 1tcSax-00000009gpB-1q2F; Mon, 27 Jan 2025 17:08:19 +0000 Date: Mon, 27 Jan 2025 17:08:19 +0000 From: Matthew Wilcox To: "Liam R. Howlett" Cc: Andrew Morton , maple-tree@lists.infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, Oleg Nesterov , Masami Hiramatsu , Jann Horn , Lorenzo Stoakes , Peter Zijlstra , Michal Hocko , Peng Zhang Subject: Re: [PATCH v2] kernel: Be more careful about dup_mmap() failures and uprobe registering Message-ID: References: <20250127170221.1761366-1-Liam.Howlett@oracle.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250127170221.1761366-1-Liam.Howlett@oracle.com> On Mon, Jan 27, 2025 at 12:02:21PM -0500, Liam R. Howlett wrote: > From: "Liam R. Howlett" > > In the even that there is a failure during dup_mmap(), the maple tree "event". Although the writing is a little clumsy here. You could just say "If there is a failure". But that begs the question of what kind of failure, and I think it's only a memory allocation failure? So you could say "If a memory allocation fails during dup_mmap()," > can be left in an unsafe state for other iterators besides the exit > path. All the locks are dropped before the exit_mmap() call (in > mm/mmap.c), but the incomplete mm_struct can be reached through (at > least) the rmap finding the vmas which have a pointer back to the > mm_struct. > > Up to this point, there have been no issues with being able to find an > mm_struct that was only partially initialised. Syzbot was able to make > the incomplete mm_struct fail with recent forking changes, so it has > been proven unsafe to use the mm_struct that hasn't been initialised, as > referenced in the link below. > > Although 8ac662f5da19f ("fork: avoid inappropriate uprobe access to > invalid mm") fixed the uprobe access, it does not completely remove the > race. > > This patch sets the MMF_OOM_SKIP to avoid the iteration of the vmas on > the oom side (even though this is extremely unlikely to be selected as > an oom victim in the race window), and sets MMF_UNSTABLE to avoid other > potential users from using a partially initialised mm_struct. > > When registering vmas for uprobe, skip the vmas in an mm that is marked > unstable. Modifying a vma in an unstable mm may cause issues if the mm > isn't fully initialised. > > Link: https://lore.kernel.org/all/6756d273.050a0220.2477f.003d.GAE@google.com/ > Fixes: d240629148377 ("fork: use __mt_dup() to duplicate maple tree in dup_mmap()") > Cc: Oleg Nesterov > Cc: Masami Hiramatsu > Cc: Jann Horn > Cc: Lorenzo Stoakes > Cc: Peter Zijlstra > Cc: Michal Hocko > Cc: Peng Zhang > Signed-off-by: Liam R. Howlett > --- > > v1: https://lore.kernel.org/all/20250123205849.793810-1-Liam.Howlett@oracle.com/ > > Changes since: > v1 > - Added check_stable_address_space() to uprobe code - Thanks Lorenzo > - Added Oleg & Masami to Cc list. > > kernel/events/uprobes.c | 4 ++++ > kernel/fork.c | 17 ++++++++++++++--- > 2 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c > index fa04b14a7d723..90ebcdbad05ca 100644 > --- a/kernel/events/uprobes.c > +++ b/kernel/events/uprobes.c > @@ -28,6 +28,7 @@ > #include > #include > #include > +#include /* check_stable_address_space */ > > #include > > @@ -1260,6 +1261,9 @@ register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new) > * returns NULL in find_active_uprobe_rcu(). > */ > mmap_write_lock(mm); > + if (check_stable_address_space(mm)) > + goto unlock; > + > vma = find_vma(mm, info->vaddr); > if (!vma || !valid_vma(vma, is_register) || > file_inode(vma->vm_file) != uprobe->inode) > diff --git a/kernel/fork.c b/kernel/fork.c > index ded49f18cd95c..20b2120f019ca 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -760,7 +760,8 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm, > mt_set_in_rcu(vmi.mas.tree); > ksm_fork(mm, oldmm); > khugepaged_fork(mm, oldmm); > - } else if (mpnt) { > + } else { > + > /* > * The entire maple tree has already been duplicated. If the > * mmap duplication fails, mark the failure point with > @@ -768,8 +769,18 @@ static __latent_entropy int dup_mmap(struct mm_struct *mm, > * stop releasing VMAs that have not been duplicated after this > * point. > */ > - mas_set_range(&vmi.mas, mpnt->vm_start, mpnt->vm_end - 1); > - mas_store(&vmi.mas, XA_ZERO_ENTRY); > + if (mpnt) { > + mas_set_range(&vmi.mas, mpnt->vm_start, mpnt->vm_end - 1); > + mas_store(&vmi.mas, XA_ZERO_ENTRY); > + /* Avoid OOM iterating a broken tree */ > + set_bit(MMF_OOM_SKIP, &mm->flags); > + } > + /* > + * The mm_struct is going to exit, but the locks will be dropped > + * first. Set the mm_struct as unstable is advisable as it is > + * not fully initialised. > + */ > + set_bit(MMF_UNSTABLE, &mm->flags); > } > out: > mmap_write_unlock(mm); > -- > 2.43.0 > > > -- > maple-tree mailing list > maple-tree@lists.infradead.org > https://lists.infradead.org/mailman/listinfo/maple-tree