From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 319F3C4345F for ; Sat, 13 Apr 2024 21:47:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 82E606B0083; Sat, 13 Apr 2024 17:47:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7DEDF6B0085; Sat, 13 Apr 2024 17:47:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 67E6D6B0087; Sat, 13 Apr 2024 17:47:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 4A32D6B0083 for ; Sat, 13 Apr 2024 17:47:11 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 9E637120167 for ; Sat, 13 Apr 2024 21:47:10 +0000 (UTC) X-FDA: 82005844620.06.4ED4340 Received: from mail-yb1-f173.google.com (mail-yb1-f173.google.com [209.85.219.173]) by imf28.hostedemail.com (Postfix) with ESMTP id DD94EC0006 for ; Sat, 13 Apr 2024 21:47:08 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ljry2Tag; spf=pass (imf28.hostedemail.com: domain of surenb@google.com designates 209.85.219.173 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1713044828; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=mH5yU1UU4J6v01NpTBvFGYdIvUalUK5gzhtQ+379C/I=; b=5fBDEvr3n1cJ6b+1Rveds03kJCIK3mMFUiDK1hN5sjUjb/LOgILXGVNUXbdyZ6VrOAd6/Q LWLhztm9Lme5Ea1CpPN5vUsFseT1/ZisRRx4oMoFztYyhtB8rz/gim1wfYfii4A1AQIsX1 JZwJbtW/B+FpODM6IxblWjO6qhRHrqw= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1713044828; a=rsa-sha256; cv=none; b=5jnP857J37QWxliLqAFtATJFVS5Nt7eZhGbStwvoqOeNloAGibA7CL1Dpt/Rur9FEoBpgo 50rJhImH5QiI2mi9RoixQV4/BO0pb8ytv6yJjfoIBk6aOaUVBpr3JdVD9Dg39swePmm9Ud jjZVztVtss2jG+dOUWukCb/FD4MofcI= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ljry2Tag; spf=pass (imf28.hostedemail.com: domain of surenb@google.com designates 209.85.219.173 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-yb1-f173.google.com with SMTP id 3f1490d57ef6-dcc80d6006aso2002669276.0 for ; Sat, 13 Apr 2024 14:47:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1713044828; x=1713649628; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=mH5yU1UU4J6v01NpTBvFGYdIvUalUK5gzhtQ+379C/I=; b=ljry2Tagb9fau38xqTlgJAOHRoK2aoCFZLz/+Kf92tDq5zc3S3ixcYIZgOdpc+4AX4 gnMbpeAFzdWH5q+DccxgHmqjVRZa0nNOjyW6Z5ukwEVZfOoS4n/y/UJIVmRnJN5vXoiL PDJGtwMTToiGUv2HQucdt1wRWtGwI//ZvqkPjjpYuU+bv/BTD9+0kzvYPIomu2pmSbM2 OFNLuhn8DT60GBed8vG6oqbtZ2LZnX4F5xNcjaFsFcw7vm+ZNYdbD34Ld6qATTUW1qyG EALDZ6hc4IGpQZyN0Z5QAz82p9iz/opVVTq8eR4UPX1kQ5r1848qHXxOhSdC7V0xDBK3 ksaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713044828; x=1713649628; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=mH5yU1UU4J6v01NpTBvFGYdIvUalUK5gzhtQ+379C/I=; b=B0Yhf0oR/0OWRUWFOEtL+deJ3OSPzmIYDhA/lQwm/24hpQhQKnv5TnR32PHDRshHuU SrTDJWTkk5DmyozY2PrpYO6nCZCjk3ws/j9SoRXp7EL2gXnlC0SR27o3X1HCxuKURo2K LBbaLDfBs8+Og6XtxlXRmaoFE6fw5PyMt+n06ReuSiGdLpUWW02avmG6a+M5NnXB4ovA aiWPajlKnbdMjYuAOH+Ba0AbHMAadfYSWU8YO367gHuXss8tNhEc0tEgpTbiD9ZPKxF9 z94dK88Wo8Xds6VuvZlcK4prlhPFGcMO3WYUZY4esuPh2ZGdntkdGht49AoT6mAXZ21r Uevg== X-Forwarded-Encrypted: i=1; AJvYcCUAuU9P8vCYGfcPgcmNy9dK/zvFj8nIvEyavx+7FIU0JNnIoGdmPef4iU8uov8dIRqpomGHUBARNPzZHfB2eUY3AwA= X-Gm-Message-State: AOJu0Yy64FgN3I6nkJ9XWGXLujJOwIzaMpbWtzp2GcLYga9c2y6ruPAO 63i9vGCJI1SvcNGHnp7wFvyF5DCYz5QJkR64UiqgLlwW6Q+UBCtUiIFdJ9kIAphkp0xXxbUHTom ahw9YFBqWwSnxzviWDJ/85o2N0gos7m8zb6jG X-Google-Smtp-Source: AGHT+IG/GjntQoa0HI5RhiEC8sNuWYHvstFUNYvTBidST9LMhJw1ALiwKoLNTqJRuae8Ce5p/NgKQrXV8iok42LLJt8= X-Received: by 2002:a25:15c5:0:b0:dca:e4fd:b6d6 with SMTP id 188-20020a2515c5000000b00dcae4fdb6d6mr5038035ybv.61.1713044827769; Sat, 13 Apr 2024 14:47:07 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Suren Baghdasaryan Date: Sat, 13 Apr 2024 14:46:56 -0700 Message-ID: Subject: Re: [PATCH] mm: Always sanity check anon_vma first for per-vma locks To: Matthew Wilcox Cc: Peter Xu , "Liam R. Howlett" , LKML , linux-mm@kvack.org, Andrew Morton , Lokesh Gidra , Alistair Popple Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: DD94EC0006 X-Rspam-User: X-Stat-Signature: cwbnxokfowa7ipd31wqxskhjsr3hihpu X-Rspamd-Server: rspam03 X-HE-Tag: 1713044828-631766 X-HE-Meta: U2FsdGVkX185FlWFM2vydwWN6X29902oaPjVuSK70w4XZJnMNHnXkfFc/rjMiGAleeedT9FZvajrJs9kAy59bJ5La0oXLevviLO8C3oSqnnaqtuCxC6G0mrzi2bfCPhmFMyhynOKeDUw32R6gyL2IQRprLGjK95C95Zx+8qyGJKecryaYDIZNMc+fHcY3HIwwBRTjOc+SUm2PpM3yOFqtfszrNpfaZ9A4EBhgIno2vzQuw7X0XPMQrRRw8D9BnAmud6jW24084wSyyzVMN5q0sFk3opoBB65eP4LNW3aCQ4Xqq0Gmk7sW0Z90CLVmAlRP2piYcuI15LqWR/CLSL1OgW2Y5OG2W4rIjOxVhqiNIXdXvgpwCt7zKLgDXpvsbBsEblZg/9pC2TdU8Uf404nrhkhhYZo81QalGcTvMkI91a/gxqBxWEqZ+Y0MDNXHy0EVZecZyQoo37idK+sZLhv7RNqGAPOQUOuPq+MiW1+dO4+3QrMfNpE6KWfJwfTZ2ZJCsvS6GokrqUgnWQvrRlxPwRC8sqfXsVc3QwAmNEea9deLSteshTXt5xVJ08lBqLU29Zr87kuXrLdRRoH9MahisjhIeJAjQvvyiyoXYsSoQb8nsjZhXDjIe7wzrz2gyJjXx/9T+v5RRpxzRUvmAVyJrfmRuWrTSl1hYk3bNgWh9B0FJVjk1Nbk8AZMMayeiAGYKXDxaOzXBstgXf7gO873uCGs/vc+PHNguKmu+kTTaZ9B+FAq13MaQZnj0OLhabxeoDK68cs/ubOP3ZFKx3IK+8dElkv80C89OjdGpFZPX4Wi4pt2zHyVGBbFE/HB6vlV4LpDNP28mNXuWl2UbjpzXAgLzGA8tRDZnd/Z+2mEMNd+P1O81/mU/EEgwovRmy+IdoVFKKS5pc3oXXxMWHWVl8x2GztYXRk4hsez5i6ZqHOooMQECH7usJbb+0S13tcVdcvXvb0aYKifDJAP1H mLiUnJP/ AEd5lOGKpug3wb7aZvwyjDF7/CNPZjmNBId+BsH6ZwmqrIguo3VhQgUpHJg7cHo+op+S1CzPhS+4ATZYCfvNqr3MMyFKk+WgjP8n/THs+aIcRMGhiYNhOUg8fcs9ncarc6Ga3TG07wkWxaPqxgPXl1+7TVmf65XmD5J69mnypkjgLuLPDVZ4Lxl385tbCFv1W1oKnaX6ixNl9FMFSr7PBTi1aLhFYKQejDrOzvFmBQKimmNwiqHTOkhQ3qUUBOWDPu/f8U4WRDbZEblfc66iNUk7YICgcMGOfIiuNxGDjsp56oSQTA3zQLe9JA3Ct2A0Ah3yqeVt9FITbpyE= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Apr 12, 2024 at 8:31=E2=80=AFAM Matthew Wilcox wrote: > > On Fri, Apr 12, 2024 at 04:19:27PM +0100, Matthew Wilcox wrote: > > On Fri, Apr 12, 2024 at 09:53:29AM -0500, Suren Baghdasaryan wrote: > > > Unless vmf_anon_prepare() already explains why vma->anon_vma poses a > > > problem for per-vma locks, we should have an explanation there. This > > > comment would serve that purpose IMO. > > > > I'll do you one better; here's some nice kernel-doc for > > vmd_anon_prepare(): > > And here's a followup patch to fix some minor issues in uffd. > > - Rename lock_vma() to uffd_lock_vma() because it really is uffd > specific. I'm planning to expand the scope of lock_vma() and reuse it for /proc/pid/maps reading under per-VMA locks. No objection to renaming it for now but I'll likely rename it back later once it's used in more places. > - Remove comment referencing unlock_vma() which doesn't exist. > - Fix the comment about lock_vma_under_rcu() which I just made > incorrect. > > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c > index f6267afe65d1..a32171158c38 100644 > --- a/mm/userfaultfd.c > +++ b/mm/userfaultfd.c > @@ -56,17 +56,16 @@ struct vm_area_struct *find_vma_and_prepare_anon(stru= ct mm_struct *mm, > > #ifdef CONFIG_PER_VMA_LOCK > /* > - * lock_vma() - Lookup and lock vma corresponding to @address. > + * uffd_lock_vma() - Lookup and lock vma corresponding to @address. > * @mm: mm to search vma in. > * @address: address that the vma should contain. > * > - * Should be called without holding mmap_lock. vma should be unlocked af= ter use > - * with unlock_vma(). > + * Should be called without holding mmap_lock. > * > * Return: A locked vma containing @address, -ENOENT if no vma is found,= or > * -ENOMEM if anon_vma couldn't be allocated. > */ > -static struct vm_area_struct *lock_vma(struct mm_struct *mm, > +static struct vm_area_struct *uffd_lock_vma(struct mm_struct *mm, > unsigned long address) > { > struct vm_area_struct *vma; > @@ -74,9 +73,8 @@ static struct vm_area_struct *lock_vma(struct mm_struct= *mm, > vma =3D lock_vma_under_rcu(mm, address); > if (vma) { > /* > - * lock_vma_under_rcu() only checks anon_vma for private > - * anonymous mappings. But we need to ensure it is assign= ed in > - * private file-backed vmas as well. > + * We know we're going to need to use anon_vma, so check > + * that early. > */ > if (!(vma->vm_flags & VM_SHARED) && unlikely(!vma->anon_v= ma)) > vma_end_read(vma); > @@ -107,7 +105,7 @@ static struct vm_area_struct *uffd_mfill_lock(struct = mm_struct *dst_mm, > { > struct vm_area_struct *dst_vma; > > - dst_vma =3D lock_vma(dst_mm, dst_start); > + dst_vma =3D uffd_lock_vma(dst_mm, dst_start); > if (IS_ERR(dst_vma) || validate_dst_vma(dst_vma, dst_start + len)= ) > return dst_vma; > > @@ -1437,7 +1435,7 @@ static int uffd_move_lock(struct mm_struct *mm, > struct vm_area_struct *vma; > int err; > > - vma =3D lock_vma(mm, dst_start); > + vma =3D uffd_lock_vma(mm, dst_start); > if (IS_ERR(vma)) > return PTR_ERR(vma); > > @@ -1452,7 +1450,7 @@ static int uffd_move_lock(struct mm_struct *mm, > } > > /* > - * Using lock_vma() to get src_vma can lead to following deadlock= : > + * Using uffd_lock_vma() to get src_vma can lead to following dea= dlock: > * > * Thread1 Thread2 > * ------- ------- > @@ -1474,7 +1472,7 @@ static int uffd_move_lock(struct mm_struct *mm, > err =3D find_vmas_mm_locked(mm, dst_start, src_start, dst_vmap, s= rc_vmap); > if (!err) { > /* > - * See comment in lock_vma() as to why not using > + * See comment in uffd_lock_vma() as to why not using > * vma_start_read() here. > */ > down_read(&(*dst_vmap)->vm_lock->lock);