Re: CVE-2024-36000: mm/hugetlb: fix missing hugetlb_lock for resv uncharge

[Oscar Salvador]

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