Message ID | 20221214222110.200487-1-mathieu.desnoyers@efficios.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:504a:0:0:0:0:0 with SMTP id h10csp49035wrt; Wed, 14 Dec 2022 14:23:41 -0800 (PST) X-Google-Smtp-Source: AA0mqf7UOuNkrOdegy5HKIHJ15PVzjJ87EqIQ4F3unpcVDn8//oz4Z6qBMNPOl2PQx+M5lP9AWaB X-Received: by 2002:a05:6a20:7d8a:b0:aa:5c2d:ae39 with SMTP id v10-20020a056a207d8a00b000aa5c2dae39mr45114871pzj.22.1671056621112; Wed, 14 Dec 2022 14:23:41 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1671056621; cv=none; d=google.com; s=arc-20160816; b=eLrHXp5Rp7qvotiOtDwJ+KQCMCjlFNgEuFVi4o843jBSTgckoXqTPUlsqfVsG1w7ug l8DymqypIqWf+5v/5mP6dqT371LSeD+t0XNKIzCfcZ7xJoFJKah8q3HkZtU7NxTuVL1O fca/gl5h2XMwG2Job3LLOW7TPW/+BeqaFgTUwBv2DQNnpztIVXaERjqVsuuBmMIo825T ecjxYnYf1jpfvUr3f+sbk65FOG8ZMxKUBgQ2WFquardmkk1rnYw32xHvfRQVUJ/cgWuq Li0zymKzINrPCOhWB51SQSU7/5S+97r7xc38K7NfRV0Q26u6DV07jQ1/RqdkpLD7ZkN8 JoZA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=QjGhfKQquteToFYBz4/MQr+ElY1789ItCPJua6nh46I=; b=AJu7Kp0CulmxEQOJQyRBOZlkE104CTN7W+q2rNvmFZpkrmslUe2ObxmGxmBATDn2AL WFqZXgEUTBRQuI2XKFdO2TVI/z2h0T7V0ZoOQO/b4grN2O90LdhOcLXdZmY8LG1Cqyft rjj6AWvzFsDOvUMdMo91vwd+yB+7koZTBg6/X6hV8VHddQnQq5knovTWfpHA8jNKuRlD jHbqReoLJWNlU+k8kbWxxhMIHhpR9iuOhccXw2cbUEY4EU7F1iA5X4wTb/JsFwGd7WV+ f1fZgkLXBnTjDDJXwGAH68oKGtxxpo1YXvPUbeAgDQnKYqV2YokATNGPJBIKdxugle/K ABHA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@efficios.com header.s=smtpout1 header.b=GqNGuGAN; 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=NONE sp=NONE dis=NONE) header.from=efficios.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id 203-20020a6305d4000000b00477257f2f01si1010395pgf.339.2022.12.14.14.23.25; Wed, 14 Dec 2022 14:23:41 -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=@efficios.com header.s=smtpout1 header.b=GqNGuGAN; 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=NONE sp=NONE dis=NONE) header.from=efficios.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229588AbiLNWVx (ORCPT <rfc822;jeantsuru.cumc.mandola@gmail.com> + 99 others); Wed, 14 Dec 2022 17:21:53 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52762 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229749AbiLNWV2 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 14 Dec 2022 17:21:28 -0500 Received: from smtpout.efficios.com (smtpout.efficios.com [167.114.26.122]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2BA8F116E; Wed, 14 Dec 2022 14:21:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=efficios.com; s=smtpout1; t=1671056483; bh=OwPacgMfzq+t+Js+BScNggpyccUTyftXnAv/AFiH6zw=; h=From:To:Cc:Subject:Date:From; b=GqNGuGANib8ov9H9kk3JHTVguBne0Fq6w32B7YeuElmDddZfC27OijEBZF7qvDIE9 AmvlSGspDuxrtk4kL/DETEYf/C7U7oYscCP0hWIZaIiIluj+uTGSnOQSqHWQ1KpZU0 /OXQkcNEx4e7kTGF9Jtj6Hb4W4E51/Aw/v91jLhyCupDRCTlO2FT4qbPjHW3PAFUFG +1/O1ngi4uSX2uBoe4rS08zGUXzL2EQQZyoMidwsGjvstKyTvw8V4ujqxyfmhYPGX4 pKE2v8TtOdqEylTqOZVNW8tiw1+PXWERuLKuG4payC+03Xn1JD3N0vPYJJWTZQCLdJ /iHSF+ynCAWSQ== Received: from localhost.localdomain (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) by smtpout.efficios.com (Postfix) with ESMTPSA id 4NXVFb1fSJzbZs; Wed, 14 Dec 2022 17:21:23 -0500 (EST) From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> To: Andrew Morton <akpm@linux-foundation.org> Cc: linux-kernel@vger.kernel.org, Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>, Ben Widawsky <ben.widawsky@intel.com>, Dave Hansen <dave.hansen@linux.intel.com>, Feng Tang <feng.tang@intel.com>, Michal Hocko <mhocko@kernel.org>, Andrea Arcangeli <aarcange@redhat.com>, Mel Gorman <mgorman@techsingularity.net>, Mike Kravetz <mike.kravetz@oracle.com>, Randy Dunlap <rdunlap@infradead.org>, Vlastimil Babka <vbabka@suse.cz>, Andi Kleen <ak@linux.intel.com>, Dan Williams <dan.j.williams@intel.com>, Huang Ying <ying.huang@intel.com>, linux-api@vger.kernel.org, stable@vger.kernel.org Subject: [RFC PATCH] mm/mempolicy: Fix memory leak in set_mempolicy_home_node system call Date: Wed, 14 Dec 2022 17:21:10 -0500 Message-Id: <20221214222110.200487-1-mathieu.desnoyers@efficios.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS 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?1752229867635979556?= X-GMAIL-MSGID: =?utf-8?q?1752229867635979556?= |
Series |
[RFC] mm/mempolicy: Fix memory leak in set_mempolicy_home_node system call
|
|
Commit Message
Mathieu Desnoyers
Dec. 14, 2022, 10:21 p.m. UTC
When encountering any vma in the range with policy other than MPOL_BIND
or MPOL_PREFERRED_MANY, an error is returned without issuing a mpol_put
on the policy just allocated with mpol_dup().
This allows arbitrary users to leak kernel memory.
Fixes: c6018b4b2549 ("mm/mempolicy: add set_mempolicy_home_node syscall")
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Cc: Ben Widawsky <ben.widawsky@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Feng Tang <feng.tang@intel.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Huang Ying <ying.huang@intel.com>
Cc: <linux-api@vger.kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: stable@vger.kernel.org # 5.17+
---
mm/mempolicy.c | 1 +
1 file changed, 1 insertion(+)
Comments
On 12/14/22 14:21, Mathieu Desnoyers wrote: > When encountering any vma in the range with policy other than MPOL_BIND > or MPOL_PREFERRED_MANY, an error is returned without issuing a mpol_put > on the policy just allocated with mpol_dup(). > > This allows arbitrary users to leak kernel memory. > > Fixes: c6018b4b2549 ("mm/mempolicy: add set_mempolicy_home_node syscall") > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > Cc: Ben Widawsky <ben.widawsky@intel.com> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: Feng Tang <feng.tang@intel.com> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: Mel Gorman <mgorman@techsingularity.net> > Cc: Mike Kravetz <mike.kravetz@oracle.com> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Vlastimil Babka <vbabka@suse.cz> > Cc: Andi Kleen <ak@linux.intel.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Huang Ying <ying.huang@intel.com> > Cc: <linux-api@vger.kernel.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: stable@vger.kernel.org # 5.17+ Reviewed-by: Randy Dunlap <rdunlap@infradead.org> Thanks. > --- > mm/mempolicy.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index 61aa9aedb728..02c8a712282f 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -1540,6 +1540,7 @@ SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, le > * the home node for vmas we already updated before. > */ > if (new->mode != MPOL_BIND && new->mode != MPOL_PREFERRED_MANY) { > + mpol_put(new); > err = -EOPNOTSUPP; > break; > }
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> writes: > When encountering any vma in the range with policy other than MPOL_BIND > or MPOL_PREFERRED_MANY, an error is returned without issuing a mpol_put > on the policy just allocated with mpol_dup(). > > This allows arbitrary users to leak kernel memory. > > Fixes: c6018b4b2549 ("mm/mempolicy: add set_mempolicy_home_node syscall") > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > Cc: Ben Widawsky <ben.widawsky@intel.com> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: Feng Tang <feng.tang@intel.com> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: Mel Gorman <mgorman@techsingularity.net> > Cc: Mike Kravetz <mike.kravetz@oracle.com> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Vlastimil Babka <vbabka@suse.cz> > Cc: Andi Kleen <ak@linux.intel.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Huang Ying <ying.huang@intel.com> > Cc: <linux-api@vger.kernel.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: stable@vger.kernel.org # 5.17+ Reviewed-by: "Huang, Ying" <ying.huang@intel.com> Thanks! > --- > mm/mempolicy.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index 61aa9aedb728..02c8a712282f 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -1540,6 +1540,7 @@ SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, le > * the home node for vmas we already updated before. > */ > if (new->mode != MPOL_BIND && new->mode != MPOL_PREFERRED_MANY) { > + mpol_put(new); > err = -EOPNOTSUPP; > break; > }
On Wed 14-12-22 17:21:10, Mathieu Desnoyers wrote: > When encountering any vma in the range with policy other than MPOL_BIND > or MPOL_PREFERRED_MANY, an error is returned without issuing a mpol_put > on the policy just allocated with mpol_dup(). > > This allows arbitrary users to leak kernel memory. > > Fixes: c6018b4b2549 ("mm/mempolicy: add set_mempolicy_home_node syscall") > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > Cc: Ben Widawsky <ben.widawsky@intel.com> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: Feng Tang <feng.tang@intel.com> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: Mel Gorman <mgorman@techsingularity.net> > Cc: Mike Kravetz <mike.kravetz@oracle.com> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Vlastimil Babka <vbabka@suse.cz> > Cc: Andi Kleen <ak@linux.intel.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Huang Ying <ying.huang@intel.com> > Cc: <linux-api@vger.kernel.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: stable@vger.kernel.org # 5.17+ Acked-by: Michal Hocko <mhocko@suse.com> Thanks for catching this! Btw. looking at the code again it seems rather pointless to duplicate the policy just to throw it away anyway. A slightly bigger diff but this looks more reasonable to me. What do you think? I can also send it as a clean up on top of your fix. --- diff --git a/mm/mempolicy.c b/mm/mempolicy.c index 61aa9aedb728..918cdc8a7f0c 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -1489,7 +1489,7 @@ SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, le { struct mm_struct *mm = current->mm; struct vm_area_struct *vma; - struct mempolicy *new; + struct mempolicy *new. *old; unsigned long vmstart; unsigned long vmend; unsigned long end; @@ -1521,30 +1521,28 @@ SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, le return 0; mmap_write_lock(mm); for_each_vma_range(vmi, vma, end) { - vmstart = max(start, vma->vm_start); - vmend = min(end, vma->vm_end); - new = mpol_dup(vma_policy(vma)); - if (IS_ERR(new)) { - err = PTR_ERR(new); - break; - } - /* - * Only update home node if there is an existing vma policy - */ - if (!new) - continue; - /* * If any vma in the range got policy other than MPOL_BIND * or MPOL_PREFERRED_MANY we return error. We don't reset * the home node for vmas we already updated before. */ - if (new->mode != MPOL_BIND && new->mode != MPOL_PREFERRED_MANY) { + old = vma_policy(vma); + if (!old) + continue; + if (old->mode != MPOL_BIND && old->mode != MPOL_PREFERRED_MANY) { err = -EOPNOTSUPP; break; } + new = mpol_dup(vma_policy(vma)); + if (IS_ERR(new)) { + err = PTR_ERR(new); + break; + } + new->home_node = home_node; + vmstart = max(start, vma->vm_start); + vmend = min(end, vma->vm_end); err = mbind_range(mm, vmstart, vmend, new); mpol_put(new); if (err)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> writes: > When encountering any vma in the range with policy other than MPOL_BIND > or MPOL_PREFERRED_MANY, an error is returned without issuing a mpol_put > on the policy just allocated with mpol_dup(). > > This allows arbitrary users to leak kernel memory. > Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > Fixes: c6018b4b2549 ("mm/mempolicy: add set_mempolicy_home_node syscall") > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> > Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> > Cc: Ben Widawsky <ben.widawsky@intel.com> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: Feng Tang <feng.tang@intel.com> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Andrea Arcangeli <aarcange@redhat.com> > Cc: Mel Gorman <mgorman@techsingularity.net> > Cc: Mike Kravetz <mike.kravetz@oracle.com> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Vlastimil Babka <vbabka@suse.cz> > Cc: Andi Kleen <ak@linux.intel.com> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Huang Ying <ying.huang@intel.com> > Cc: <linux-api@vger.kernel.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: stable@vger.kernel.org # 5.17+ > --- > mm/mempolicy.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index 61aa9aedb728..02c8a712282f 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -1540,6 +1540,7 @@ SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, le > * the home node for vmas we already updated before. > */ > if (new->mode != MPOL_BIND && new->mode != MPOL_PREFERRED_MANY) { > + mpol_put(new); > err = -EOPNOTSUPP; > break; > } > -- > 2.25.1
Michal Hocko <mhocko@suse.com> writes: > On Wed 14-12-22 17:21:10, Mathieu Desnoyers wrote: >> When encountering any vma in the range with policy other than MPOL_BIND >> or MPOL_PREFERRED_MANY, an error is returned without issuing a mpol_put >> on the policy just allocated with mpol_dup(). >> >> This allows arbitrary users to leak kernel memory. >> >> Fixes: c6018b4b2549 ("mm/mempolicy: add set_mempolicy_home_node syscall") >> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> >> Cc: Ben Widawsky <ben.widawsky@intel.com> >> Cc: Dave Hansen <dave.hansen@linux.intel.com> >> Cc: Feng Tang <feng.tang@intel.com> >> Cc: Michal Hocko <mhocko@kernel.org> >> Cc: Andrea Arcangeli <aarcange@redhat.com> >> Cc: Mel Gorman <mgorman@techsingularity.net> >> Cc: Mike Kravetz <mike.kravetz@oracle.com> >> Cc: Randy Dunlap <rdunlap@infradead.org> >> Cc: Vlastimil Babka <vbabka@suse.cz> >> Cc: Andi Kleen <ak@linux.intel.com> >> Cc: Dan Williams <dan.j.williams@intel.com> >> Cc: Huang Ying <ying.huang@intel.com> >> Cc: <linux-api@vger.kernel.org> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: stable@vger.kernel.org # 5.17+ > > Acked-by: Michal Hocko <mhocko@suse.com> > Thanks for catching this! > > Btw. looking at the code again it seems rather pointless to duplicate > the policy just to throw it away anyway. A slightly bigger diff but this > looks more reasonable to me. What do you think? I can also send it as a > clean up on top of your fix. > --- > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index 61aa9aedb728..918cdc8a7f0c 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -1489,7 +1489,7 @@ SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, le > { > struct mm_struct *mm = current->mm; > struct vm_area_struct *vma; > - struct mempolicy *new; > + struct mempolicy *new. *old; > unsigned long vmstart; > unsigned long vmend; > unsigned long end; > @@ -1521,30 +1521,28 @@ SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, le > return 0; > mmap_write_lock(mm); > for_each_vma_range(vmi, vma, end) { > - vmstart = max(start, vma->vm_start); > - vmend = min(end, vma->vm_end); > - new = mpol_dup(vma_policy(vma)); > - if (IS_ERR(new)) { > - err = PTR_ERR(new); > - break; > - } > - /* > - * Only update home node if there is an existing vma policy > - */ > - if (!new) > - continue; > - > /* > * If any vma in the range got policy other than MPOL_BIND > * or MPOL_PREFERRED_MANY we return error. We don't reset > * the home node for vmas we already updated before. > */ > - if (new->mode != MPOL_BIND && new->mode != MPOL_PREFERRED_MANY) { > + old = vma_policy(vma); > + if (!old) > + continue; > + if (old->mode != MPOL_BIND && old->mode != MPOL_PREFERRED_MANY) { > err = -EOPNOTSUPP; > break; > } > > + new = mpol_dup(vma_policy(vma)); new = mpol_dup(old); > + if (IS_ERR(new)) { > + err = PTR_ERR(new); > + break; > + } > + > new->home_node = home_node; > + vmstart = max(start, vma->vm_start); > + vmend = min(end, vma->vm_end); > err = mbind_range(mm, vmstart, vmend, new); > mpol_put(new); > if (err) > -- > Michal Hocko > SUSE Labs
On 2022-12-15 02:51, Michal Hocko wrote: > On Wed 14-12-22 17:21:10, Mathieu Desnoyers wrote: >> When encountering any vma in the range with policy other than MPOL_BIND >> or MPOL_PREFERRED_MANY, an error is returned without issuing a mpol_put >> on the policy just allocated with mpol_dup(). >> >> This allows arbitrary users to leak kernel memory. >> >> Fixes: c6018b4b2549 ("mm/mempolicy: add set_mempolicy_home_node syscall") >> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> >> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> >> Cc: Ben Widawsky <ben.widawsky@intel.com> >> Cc: Dave Hansen <dave.hansen@linux.intel.com> >> Cc: Feng Tang <feng.tang@intel.com> >> Cc: Michal Hocko <mhocko@kernel.org> >> Cc: Andrea Arcangeli <aarcange@redhat.com> >> Cc: Mel Gorman <mgorman@techsingularity.net> >> Cc: Mike Kravetz <mike.kravetz@oracle.com> >> Cc: Randy Dunlap <rdunlap@infradead.org> >> Cc: Vlastimil Babka <vbabka@suse.cz> >> Cc: Andi Kleen <ak@linux.intel.com> >> Cc: Dan Williams <dan.j.williams@intel.com> >> Cc: Huang Ying <ying.huang@intel.com> >> Cc: <linux-api@vger.kernel.org> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: stable@vger.kernel.org # 5.17+ > > Acked-by: Michal Hocko <mhocko@suse.com> > Thanks for catching this! > > Btw. looking at the code again it seems rather pointless to duplicate > the policy just to throw it away anyway. A slightly bigger diff but this > looks more reasonable to me. What do you think? I can also send it as a > clean up on top of your fix. I think it would be best if this comes as a cleanup on top of my fix. The diff is larger than the minimal change needed to fix the leak in stable branches. Your approach looks fine, except for the vma_policy(vma) -> old change already spotted by Aneesh. Thanks, Mathieu > --- > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index 61aa9aedb728..918cdc8a7f0c 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -1489,7 +1489,7 @@ SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, le > { > struct mm_struct *mm = current->mm; > struct vm_area_struct *vma; > - struct mempolicy *new; > + struct mempolicy *new. *old; > unsigned long vmstart; > unsigned long vmend; > unsigned long end; > @@ -1521,30 +1521,28 @@ SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, le > return 0; > mmap_write_lock(mm); > for_each_vma_range(vmi, vma, end) { > - vmstart = max(start, vma->vm_start); > - vmend = min(end, vma->vm_end); > - new = mpol_dup(vma_policy(vma)); > - if (IS_ERR(new)) { > - err = PTR_ERR(new); > - break; > - } > - /* > - * Only update home node if there is an existing vma policy > - */ > - if (!new) > - continue; > - > /* > * If any vma in the range got policy other than MPOL_BIND > * or MPOL_PREFERRED_MANY we return error. We don't reset > * the home node for vmas we already updated before. > */ > - if (new->mode != MPOL_BIND && new->mode != MPOL_PREFERRED_MANY) { > + old = vma_policy(vma); > + if (!old) > + continue; > + if (old->mode != MPOL_BIND && old->mode != MPOL_PREFERRED_MANY) { > err = -EOPNOTSUPP; > break; > } > > + new = mpol_dup(vma_policy(vma)); > + if (IS_ERR(new)) { > + err = PTR_ERR(new); > + break; > + } > + > new->home_node = home_node; > + vmstart = max(start, vma->vm_start); > + vmend = min(end, vma->vm_end); > err = mbind_range(mm, vmstart, vmend, new); > mpol_put(new); > if (err)
diff --git a/mm/mempolicy.c b/mm/mempolicy.c index 61aa9aedb728..02c8a712282f 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -1540,6 +1540,7 @@ SYSCALL_DEFINE4(set_mempolicy_home_node, unsigned long, start, unsigned long, le * the home node for vmas we already updated before. */ if (new->mode != MPOL_BIND && new->mode != MPOL_PREFERRED_MANY) { + mpol_put(new); err = -EOPNOTSUPP; break; }