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 715A6E7719C for ; Fri, 10 Jan 2025 20:34:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 055696B00E4; Fri, 10 Jan 2025 15:34:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0068C6B00E5; Fri, 10 Jan 2025 15:34:56 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DE77D6B00E6; Fri, 10 Jan 2025 15:34:56 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id B720F6B00E4 for ; Fri, 10 Jan 2025 15:34:56 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 6E812120C5C for ; Fri, 10 Jan 2025 20:34:56 +0000 (UTC) X-FDA: 82992696192.26.DA3671D Received: from mail-qt1-f177.google.com (mail-qt1-f177.google.com [209.85.160.177]) by imf13.hostedemail.com (Postfix) with ESMTP id 884DC2000A for ; Fri, 10 Jan 2025 20:34:54 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=B7XKuZxh; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf13.hostedemail.com: domain of surenb@google.com designates 209.85.160.177 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736541294; a=rsa-sha256; cv=none; b=pAuvjPwQA+R+zUQ/+A4sz3hy9atql24vi/L5yDOlLfifXrEApapYVDIVtkq5ckyD4aRpwg Ffzv9UKLR6YqSQS/xvzeP3itkREQjqxFpn4lTrZzUXCow8VfB8opZl5Cbwazcr5KYk1u4b GLGPHdiWN5bOa29gBit8ipzQdd0mS48= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=B7XKuZxh; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf13.hostedemail.com: domain of surenb@google.com designates 209.85.160.177 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736541294; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to: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=vfc2COOnyMnzAA//UDIcuA90pEQo1Gwzn9YqKTubUlo=; b=beQPOTSUFEL44jvrS44h74tk8jmDnCKjkaUazYvrK23EigAvw2tXq7QhCXENI/aTgScc0Y yFTsQlKRPneH0zLNqC2vxhboo2LtPiLQwnrI3yRCdavF8mXa6r8FrK19rRLLBINBdNBpab oD8WZTF/MA3gct26P4dlXcjfTSXMhXg= Received: by mail-qt1-f177.google.com with SMTP id d75a77b69052e-4678c9310afso43121cf.1 for ; Fri, 10 Jan 2025 12:34:54 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1736541294; x=1737146094; darn=kvack.org; h=content-transfer-encoding:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=vfc2COOnyMnzAA//UDIcuA90pEQo1Gwzn9YqKTubUlo=; b=B7XKuZxhSWGmK+0iqF9aApugflqyL6ETKS7l8fKaPwDBS1Ft1jJYguGrqrcTtev4R5 k5u0KbccBE/iQfLDG9T2I79zLjoY5Qftfbko9xtEKoD1I0FDy+sLuS5fglztvp9FPtfI GNS3wkzOU2W6AtBcmtrPByOEiMS/GTOrKkEHo0EutsvhJqqH8zx9+7dQH0Pps8XH7i/f BfOcItVA6q2xmdlGx/mPACqlyLyINvm/RdZonjGIpwf03adVgK5VhidFsItu7rX/+Kj4 ae8eXOogA5jHQKyWjcBqSTONk92euwha4pp4ql0+CFKxEsNBe/nrpNctWT3EFCqkImmf Kgvg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736541294; x=1737146094; h=content-transfer-encoding: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=vfc2COOnyMnzAA//UDIcuA90pEQo1Gwzn9YqKTubUlo=; b=CRKJWHHNzapJymq7Y14R4Ym6jbGGtNSApBaSnK8yGeaG7Bo2ZDC+cpeAh1THflwLIr vtPNLsssN5sN2+E9Fett3EhlGsmV6WuxEGy3jQL/xJDsTNlammhFLkGAMzModBslhSKs pefPFAUi4XFPwWnNs4LQkfgBEXnbqZIJfYKkxProva59yBaH2LcJ6Ut4ITPT0LZQffqt VxwtQi3jx9B+wQrjJUUsixe8iQ6pkWNpNnUYkAAxbt9axw7bi2EtkpO8DKHhooha8quB fpBFZWG9A7SbL1iov3CDZYh4Ovi4NFCXXU9nXuJr5fOAf0gS58vzXljj79w2murSHz49 bDKA== X-Forwarded-Encrypted: i=1; AJvYcCW+3nR24d8q3tXNBhvHtokBPdnTlD11Waahtz2buueBA+KYZC2dXaBjtxW09t0BIWda7D3ZREw4KA==@kvack.org X-Gm-Message-State: AOJu0YwJ3m8GzPcgY0aC3Gl9HmzREKTq68UeAZ+KOdemGoxRZr5SVuwF cLlwcRwQwkRj9YV2nPMwEu0aEYfcVj9/8+IE4UKHO5I2xC5FxlXeTVIiTj8BL/gkEvvJDVy7b8H zjr8fPIolVPs5CSawYiGJRp8uvhP3xJYWmNSu X-Gm-Gg: ASbGnct2wqrEg++jdzpw7gAw8wT9AeqNL4A0Lz6b2mQDyiyWNMCTtTYqtUn6uHm3ls7 mFB5HQMmighj0mAQqdtcNzwWXsUhjFXzerUSJIg== X-Google-Smtp-Source: AGHT+IGtqqI3TXLMiG6opM5UmlrveNYcio+i6/z1dMyGaR37LhhpmrVEQZYom8LRjv5qs0CrHbDG/AFw9H1xrHbBadc= X-Received: by 2002:a05:622a:1b91:b0:466:8c7c:3663 with SMTP id d75a77b69052e-46c89d3bcafmr4959821cf.5.1736541293295; Fri, 10 Jan 2025 12:34:53 -0800 (PST) MIME-Version: 1.0 References: <20250109023025.2242447-1-surenb@google.com> <20250109023025.2242447-16-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Fri, 10 Jan 2025 12:34:42 -0800 X-Gm-Features: AbW1kvZCoB1h2hewskYj_-I0KU7cVd1ouvesflJPVIWesqxOa3OfLpcDrKUzo0A Message-ID: Subject: Re: [PATCH v8 15/16] mm: make vma cache SLAB_TYPESAFE_BY_RCU To: "Liam R. Howlett" , Suren Baghdasaryan , akpm@linux-foundation.org, peterz@infradead.org, willy@infradead.org, lorenzo.stoakes@oracle.com, mhocko@suse.com, vbabka@suse.cz, hannes@cmpxchg.org, mjguzik@gmail.com, oliver.sang@intel.com, mgorman@techsingularity.net, david@redhat.com, peterx@redhat.com, oleg@redhat.com, dave@stgolabs.net, paulmck@kernel.org, brauner@kernel.org, dhowells@redhat.com, hdanton@sina.com, hughd@google.com, lokeshgidra@google.com, minchan@google.com, jannh@google.com, shakeel.butt@linux.dev, souravpanda@google.com, pasha.tatashin@soleen.com, klarasmodin@gmail.com, richard.weiyang@gmail.com, corbet@lwn.net, linux-doc@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@android.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 884DC2000A X-Stat-Signature: wc6wxzj6w5ufq9ynjds1zwz91rcaxyzm X-HE-Tag: 1736541294-504348 X-HE-Meta: U2FsdGVkX1+eXDp0D5Juty7JgMsrd9fn95ac/M6UJTGHEFs2ICMoasqEQzMTGTd/PQseajQXRnMoP/M6Z8POtWed53I+7j2V3aSZugiGh060o0/MMAVD+p5iqSGh+wfn14DG5rOM8pGu4ulrg3FeI/6KmRDWxU2xvxg5l4T3il01VrmqJxckcgrg1ho+yZnPUJZujouhrlooDUDxhbnkQkLEkozZCrQXIctuZ3hKTuI0sjX8CMN2VdA3gJuA4VenJSJt/BbCiYHK/Hf/hCxr5VADHyhsbSWAsA8ib/18v2ZfBjdDcwAWdnKx7L57Fl/fDo1H0KfNLXSYGXP6AFIcn5UzTWu3+heq5TpGDifwHcwABOEcVbBVIrffnX0j5vqijzG8jOSvDZGDZbysYmiWTH+a4eZiWD8MVHRYTb3/tFKyNoI3rZurxIz6lZxXlBWtWuzmPjVLaV0Sj9gTmF2nD69p1WA2wV6/YJMRejAf3cTMgu0D+XzaSJO3kGwbSj09wiEuWcv2OmN5gC9Uin98dyZh2G8NdECa9FcO04mBc0ADosMIaVBBckV+J7diZHi0S0aFqR2re+K9xD4ipm6xkD1dUaZOdUHkW4SGtv+zwOGPySeGBDLzZ66WAj34pdKFVlEMsqeIexpreL/2viRVP2VqotUSnQqRjzWyzW1Zr4/dJ1Xs4X1zFl3FAW5z2dLpj9kDSsa+FN4h+rGbjyiHMW4Iuoh6jePjDTSK/DnC5B/QYpnjnGD2JO7hcCTaPC9uQIqrYRkO80LCouBfN0k9poM6FsjjxM9Dtu0CQ+6hS2flrh9fbvdhj1saUMWSZt+B97xsFjMEwNoSvlAq6yw/fGnaa4bKZ4ML9uiO9aQ1LI2LxM+prRTA8oLbNSfYgXAGr4qnwPm1bJYDBajGDIppTw8HopDj018puhsPs7HWkAhqQ1EoU6TQ8Z2qzq1y/iW0iye7U2HUcZW7LSUhIpB v6rw/Edn yTC2J1n6KOG/Skgc1Bz0W3ePD/QowRH8eD3WRgjISYsLGWRctSgUg4GEPdMPE5KcBycfFTunAeMBHkTwfn/1f7EKt/Z8XP0bPuMnDnOA+AI8x4M8OgPulLF/0EldP1HPdIvr5ikbfQu/5OwUisls3TKXI2+E8Sdtqn8tvfMDuONcxu9wN0Aq4Js4XKyZIW83WM/yMuWB4aKIdlC3GjNf5k6X5O0JEfQKXw5nB43ye5fsuocwpS0QT6rUZ4nKcTmu+TpfaHBXBivLmPWlyd1JFZYhRmYGDka2LBHPRDO3ORQy05a5xG9n1lktdbXVZ9U5VHbaWiKitft5nwDbIpydNUCVVe5GK/8I7sKU6n3OMa3RNL2gnoJRcJDd5tFaMq/pzADRmkWrDVuiTuU0uBCAzr5dh6Q== 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, Jan 10, 2025 at 11:46=E2=80=AFAM 'Liam R. Howlett' via kernel-team wrote: > > * Suren Baghdasaryan [250110 14:08]: > > On Fri, Jan 10, 2025 at 9:48=E2=80=AFAM Liam R. Howlett wrote: > > > > > > * Suren Baghdasaryan [250108 21:31]: > > > > To enable SLAB_TYPESAFE_BY_RCU for vma cache we need to ensure that > > > > object reuse before RCU grace period is over will be detected by > > > > lock_vma_under_rcu(). > > > > Current checks are sufficient as long as vma is detached before it = is > > > > freed. The only place this is not currently happening is in exit_mm= ap(). > > > > Add the missing vma_mark_detached() in exit_mmap(). > > > > Another issue which might trick lock_vma_under_rcu() during vma reu= se > > > > is vm_area_dup(), which copies the entire content of the vma into a= new > > > > one, overriding new vma's vm_refcnt and temporarily making it appea= r as > > > > attached. This might trick a racing lock_vma_under_rcu() to operate= on > > > > a reused vma if it found the vma before it got reused. To prevent t= his > > > > situation, we should ensure that vm_refcnt stays at detached state = (0) > > > > when it is copied and advances to attached state only after it is a= dded > > > > into the vma tree. Introduce vma_copy() which preserves new vma's > > > > vm_refcnt and use it in vm_area_dup(). Since all vmas are in detach= ed > > > > state with no current readers when they are freed, lock_vma_under_r= cu() > > > > will not be able to take vm_refcnt after vma got detached even if v= ma > > > > is reused. > > > > Finally, make vm_area_cachep SLAB_TYPESAFE_BY_RCU. This will facili= tate > > > > vm_area_struct reuse and will minimize the number of call_rcu() cal= ls. > > > > > > > > Signed-off-by: Suren Baghdasaryan > > > > --- > > > > include/linux/mm.h | 2 - > > > > include/linux/mm_types.h | 10 +++-- > > > > include/linux/slab.h | 6 --- > > > > kernel/fork.c | 72 ++++++++++++++++++++--------= ---- > > > > mm/mmap.c | 3 +- > > > > mm/vma.c | 11 ++--- > > > > mm/vma.h | 2 +- > > > > tools/testing/vma/vma_internal.h | 7 +--- > > > > 8 files changed, 59 insertions(+), 54 deletions(-) > > > > > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > > > index 1d6b1563b956..a674558e4c05 100644 > > > > --- a/include/linux/mm.h > > > > +++ b/include/linux/mm.h > > > > @@ -258,8 +258,6 @@ void setup_initial_init_mm(void *start_code, vo= id *end_code, > > > > struct vm_area_struct *vm_area_alloc(struct mm_struct *); > > > > struct vm_area_struct *vm_area_dup(struct vm_area_struct *); > > > > void vm_area_free(struct vm_area_struct *); > > > > -/* Use only if VMA has no other users */ > > > > -void __vm_area_free(struct vm_area_struct *vma); > > > > > > > > #ifndef CONFIG_MMU > > > > extern struct rb_root nommu_region_tree; > > > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > > > > index 2d83d79d1899..93bfcd0c1fde 100644 > > > > --- a/include/linux/mm_types.h > > > > +++ b/include/linux/mm_types.h > > > > @@ -582,6 +582,12 @@ static inline void *folio_get_private(struct f= olio *folio) > > > > > > > > typedef unsigned long vm_flags_t; > > > > > > > > +/* > > > > + * freeptr_t represents a SLUB freelist pointer, which might be en= coded > > > > + * and not dereferenceable if CONFIG_SLAB_FREELIST_HARDENED is ena= bled. > > > > + */ > > > > +typedef struct { unsigned long v; } freeptr_t; > > > > + > > > > /* > > > > * A region containing a mapping of a non-memory backed file under= NOMMU > > > > * conditions. These are held in a global tree and are pinned by = the VMAs that > > > > @@ -695,9 +701,7 @@ struct vm_area_struct { > > > > unsigned long vm_start; > > > > unsigned long vm_end; > > > > }; > > > > -#ifdef CONFIG_PER_VMA_LOCK > > > > - struct rcu_head vm_rcu; /* Used for deferred freeing.= */ > > > > -#endif > > > > + freeptr_t vm_freeptr; /* Pointer used by SLAB_TYPESAF= E_BY_RCU */ > > > > }; > > > > > > > > /* > > > > diff --git a/include/linux/slab.h b/include/linux/slab.h > > > > index 10a971c2bde3..681b685b6c4e 100644 > > > > --- a/include/linux/slab.h > > > > +++ b/include/linux/slab.h > > > > @@ -234,12 +234,6 @@ enum _slab_flag_bits { > > > > #define SLAB_NO_OBJ_EXT __SLAB_FLAG_UNUSED > > > > #endif > > > > > > > > -/* > > > > - * freeptr_t represents a SLUB freelist pointer, which might be en= coded > > > > - * and not dereferenceable if CONFIG_SLAB_FREELIST_HARDENED is ena= bled. > > > > - */ > > > > -typedef struct { unsigned long v; } freeptr_t; > > > > - > > > > /* > > > > * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests. > > > > * > > > > diff --git a/kernel/fork.c b/kernel/fork.c > > > > index 9d9275783cf8..770b973a099c 100644 > > > > --- a/kernel/fork.c > > > > +++ b/kernel/fork.c > > > > @@ -449,6 +449,41 @@ struct vm_area_struct *vm_area_alloc(struct mm= _struct *mm) > > > > return vma; > > > > } > > > > > > > > > > There exists a copy_vma() which copies the vma to a new area in the m= m > > > in rmap. Naming this vma_copy() is confusing :) > > > > > > It might be better to just put this code in the vm_area_dup() or call= it > > > __vm_area_dup(), or __vma_dup() ? > > > > Hmm. It's not really duplicating a vma but copying its content (no > > allocation). How about __vm_area_copy() to indicate it is copying > > vm_area_struct content? > > > > > > > > > +static void vma_copy(const struct vm_area_struct *src, struct vm_a= rea_struct *dest) > > > > +{ > > > > + dest->vm_mm =3D src->vm_mm; > > > > + dest->vm_ops =3D src->vm_ops; > > > > + dest->vm_start =3D src->vm_start; > > > > + dest->vm_end =3D src->vm_end; > > > > + dest->anon_vma =3D src->anon_vma; > > > > + dest->vm_pgoff =3D src->vm_pgoff; > > > > + dest->vm_file =3D src->vm_file; > > > > + dest->vm_private_data =3D src->vm_private_data; > > > > + vm_flags_init(dest, src->vm_flags); > > > > + memcpy(&dest->vm_page_prot, &src->vm_page_prot, > > > > + sizeof(dest->vm_page_prot)); > > > > + /* > > > > + * src->shared.rb may be modified concurrently, but the clone > > > > + * will be reinitialized. > > > > + */ > > > > + data_race(memcpy(&dest->shared, &src->shared, sizeof(dest->sh= ared))); > > > > + memcpy(&dest->vm_userfaultfd_ctx, &src->vm_userfaultfd_ctx, > > > > + sizeof(dest->vm_userfaultfd_ctx)); > > > > +#ifdef CONFIG_ANON_VMA_NAME > > > > + dest->anon_name =3D src->anon_name; > > > > +#endif > > > > +#ifdef CONFIG_SWAP > > > > + memcpy(&dest->swap_readahead_info, &src->swap_readahead_info, > > > > + sizeof(dest->swap_readahead_info)); > > > > +#endif > > > > +#ifndef CONFIG_MMU > > > > + dest->vm_region =3D src->vm_region; > > > > +#endif > > > > +#ifdef CONFIG_NUMA > > > > + dest->vm_policy =3D src->vm_policy; > > > > +#endif > > > > +} > > > > + > > > > struct vm_area_struct *vm_area_dup(struct vm_area_struct *orig) > > > > { > > > > struct vm_area_struct *new =3D kmem_cache_alloc(vm_area_cache= p, GFP_KERNEL); > > > > @@ -458,11 +493,7 @@ struct vm_area_struct *vm_area_dup(struct vm_a= rea_struct *orig) > > > > > > > > ASSERT_EXCLUSIVE_WRITER(orig->vm_flags); > > > > ASSERT_EXCLUSIVE_WRITER(orig->vm_file); > > > > - /* > > > > - * orig->shared.rb may be modified concurrently, but the clon= e > > > > - * will be reinitialized. > > > > - */ > > > > - data_race(memcpy(new, orig, sizeof(*new))); > > > > + vma_copy(orig, new); > > > > vma_lock_init(new, true); > > > > > > I think this suffers from a race still? > > > > > > That is, we can still race between vm_lock_seq =3D=3D mm_lock_seq and= the > > > lock acquire, where a free and reuse happens. In the even that the > > > reader is caught between the sequence and lock taking, the > > > vma->vmlock_dep_map may not be replaced and it could see the old lock > > > (or zero?) and things go bad: > > > > > > It could try to take vmlock_dep_map =3D=3D 0 in read mode. > > > > > > It can take the old lock, detect the refcnt is wrong and release the = new > > > lock. > > > > I don't think this race can happen. Notice a call to > > vma_assert_detached() inside vm_area_free(), so before vma is freed > > and possibly reused, it has to be detached. vma_mark_detached() > > ensures that there are no current or future readers by executing the > > __vma_enter_locked() + __vma_exit_locked() sequence if vm_refcnt is > > not already at 0. Once __vma_exit_locked() is done, vm_refcnt is at 0 > > and any new reader will be rejected on > > __refcount_inc_not_zero_limited(), before even checking vm_lock_seq =3D= =3D > > mm_lock_seq. > > Isn't the vm_lock_seq check before the ref count in vma_start_read()? > > From patch 11/16: > > @@ -720,13 +752,19 @@ static inline bool vma_start_read(struct vm_area_st= ruct *vma) > if (READ_ONCE(vma->vm_lock_seq) =3D=3D READ_ONCE(vma->vm_mm->mm_l= ock_seq.sequence)) > return false; > > - if (unlikely(down_read_trylock(&vma->vm_lock.lock) =3D=3D 0)) > + /* > + * If VMA_LOCK_OFFSET is set, __refcount_inc_not_zero_limited() w= ill fail > + * because VMA_REF_LIMIT is less than VMA_LOCK_OFFSET. > + */ > + if (unlikely(!__refcount_inc_not_zero_limited(&vma->vm_refcnt, &o= ldcnt, > + VMA_REF_LIMIT))) > return false; > > + rwsem_acquire_read(&vma->vmlock_dep_map, 0, 1, _RET_IP_); > > > > >Even if a reader tries to sneak in between > > __vma_enter_locked() and __vma_exit_locked() calls, > > __refcount_inc_not_zero_limited() will reject it because > > VMA_LOCK_OFFSET is set and VMA_REF_LIMIT will be violated. > > IOW, when VMA is freed, it's guaranteed to be detached with no current > > or future readers, therefore "race between vm_lock_seq =3D=3D mm_lock_s= eq > > and the lock acquire, where a free and reuse happens" should not be > > possible. > > > > Did I understand your concern correctly and does my explanation make > > sense to you? > > It is close to what Vlastimil said before. > > Here is the sequence for a NULL dereference, refcnt value is not needed: > > A: B: C: > lock_vma_under_rcu() > vma =3D mas_walk() > vma_start_read() > vm_lock_seq =3D=3D mm->mm_lock_seq.sequence > > vma_start_write > vma detached and freed At this point B makes vm_refcnt=3D=3D0. > > > vm_area_dup() > - vma reallocated > - zero vma vm_refcnt here is still 0. > Here before calling rwsem_acquire_read(), the reader has to successfully do __refcount_inc_not_zero_limited() and that will fail because vm_refcnt=3D=3D0. So rwsem_acquire_read(NULL) will not be called. What am I missing? > rwsem_acquire_read(NULL) > > > Here is a sequence for unlocking the new lock while locking the old one. > The refcnt failure detects the detached state but does not protect > against the wrong lock use: > A: B: C: > lock_vma_under_rcu() > vma =3D mas_walk() > vma_start_read() > vm_lock_seq =3D=3D mm->mm_lock_seq.sequence > > vma_start_write > vma detached and freed > > vm_area_dup() > - vma reallocated vm_refcnt is still 0 after reallocation. > > rwsem_acquire_read(old lock) > __refcount_inc_not_zero_limited() fails Are we looking at the same code (https://lore.kernel.org/all/20250109023025.2242447-12-surenb@google.com/)? The sequence should be reversed here like this: if (!__refcount_inc_not_zero_limited()) return false; rwsem_acquire_read(old lock) and because __refcount_inc_not_zero_limited() fails, rwsem_acquire_read(old lock) should never be called. > > vma_init_lock(); > > rwsem_release(new lock) The reader will not call rwsem_release(new lock) because it failed to acquire the lock. > > I don't think avoiding the copy of the ref count from the old vma is > enough to stop these races? > > Thanks, > Liam > > To unsubscribe from this group and stop receiving emails from it, send an= email to kernel-team+unsubscribe@android.com. >