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] [day] [month] [year] [list]

Message-ID: <ZmwuDvvTDpCFGTdi@localhost.localdomain>
Date: Fri, 14 Jun 2024 13:48:30 +0200
From: Oscar Salvador <osalvador@...e.de>
To: Peter Xu <peterx@...hat.com>
Cc: Michal Hocko <mhocko@...e.com>, Oscar Salvador <OSalvador@...e.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

On Thu, May 23, 2024 at 11:42:30AM -0400, Peter Xu wrote:
> I really don't know enough on these areas to tell, perhaps I missed
> something.  But maybe any of you may have some idea..  In general, I think
> besides LOCKDEP the lock is definitely needed to at least make sure things
> like:
> 
> 	__set_hugetlb_cgroup(folio, NULL, rsvd);

I do not think this is a problem, you are only setting folio->_hugetlb_cgroup_rsvd
to the hugetlb cgroup.
And no one else should fiddle with that folio.

> 	page_counter_uncharge(),

This on the hand might be another story:

 page_counter_uncharge
  new = atomic_long_sub_return(nr_pages, &counter->usage)
  propagate_protected_usage

The first atomic_long_sub_return is ok because it is an atomic one, so
whoever comes last will not see e.g: a half-updated value.
But propagate_protected_usage() is a bit more convoluted as involves a bunch of
atomic operations and comparasions that in case they are not serialized, the counters
will not be consistent, which means that any charge/uncharge operation that comes after
might not reflect reality.

So I guess we could end up with scenarios where cgroups would not get as many pages as
they should, or maybe more pages than they should.

If this reasoning is accurate, I am leaning towards taking this as a security fix.

-- 
Oscar Salvador
SUSE Labs