linux-kernel - Re: CVE-2024-36000: mm/hugetlb: fix missing hugetlb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zk7ws6H0wwuiFAJW@tiehlicka>
Date: Thu, 23 May 2024 09:30:59 +0200
From: Michal Hocko <mhocko@...e.com>
To: Oscar Salvador <OSalvador@...e.com>
Cc: Peter Xu <peterx@...hat.com>, cve@...nel.org,
	linux-kernel@...r.kernel.org, linux-cve-announce@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: CVE-2024-36000: mm/hugetlb: fix missing hugetlb_lock for resv
 uncharge

Let me add Oscar,

On Tue 21-05-24 15:38:45, Peter Xu wrote:
> 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.

OK, I see. Thanks for the clarification.

> 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@...ux.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..

Oscar, could you have a look please?

-- 
Michal Hocko
SUSE Labs