Re: [PATCH 01/10] mm/vma: don't leak the commit charge on mmap failure

[David Hildenbrand (Arm)]

On 10/15/26 19:54, Pedro Falcato wrote:
> On Thu, Oct 15, 2026 at 06:07:07PM +0200, David Hildenbrand (Arm) wrote:
>> From: Josef Bacik <josef@xxxxxxxxxxxxxx>
>>
>> When an accountable mapping replaces existing mappings (MAP_FIXED),
>> __mmap_setup() only charges the difference between the size of the new
>> mapping and the VM_ACCOUNT pages it replaces. It then zeroes
>> vms->nr_accounted so that unmapping the old VMAs does not return their
>> charge, which now backs the new mapping.
>>
>> If the operation fails after that point, the error path returns
>> map->charged, i.e. only the difference. By then vms_clean_up_area() has
>> zapped the page tables and closed the old VMAs, so
>> vms_abort_munmap_vmas() cannot reattach them. It leaves a gap and
>> finishes the unmap with vms_complete_munmap_vmas(), which uncharges
>> vms->nr_accounted == 0 pages. The old VMAs are gone, the new one was
>> never created, and the charge of the old ones is never returned, not
>> even when the process exits.
>>
>> No privilege is needed to hit this, only a file whose mmap hook fails,
>> for example a socket (sock_no_mmap() returns -ENODEV):
>>
>> 	p = mmap(NULL, len, PROT_READ | PROT_WRITE,
>> 		 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
>> 	mmap(p, len, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_FIXED,
>> 	     socket_fd, 0);
>>
>> Every iteration adds len to Committed_AS for good. With
>> vm.overcommit_memory=2 this lets any user make all further accountable
>> allocations in the system fail with -ENOMEM.
> 
> I don't think is urgent nor does it have security implications. You can
> already broadly screw up the system from any user when overcommit_memory=2.
> This just goes around the single-process user_reserve_kbytes AFAICT.
> Am I missing something?
> 
> Just looks like a good old bug to me.

Yes, most of the things here are just good old bugs with limited criticality (as
I expressed in the cover letter and the "urgent?" highlight in the subject).

Things like use-after-free or deadlocks are rather bad, but they are mostly
hugetlb specific, requiring actual allocated hugetlb pages in the pool. So there
is limited attack surface in practice.

All valid things we should be fixing, though.

I suspect we have a flood of MM fixes coming our way with differing criticality,
so this here is one attempt to figure out how well we can deal with the
mostly-trivial ones in a fairly controlled (e.g., proper human review) yet
efficient way.

Anything less trivial or shaky needs proper time to settle, as always.

-- 
Cheers,

David