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 7ED33C77B7C for ; Tue, 24 Jun 2025 14:29:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 000A26B008A; Tue, 24 Jun 2025 10:29:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F1A4A6B009C; Tue, 24 Jun 2025 10:29:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E309E6B00B5; Tue, 24 Jun 2025 10:29:13 -0400 (EDT) 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 D424B6B008A for ; Tue, 24 Jun 2025 10:29:13 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 9B8245BDCB for ; Tue, 24 Jun 2025 14:29:13 +0000 (UTC) X-FDA: 83590526586.22.624FE66 Received: from mail-qt1-f171.google.com (mail-qt1-f171.google.com [209.85.160.171]) by imf15.hostedemail.com (Postfix) with ESMTP id DF0C9A0011 for ; Tue, 24 Jun 2025 14:29:11 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=dJGTSVnA; spf=pass (imf15.hostedemail.com: domain of surenb@google.com designates 209.85.160.171 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1750775351; a=rsa-sha256; cv=none; b=72L8OuLft505r+Y1HOCFOTQrblUqGaOEAY78USI/CltcX7MPvejCrAlXTWhBFXsN5YcU19 Zv8PBQ29Z/X+Cw5hFzaXvD+g/q22CcDE0knUMO+mCFBK46MMmbYw9/+nTs92AqFscU3EAD YUBCEUaf4Hd743un3Ra84bYWLPCEmMM= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=dJGTSVnA; spf=pass (imf15.hostedemail.com: domain of surenb@google.com designates 209.85.160.171 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=1750775351; 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=hSD2mYfxNDB+dLwDFCI+VBJcy2/hQ3H1VYIqXhGlYFI=; b=vofEVyxmd0UUfHrJ+P5nSU3cKkDI2gjqqIa5A2GV5sUBczyJhzxTTT80QsIzgRpbaloyvN y/WNA4XDaUg7jLanJB+y7X/ZyMfdIVFo8tRQ5jrJJZe9ixm/E4yFNKLNu02WXYEYobfWlX JdMsV3Lbyazr3U/Ai/ZdoWJOsc0xI4o= Received: by mail-qt1-f171.google.com with SMTP id d75a77b69052e-4a5ac8fae12so476601cf.0 for ; Tue, 24 Jun 2025 07:29:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1750775351; x=1751380151; 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=hSD2mYfxNDB+dLwDFCI+VBJcy2/hQ3H1VYIqXhGlYFI=; b=dJGTSVnA2TQoy3VoJkiejoF9c8PB1FGEnROxSzjqXhw2hgzIUM3NTBlr8ikv/K0cjN e2Tb0F0AgLJGdXRygtaIo8xF0dgVETT19uwADy7bRoD5h6h2+jKecsF2+wL4HjneTdVF ofVALVPpZHGEqGIWgWQ3gDXS6bw4ycsHG3tGT7diAi5s/k6BTH3PS/ctpeyeeTkSvws3 QjDXxg0IQfIhVNzZIS4Dw0PslLbHDFLewIIZy7MvjZCuA3yFNv4lTvy6Ldyx+zz3y2SJ TwoMz09zhiU6pE+l4arB4DCoL9SLyA+NRcMbNZrI3h57C2ot6uOdPX/hQIwQQVzbIc19 09lw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1750775351; x=1751380151; 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=hSD2mYfxNDB+dLwDFCI+VBJcy2/hQ3H1VYIqXhGlYFI=; b=jWxTF7DpZQVPzisx1ZfHuFe54aHScLHp8jpj7yqFOPb+H03/BzVHCKji9WdW6H0QY0 jmpdZe7YlEWPEh/mOd2UKjr2/Sluo2vPgrNVNGz5ZxKyEZFne3bm+tihlzcjy4pdRWSD xI999ICT6KWIHu5pAQ0C7pWikkbmhHAjg0fVCf1tv+x4gpBvES1d/naE9fvjmh1qpw4N 9S4B0TybVT0By3zu96yB6FXigPrzLRb+O2VxTfiUkFmqITldrsrDLqQYVLOYPCYz8vws u3KuEkqHw1cb5cpBtK8a1ngy9KMUPEs4kv0BburxvokfEj6Bcx+l+jAe5w3IP2aOkPlv nJxg== X-Forwarded-Encrypted: i=1; AJvYcCWvZcUJxuzhIJpChMytYCVa8nl7UuG2I5XAYrVgujnn+/3A84f0K/W6ZF82jxk8Q/5SGb9wjmdpUQ==@kvack.org X-Gm-Message-State: AOJu0Yz29Z5FjZEyzSvhvpGDx70mUnu9IH0k5tMSclwvnKlpv1N0nuD7 aHWzUMTBMoiGKARBzabekRmiBifXtY7l99Bq8RcVhwsl1WvjdEFwmaJFCE1w7vu0sEWzwXEJ8nU k1eiK+wk4P9isNIBXX/sDBn5GM/WNaeSy9nqFNcXy X-Gm-Gg: ASbGncv3+I4AaSuIsbwbRzYkaYigOefcsTwuhOkr7GjptrvZNbY0Hzb7oMsenucLQPH rrObA5lgMWkjDjxTjxVdfirbse6+ml/fisoVVW8KBxYHg7jJ5pnBcYrwcKMmEX1EZMCGKfqDBTi NOpLYZpyM6UQfpVxNLMDKiSO5ZYw0E/VWOCAvIRvzD5OEwSqwATeSKqse9/znkCcI8ddTYOlRJA w== X-Google-Smtp-Source: AGHT+IEFMI9Z2wOmOoEl6+Gqr+r5GZMQn0E+Ub3SjHdvnw+ICeQc3UNbC7lwI6PyYtFkpoyIcT8V7lNB2r1BCDrj5BM= X-Received: by 2002:a05:622a:344:b0:49c:ffd4:abcc with SMTP id d75a77b69052e-4a7b171567emr3429201cf.27.1750775350498; Tue, 24 Jun 2025 07:29:10 -0700 (PDT) MIME-Version: 1.0 References: <20250624-anon_name_cleanup-v2-0-600075462a11@suse.cz> <20250624-anon_name_cleanup-v2-1-600075462a11@suse.cz> In-Reply-To: From: Suren Baghdasaryan Date: Tue, 24 Jun 2025 07:28:59 -0700 X-Gm-Features: Ac12FXzjUywJN_m_OazPDhVZ_Bpe8eTuhidb-3HMvM1fE8Nqso1HTUWVB-CSsdw Message-ID: Subject: Re: [PATCH v2 1/4] mm, madvise: simplify anon_name handling To: David Hildenbrand Cc: Vlastimil Babka , Andrew Morton , "Liam R. Howlett" , Lorenzo Stoakes , Jann Horn , Mike Rapoport , Michal Hocko , Colin Cross , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: 88tz8s7i7ktnenccq3spo3hx3pbercjx X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: DF0C9A0011 X-Rspam-User: X-HE-Tag: 1750775351-996704 X-HE-Meta: U2FsdGVkX1+cRsjUS4u0HzTzpjvwbSficSB3D5dFnhNsTdAS4tAdj5VddNuGmMng2oASMT4IjuAP258bx+rgIvUtrPxVuXMZxfgf4xI37EZkSF45SGGVU+bFxkw8Lpx9ZjtdpeIBTp2MwWINqd8Yx6I5eSiWYGSIrsFPp2mikDotxH9eFhF+ke5cMggnqgM2gYou3tU8jc6F02dsYwuN6iA5jnXuMMqCAdXAuhKfPNbYXN2qkhvzH1yPAc8zdboYVDxBuiLstnqPi0fEsZd9b3I42jFO1AX62RIrhT7O6Bo69UByUzhbBeB6R13+TEqrQzHejOeHYXhTtVNdcjU4JPI1JDdo0gah4IAr7gZrt+kR/oBZhFhKqCOW3/EtFLpD58owWOdAzcS5YkmHK1gkcQM0ajDDfmSDT+2VSUslxygG4PTgaANRe6842Dosy0q6RZaJJLZQ5U3jveL80tOnK87cR9gCopCd3XPpQbGXM3n6PvbWfEdPvNLH1EgEhse3P3NaguuyrM7q7ui7vziYULs9kbjvmUt9Zv83X47keW6XDvRKGpGvcM7C6nAPcURHVSRu7NmIx5KqFLegivT3+jdGuId/domhWRY1spke0VqfUEKeccz3bUbI1urfGgqgL6e91rCK0OHOGEXsSakAREfjoeWG6SXVaLz6z5FvjTh/JrT2xyUii7P2O0GHA5XnLuWfA7o5iS3+Ns07bEb0q/lEQGEP3UYhUSPty1ALtUwDdXqrplo95Nq53AZp8MtF9+vdF+xHGo53CMUJm7++vuXgJ02eMunT9xwZA2c2qb2AV08fDLvx5jeNmDyomA0wU4aNuqHy472ZG11KUrd6Dbng8txlK9qWABJV4DfHvNpLEFSqlL+Ux5MaXuAABotTIfL9I5enjScq3mGbcZg9qpjZdZi422NmmZiEHv4NEoWS4pR8Pw04BeuW9hR8av0hJ/zKDrOxCns= 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 Tue, Jun 24, 2025 at 6:58=E2=80=AFAM David Hildenbrand wrote: > > On 24.06.25 15:03, Vlastimil Babka wrote: > > Since the introduction in 9a10064f5625 ("mm: add a field to store names > > for private anonymous memory") the code to set anon_name on a vma has > > been using madvise_update_vma() to call replace_anon_vma_name(). Since > > the former is called also by a number of other madvise behaviours that > > do not set a new anon_name, they have been passing the existing > > anon_name of the vma to make replace_vma_anon_name() a no-op. s/replace_vma_anon_name/replace_anon_vma_name > > > > This is rather wasteful as it needs anon_vma_name_eq() to determine the > > no-op situations, and checks for when replace_vma_anon_name() is allowe= d s/replace_vma_anon_name/replace_anon_vma_name > > (the vma is anon/shmem) duplicate the checks already done earlier in > > madvise_vma_behavior(). It has also lead to commit 942341dcc574 ("mm: > > fix use-after-free when anon vma name is used after vma is freed") > > adding anon_name refcount get/put operations exactly to the cases that > > actually do not change anon_name - just so the replace_vma_anon_name() s/replace_vma_anon_name/replace_anon_vma_name > > can keep safely determining it has nothing to do. > > > > The recent madvise cleanups made this suboptimal handling very obvious, > > but happily also allow for an easy fix. madvise_update_vma() now has th= e > > complete information whether it's been called to set a new anon_name, s= o > > stop passing it the existing vma's name and doing the refcount get/put > > in its only caller madvise_vma_behavior(). > > > > In madvise_update_vma() itself, limit calling of replace_anon_vma_name(= ) > > only to cases where we are setting a new name, otherwise we know it's a > > no-op. We can rely solely on the __MADV_SET_ANON_VMA_NAME behaviour and > > can remove the duplicate checks for vma being anon/shmem that were done > > already in madvise_vma_behavior(). > > > > Additionally, by using vma_modify_flags() when not modifying the > > anon_name, avoid explicitly passing the existing vma's anon_name and > > storing a pointer to it in struct madv_behavior or a local variable. > > This prevents the danger of accessing a freed anon_name after vma > > merging, previously fixed by commit 942341dcc574. > > > > Signed-off-by: Vlastimil Babka > > --- > > mm/madvise.c | 37 +++++++++++++------------------------ > > 1 file changed, 13 insertions(+), 24 deletions(-) > > > > diff --git a/mm/madvise.c b/mm/madvise.c > > index 4491bf080f55d6d1aeffb2ff0b8fdd28904af950..fca0e9b3e844ad766e83ac0= 4cc0d7f4099c74005 100644 > > --- a/mm/madvise.c > > +++ b/mm/madvise.c > > @@ -176,25 +176,29 @@ static int replace_anon_vma_name(struct vm_area_s= truct *vma, > > } > > #endif /* CONFIG_ANON_VMA_NAME */ > > /* > > - * Update the vm_flags on region of a vma, splitting it or merging it = as > > - * necessary. Must be called with mmap_lock held for writing; > > - * Caller should ensure anon_name stability by raising its refcount ev= en when > > - * anon_name belongs to a valid vma because this function might free t= hat vma. > > + * Update the vm_flags and/or anon_name on region of a vma, splitting = it or > > + * merging it as necessary. Must be called with mmap_lock held for wri= ting. > > */ > > static int madvise_update_vma(vm_flags_t new_flags, > > struct madvise_behavior *madv_behavior) > > { > > - int error; > > struct vm_area_struct *vma =3D madv_behavior->vma; > > struct madvise_behavior_range *range =3D &madv_behavior->range; > > struct anon_vma_name *anon_name =3D madv_behavior->anon_name; > > + bool set_new_anon_name =3D madv_behavior->behavior =3D=3D __MADV_= SET_ANON_VMA_NAME; > > VMA_ITERATOR(vmi, madv_behavior->mm, range->start); > > > > - if (new_flags =3D=3D vma->vm_flags && anon_vma_name_eq(anon_vma_n= ame(vma), anon_name)) > > + if (new_flags =3D=3D vma->vm_flags && (!set_new_anon_name || > > + anon_vma_name_eq(anon_vma_name(vma), anon_name))) > > return 0; > > > > - vma =3D vma_modify_flags_name(&vmi, madv_behavior->prev, vma, > > + if (set_new_anon_name) > > + vma =3D vma_modify_flags_name(&vmi, madv_behavior->prev, = vma, > > range->start, range->end, new_flags, anon_name); > > + else > > + vma =3D vma_modify_flags(&vmi, madv_behavior->prev, vma, > > + range->start, range->end, new_flags); > > + > > if (IS_ERR(vma)) > > return PTR_ERR(vma); > > > > @@ -203,11 +207,8 @@ static int madvise_update_vma(vm_flags_t new_flags= , > > /* vm_flags is protected by the mmap_lock held in write mode. */ > > vma_start_write(vma); > > vm_flags_reset(vma, new_flags); > > - if (!vma->vm_file || vma_is_anon_shmem(vma)) { > > - error =3D replace_anon_vma_name(vma, anon_name); > > - if (error) > > - return error; > > - } > > + if (set_new_anon_name) > > + return replace_anon_vma_name(vma, anon_name); > > Took me a second to find where this is already checked (-> > madvise_vma_behavior()). > > :) > > Acked-by: David Hildenbrand Reviewed-by: Suren Baghdasaryan > > -- > Cheers, > > David / dhildenb >