Received-SPF: pass (google.com: domain of linux-kernel+bounces-185387-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 21 May 2024 15:38:45 -0400
From: Peter Xu <peterx@redhat.com>
To: Michal Hocko <mhocko@suse.com>
Cc: cve@kernel.org, linux-kernel@vger.kernel.org,
	linux-cve-announce@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: CVE-2024-36000: mm/hugetlb: fix missing hugetlb_lock for resv
 uncharge
Message-ID: <Zkz4RRgfwUHPbQ5z@x1n>
References: <2024052023-CVE-2024-36000-cfc4@gregkh>
 <Zkto8rbtAUBql-78@tiehlicka>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <Zkto8rbtAUBql-78@tiehlicka>

On Mon, May 20, 2024 at 05:14:58PM +0200, Michal Hocko wrote:
> Peter,

Hi, Michal,

> does b76b46902c2d ("mm/hugetlb: fix missing hugetlb_lock for resv
> uncharge") really have any security implications? I fail to see any but
> UFFD is not really my area so I might be missing something very easily.

AFAIU that issue wasn't userfault specific, but a generic issue for hugetlb
- I believe that can also trigger in other paths whoever try to call
alloc_hugetlb_folio(), and UFFDIO_COPY is one user of it.

I looked at that and provided a fix only because the report originated from
the uffd report, so Andrew normally pointing those to me, and since I
looked anyway I tried to fix that.

Here in general what I can see is that the lock is needed since this
commit:

    commit 94ae8ba7176666d1e7d8bbb9f93670a27540b6a8
    Author: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
    Date:   Tue Jul 31 16:42:35 2012 -0700

    hugetlb/cgroup: assign the page hugetlb cgroup when we move the page to active list.

That commit mentioned that we rely on the lock to make sure all hugetlb
folios on the active list will have a valid memcg.  However I'm not sure
whether it's still required now (after all that's 2012..), e.g., I'm
looking at hugetlb_cgroup_css_offline(), and hugetlb_cgroup_move_parent()
looks all safe to even take empty memcg folios with the latest code at
least:

	/*
	 * We can have pages in active list without any cgroup
	 * ie, hugepage with less than 3 pages. We can safely
	 * ignore those pages.
	 */
	if (!page_hcg || page_hcg != h_cg)
		goto out;

In short, I don't know any further security implications on this problem
besides LOCKDEP enabled.  But I don't think I fully understand the hugetlb
reservation code, so please just take that with a grain of salt.  E.g.,
right now we do the hugetlb_cgroup_uncharge_folio_rsvd(), then could it
happen that this folio will still be used finally and got injected into the
pgtables (after all, alloc_hugetlb_folio() will still return this folio to
the caller with a success), and would that be a problem if this folio has
its _hugetlb_cgroup_rsvd==NULL?  That looks like another question besides
this specific problem, though..

Thanks,

> 
> On Mon 20-05-24 11:48:36, Greg KH wrote:
> > Description
> > ===========
> > 
> > In the Linux kernel, the following vulnerability has been resolved:
> > 
> > mm/hugetlb: fix missing hugetlb_lock for resv uncharge
> > 
> > There is a recent report on UFFDIO_COPY over hugetlb:
> > 
> > https://lore.kernel.org/all/000000000000ee06de0616177560@google.com/
> > 
> > 350:	lockdep_assert_held(&hugetlb_lock);
> > 
> > Should be an issue in hugetlb but triggered in an userfault context, where
> > it goes into the unlikely path where two threads modifying the resv map
> > together.  Mike has a fix in that path for resv uncharge but it looks like
> > the locking criteria was overlooked: hugetlb_cgroup_uncharge_folio_rsvd()
> > will update the cgroup pointer, so it requires to be called with the lock
> > held.
> > 
> > The Linux kernel CVE team has assigned CVE-2024-36000 to this issue.
> > 
> > 
> > Affected and fixed versions
> > ===========================
> > 
> > 	Issue introduced in 5.10 with commit 79aa925bf239 and fixed in 6.1.91 with commit 4c806333efea
> > 	Issue introduced in 5.10 with commit 79aa925bf239 and fixed in 6.6.30 with commit f6c5d21db16a
> > 	Issue introduced in 5.10 with commit 79aa925bf239 and fixed in 6.8.9 with commit 538faabf31e9
> > 	Issue introduced in 5.10 with commit 79aa925bf239 and fixed in 6.9 with commit b76b46902c2d
> > 	Issue introduced in 5.9.7 with commit f87004c0b2bd
> > 
> > Please see https://www.kernel.org for a full list of currently supported
> > kernel versions by the kernel community.
> > 
> > Unaffected versions might change over time as fixes are backported to
> > older supported kernel versions.  The official CVE entry at
> > 	https://cve.org/CVERecord/?id=CVE-2024-36000
> > will be updated if fixes are backported, please check that for the most
> > up to date information about this issue.
> > 
> > 
> > Affected files
> > ==============
> > 
> > The file(s) affected by this issue are:
> > 	mm/hugetlb.c
> > 
> > 
> > Mitigation
> > ==========
> > 
> > The Linux kernel CVE team recommends that you update to the latest
> > stable kernel version for this, and many other bugfixes.  Individual
> > changes are never tested alone, but rather are part of a larger kernel
> > release.  Cherry-picking individual commits is not recommended or
> > supported by the Linux kernel community at all.  If however, updating to
> > the latest release is impossible, the individual changes to resolve this
> > issue can be found at these commits:
> > 	https://git.kernel.org/stable/c/4c806333efea1000a2a9620926f560ad2e1ca7cc
> > 	https://git.kernel.org/stable/c/f6c5d21db16a0910152ec8aa9d5a7aed72694505
> > 	https://git.kernel.org/stable/c/538faabf31e9c53d8c870d114846fda958a0de10
> > 	https://git.kernel.org/stable/c/b76b46902c2d0395488c8412e1116c2486cdfcb2
> 
> -- 
> Michal Hocko
> SUSE Labs
> 

-- 
Peter Xu