Message ID | 20221228194249.170354-1-surenb@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4e01:0:0:0:0:0 with SMTP id p1csp2057806wrt; Wed, 28 Dec 2022 11:45:52 -0800 (PST) X-Google-Smtp-Source: AMrXdXsPdKQ+yW6qF3hRE+MyVxfEyXzXuRO+Iwd8Lav3Y2Hzozm5aJ/N9zevEZVWbmKxxzJOe64y X-Received: by 2002:aa7:9382:0:b0:56b:da42:95c7 with SMTP id t2-20020aa79382000000b0056bda4295c7mr27996708pfe.30.1672256752488; Wed, 28 Dec 2022 11:45:52 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1672256752; cv=none; d=google.com; s=arc-20160816; b=y5fZRxYVIhw1vG/aVamgnAX5DqLHPA/XZUneiaQwobSN+kJKuRGqM/NgCLoTlYGrLk liRgvMy4ZKYtJo2fKLqSBiO7c1AlJCmGa/VzcA/4omtEP0N52tX5Q4VEeyDK0Z/si6m4 Vr+m42Jyixbii+XkdhsiWJTFAn3FlvO2WDMgEI/EXLHPHRhKSfeLJrZDY0IQ86MLzdGq ppK62tjnXOSeebdhiljcrQc/fXQdiisBL4SiQwF5oWtr80ZrbBfyO6TJbWSrNhL5VDgs YpEwcTdIhpOPHKXhlPGIK3YGqZc0PqKHXlkOPmuKvdx7vc7rSqBRsrptXgSK1R8EECzu RE2g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:mime-version:date :dkim-signature; bh=h66ZlJIW0sPnpY7cABzav3ob2j9zOJpX5LkgC0xA0/8=; b=HSE01wlsv9+ZmdcasC4IZ0lQtdpuset9pOLKz57D92yQsjWopcT8DyvstqCk+n8IK8 pIkcNepJ8IYKoJCGag9RdiSjkCrl8wukMl0nZcstPooH1KiSvj/SaLbLkpjT7MVMusg7 tfywSjNS/I6bHHtzMD/yUhnRXYK30dkyi7Z5avRRCXLMS9WaXp80extoEXiBbpxEjyBf iU6PZImAa+otmxHrHd0yXukwftKKoJ75OSIcxfNOrJs0eN+oW4FOGOptJiSAguIxgb4P abwFDC87ZO3qBo+/qNksEEeShkG3UGym6wjpc6bOBx8bYeWEkXHmgNq0dg0C/TcTVU9K 3qXw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=ZmaFVF7j; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id h3-20020a056a00218300b0056ee105315fsi18386860pfi.178.2022.12.28.11.45.36; Wed, 28 Dec 2022 11:45:52 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=ZmaFVF7j; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230080AbiL1Tm6 (ORCPT <rfc822;eddaouddi.ayoub@gmail.com> + 99 others); Wed, 28 Dec 2022 14:42:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49974 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230078AbiL1Tm4 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 28 Dec 2022 14:42:56 -0500 Received: from mail-yw1-x1149.google.com (mail-yw1-x1149.google.com [IPv6:2607:f8b0:4864:20::1149]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2A42B1165 for <linux-kernel@vger.kernel.org>; Wed, 28 Dec 2022 11:42:55 -0800 (PST) Received: by mail-yw1-x1149.google.com with SMTP id 00721157ae682-434eb7c6fa5so184197287b3.14 for <linux-kernel@vger.kernel.org>; Wed, 28 Dec 2022 11:42:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=h66ZlJIW0sPnpY7cABzav3ob2j9zOJpX5LkgC0xA0/8=; b=ZmaFVF7jonGinSfOfqJLrvexRkxJcgPqsnKMM2BWGIh/BVHh7PqXo08JdBFt3JvzDT MtFcfUdyqDiaFCLySq0/hKOSrDV1GcyYBXtmNsEtbbs0AD5T4KILU972sTh9LBVzvylG tQ34yld8u16qX7XcANjKxhlHCij158UtIOUP/9gRAC7H35s/1sJXhDZT3sMLn3/tZhV7 aWcN73j9+nlL33mr6ie5dvAYJUjVKshl6AviGRMeNglXhoCBBVDSmhZzyQClFEryL6aG E8UwArx1nYdxTy3CYrwtb/ObqUZ8XeDEoelJ/4aWBtjKXZWtTcThztS84KOf7idyfQVd fSSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=h66ZlJIW0sPnpY7cABzav3ob2j9zOJpX5LkgC0xA0/8=; b=xtKEwWUPNOyz6qLjjpyBrSjcA3HO/XRIIxg4LpG7cPwhjIOhS/Ms2Rn+7k96fNOc59 IF2KbbR/KQt6Kv+VqcM8AaJC0CT2kaVIpiYFRF0iuVegZkM8WYS8ldXCt8tV1Ldj7TZC u8G7FCDezZ1IfNh4J8nGCpUCxsMd5jcV5pBovea3WKMLcZAta7QNh64kdcK650+UbBL/ E3AC1Nymor7/vbkJmYvyzL/+MQfQxUoL431vQl3Umm//873ttMm5TTdSjAwveBtyjl+X Hj6UM4SILeRA5/JaPLnk32COUl7S/bq6xRUJsrpS/3uEWcWboHXyNbMRd2wQjLZRD5Z3 G6mA== X-Gm-Message-State: AFqh2kr/VIjlOYycfaLeFiMslH9p7b8cbitRM62l3tpmAcPie7HN9nwI QfAxUM3J9rM2P4H+JXvFsmSibsiwd1A= X-Received: from surenb-desktop.mtv.corp.google.com ([2620:15c:211:200:77b8:ecbc:954d:4461]) (user=surenb job=sendgmr) by 2002:a25:d496:0:b0:70c:4fa3:2cce with SMTP id m144-20020a25d496000000b0070c4fa32ccemr3787363ybf.539.1672256574382; Wed, 28 Dec 2022 11:42:54 -0800 (PST) Date: Wed, 28 Dec 2022 11:42:49 -0800 Mime-Version: 1.0 X-Mailer: git-send-email 2.39.0.314.g84b9a713c41-goog Message-ID: <20221228194249.170354-1-surenb@google.com> Subject: [PATCH 1/1] mm: fix vma->anon_name memory leak for anonymous shmem VMAs From: Suren Baghdasaryan <surenb@google.com> To: akpm@linux-foundation.org Cc: hughd@google.com, hannes@cmpxchg.org, david@redhat.com, vincent.whitchurch@axis.com, seanjc@google.com, rppt@kernel.org, shy828301@gmail.com, pasha.tatashin@soleen.com, paul.gortmaker@windriver.com, peterx@redhat.com, vbabka@suse.cz, Liam.Howlett@Oracle.com, ccross@google.com, willy@infradead.org, arnd@arndb.de, cgel.zte@gmail.com, yuzhao@google.com, bagasdotme@gmail.com, suleiman@google.com, steven@liquorix.net, heftig@archlinux.org, cuigaosheng1@huawei.com, kirill@shutemov.name, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, surenb@google.com, syzbot+91edf9178386a07d06a7@syzkaller.appspotmail.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1753488296316395951?= X-GMAIL-MSGID: =?utf-8?q?1753488296316395951?= |
Series |
[1/1] mm: fix vma->anon_name memory leak for anonymous shmem VMAs
|
|
Commit Message
Suren Baghdasaryan
Dec. 28, 2022, 7:42 p.m. UTC
free_anon_vma_name() is missing a check for anonymous shmem VMA which
leads to a memory leak due to refcount not being dropped. Fix this by
adding the missing check.
Fixes: d09e8ca6cb93 ("mm: anonymous shared memory naming")
Reported-by: syzbot+91edf9178386a07d06a7@syzkaller.appspotmail.com
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
include/linux/mm_inline.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 28.12.22 20:42, Suren Baghdasaryan wrote: > free_anon_vma_name() is missing a check for anonymous shmem VMA which > leads to a memory leak due to refcount not being dropped. Fix this by > adding the missing check. > > Fixes: d09e8ca6cb93 ("mm: anonymous shared memory naming") > Reported-by: syzbot+91edf9178386a07d06a7@syzkaller.appspotmail.com > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > --- > include/linux/mm_inline.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h > index e8ed225d8f7c..d650ca2c5d29 100644 > --- a/include/linux/mm_inline.h > +++ b/include/linux/mm_inline.h > @@ -413,7 +413,7 @@ static inline void free_anon_vma_name(struct vm_area_struct *vma) > * Not using anon_vma_name because it generates a warning if mmap_lock > * is not held, which might be the case here. > */ > - if (!vma->vm_file) > + if (!vma->vm_file || vma_is_anon_shmem(vma)) > anon_vma_name_put(vma->anon_name); Wouldn't it be me more consistent to check for "vma->anon_name"? That's what dup_anon_vma_name() checks. And it's safe now because anon_name is no longer overloaded in vm_area_struct.
On Mon, Jan 2, 2023 at 4:00 AM David Hildenbrand <david@redhat.com> wrote: > > On 28.12.22 20:42, Suren Baghdasaryan wrote: > > free_anon_vma_name() is missing a check for anonymous shmem VMA which > > leads to a memory leak due to refcount not being dropped. Fix this by > > adding the missing check. > > > > Fixes: d09e8ca6cb93 ("mm: anonymous shared memory naming") > > Reported-by: syzbot+91edf9178386a07d06a7@syzkaller.appspotmail.com > > Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > --- > > include/linux/mm_inline.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h > > index e8ed225d8f7c..d650ca2c5d29 100644 > > --- a/include/linux/mm_inline.h > > +++ b/include/linux/mm_inline.h > > @@ -413,7 +413,7 @@ static inline void free_anon_vma_name(struct vm_area_struct *vma) > > * Not using anon_vma_name because it generates a warning if mmap_lock > > * is not held, which might be the case here. > > */ > > - if (!vma->vm_file) > > + if (!vma->vm_file || vma_is_anon_shmem(vma)) > > anon_vma_name_put(vma->anon_name); > > Wouldn't it be me more consistent to check for "vma->anon_name"? > > That's what dup_anon_vma_name() checks. And it's safe now because > anon_name is no longer overloaded in vm_area_struct. Thanks for the suggestion, David. Yes, with the recent change that does not overload anon_name, checking for "vma->anon_name" would be simpler. I think we can also drop anon_vma_name() function now (https://elixir.bootlin.com/linux/v6.2-rc2/source/mm/madvise.c#L94) since vma->anon_name does not depend on vma->vm_file anymore, remove the last part of this comment: https://elixir.bootlin.com/linux/v6.2-rc2/source/include/linux/mm_types.h#L584 and use vma->anon_name directly going forward. If all that sounds good, I'll post a separate patch implementing all these changes. So, for this patch I would suggest keeping it as is because functionally it is correct and will change this check along with other corrections I mentioned above in a separate patch. Does that sound good? - if (!vma->vm_file) + if (!vma->vm_file || vma_is_anon_shmem(vma)) > > -- > Thanks, > > David / dhildenb >
On 03.01.23 20:53, Suren Baghdasaryan wrote: > On Mon, Jan 2, 2023 at 4:00 AM David Hildenbrand <david@redhat.com> wrote: >> >> On 28.12.22 20:42, Suren Baghdasaryan wrote: >>> free_anon_vma_name() is missing a check for anonymous shmem VMA which >>> leads to a memory leak due to refcount not being dropped. Fix this by >>> adding the missing check. >>> >>> Fixes: d09e8ca6cb93 ("mm: anonymous shared memory naming") >>> Reported-by: syzbot+91edf9178386a07d06a7@syzkaller.appspotmail.com >>> Signed-off-by: Suren Baghdasaryan <surenb@google.com> >>> --- >>> include/linux/mm_inline.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h >>> index e8ed225d8f7c..d650ca2c5d29 100644 >>> --- a/include/linux/mm_inline.h >>> +++ b/include/linux/mm_inline.h >>> @@ -413,7 +413,7 @@ static inline void free_anon_vma_name(struct vm_area_struct *vma) >>> * Not using anon_vma_name because it generates a warning if mmap_lock >>> * is not held, which might be the case here. >>> */ >>> - if (!vma->vm_file) >>> + if (!vma->vm_file || vma_is_anon_shmem(vma)) >>> anon_vma_name_put(vma->anon_name); >> >> Wouldn't it be me more consistent to check for "vma->anon_name"? >> >> That's what dup_anon_vma_name() checks. And it's safe now because >> anon_name is no longer overloaded in vm_area_struct. > > Thanks for the suggestion, David. Yes, with the recent change that > does not overload anon_name, checking for "vma->anon_name" would be > simpler. I think we can also drop anon_vma_name() function now > (https://elixir.bootlin.com/linux/v6.2-rc2/source/mm/madvise.c#L94) > since vma->anon_name does not depend on vma->vm_file anymore, remove > the last part of this comment: > https://elixir.bootlin.com/linux/v6.2-rc2/source/include/linux/mm_types.h#L584 > and use vma->anon_name directly going forward. If all that sounds > good, I'll post a separate patch implementing all these changes. > So, for this patch I would suggest keeping it as is because > functionally it is correct and will change this check along with other > corrections I mentioned above in a separate patch. Does that sound > good? Works for me. Acked-by: David Hildenbrand <david@redhat.com> for this one, as it fixes the issue.
On Wed, Jan 4, 2023 at 1:04 AM David Hildenbrand <david@redhat.com> wrote: > > On 03.01.23 20:53, Suren Baghdasaryan wrote: > > On Mon, Jan 2, 2023 at 4:00 AM David Hildenbrand <david@redhat.com> wrote: > >> > >> On 28.12.22 20:42, Suren Baghdasaryan wrote: > >>> free_anon_vma_name() is missing a check for anonymous shmem VMA which > >>> leads to a memory leak due to refcount not being dropped. Fix this by > >>> adding the missing check. > >>> > >>> Fixes: d09e8ca6cb93 ("mm: anonymous shared memory naming") > >>> Reported-by: syzbot+91edf9178386a07d06a7@syzkaller.appspotmail.com > >>> Signed-off-by: Suren Baghdasaryan <surenb@google.com> > >>> --- > >>> include/linux/mm_inline.h | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h > >>> index e8ed225d8f7c..d650ca2c5d29 100644 > >>> --- a/include/linux/mm_inline.h > >>> +++ b/include/linux/mm_inline.h > >>> @@ -413,7 +413,7 @@ static inline void free_anon_vma_name(struct vm_area_struct *vma) > >>> * Not using anon_vma_name because it generates a warning if mmap_lock > >>> * is not held, which might be the case here. > >>> */ > >>> - if (!vma->vm_file) > >>> + if (!vma->vm_file || vma_is_anon_shmem(vma)) > >>> anon_vma_name_put(vma->anon_name); > >> > >> Wouldn't it be me more consistent to check for "vma->anon_name"? > >> > >> That's what dup_anon_vma_name() checks. And it's safe now because > >> anon_name is no longer overloaded in vm_area_struct. > > > > Thanks for the suggestion, David. Yes, with the recent change that > > does not overload anon_name, checking for "vma->anon_name" would be > > simpler. I think we can also drop anon_vma_name() function now > > (https://elixir.bootlin.com/linux/v6.2-rc2/source/mm/madvise.c#L94) > > since vma->anon_name does not depend on vma->vm_file anymore, remove > > the last part of this comment: > > https://elixir.bootlin.com/linux/v6.2-rc2/source/include/linux/mm_types.h#L584 > > and use vma->anon_name directly going forward. If all that sounds > > good, I'll post a separate patch implementing all these changes. > > So, for this patch I would suggest keeping it as is because > > functionally it is correct and will change this check along with other > > corrections I mentioned above in a separate patch. Does that sound > > good? > > Works for me. > > Acked-by: David Hildenbrand <david@redhat.com> Thank you! Will post the followup cleanup patch shorly. > > for this one, as it fixes the issue. > > -- > Thanks, > > David / dhildenb >
On Wed, Jan 4, 2023 at 10:24 AM Suren Baghdasaryan <surenb@google.com> wrote: > > On Wed, Jan 4, 2023 at 1:04 AM David Hildenbrand <david@redhat.com> wrote: > > > > On 03.01.23 20:53, Suren Baghdasaryan wrote: > > > On Mon, Jan 2, 2023 at 4:00 AM David Hildenbrand <david@redhat.com> wrote: > > >> > > >> On 28.12.22 20:42, Suren Baghdasaryan wrote: > > >>> free_anon_vma_name() is missing a check for anonymous shmem VMA which > > >>> leads to a memory leak due to refcount not being dropped. Fix this by > > >>> adding the missing check. > > >>> > > >>> Fixes: d09e8ca6cb93 ("mm: anonymous shared memory naming") > > >>> Reported-by: syzbot+91edf9178386a07d06a7@syzkaller.appspotmail.com > > >>> Signed-off-by: Suren Baghdasaryan <surenb@google.com> > > >>> --- > > >>> include/linux/mm_inline.h | 2 +- > > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > > >>> > > >>> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h > > >>> index e8ed225d8f7c..d650ca2c5d29 100644 > > >>> --- a/include/linux/mm_inline.h > > >>> +++ b/include/linux/mm_inline.h > > >>> @@ -413,7 +413,7 @@ static inline void free_anon_vma_name(struct vm_area_struct *vma) > > >>> * Not using anon_vma_name because it generates a warning if mmap_lock > > >>> * is not held, which might be the case here. > > >>> */ > > >>> - if (!vma->vm_file) > > >>> + if (!vma->vm_file || vma_is_anon_shmem(vma)) > > >>> anon_vma_name_put(vma->anon_name); > > >> > > >> Wouldn't it be me more consistent to check for "vma->anon_name"? > > >> > > >> That's what dup_anon_vma_name() checks. And it's safe now because > > >> anon_name is no longer overloaded in vm_area_struct. > > > > > > Thanks for the suggestion, David. Yes, with the recent change that > > > does not overload anon_name, checking for "vma->anon_name" would be > > > simpler. I think we can also drop anon_vma_name() function now > > > (https://elixir.bootlin.com/linux/v6.2-rc2/source/mm/madvise.c#L94) > > > since vma->anon_name does not depend on vma->vm_file anymore, remove > > > the last part of this comment: > > > https://elixir.bootlin.com/linux/v6.2-rc2/source/include/linux/mm_types.h#L584 > > > and use vma->anon_name directly going forward. If all that sounds > > > good, I'll post a separate patch implementing all these changes. > > > So, for this patch I would suggest keeping it as is because > > > functionally it is correct and will change this check along with other > > > corrections I mentioned above in a separate patch. Does that sound > > > good? > > > > Works for me. > > > > Acked-by: David Hildenbrand <david@redhat.com> > > Thank you! Will post the followup cleanup patch shorly. Andrew, I posted v2 of this patch at https://lore.kernel.org/all/20230105000241.1450843-1-surenb@google.com/. Please replace the original one in your tree. I had to follow David's original suggestion instead of my planned cleanup because removing anon_vma_name() would require us to add #ifdef CONFIG_ANON_VMA_NAME everywhere we use vma->anon_name, which is quite ugly. Thanks, Suren. > > > > > for this one, as it fixes the issue. > > > > -- > > Thanks, > > > > David / dhildenb > >
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h index e8ed225d8f7c..d650ca2c5d29 100644 --- a/include/linux/mm_inline.h +++ b/include/linux/mm_inline.h @@ -413,7 +413,7 @@ static inline void free_anon_vma_name(struct vm_area_struct *vma) * Not using anon_vma_name because it generates a warning if mmap_lock * is not held, which might be the case here. */ - if (!vma->vm_file) + if (!vma->vm_file || vma_is_anon_shmem(vma)) anon_vma_name_put(vma->anon_name); }