Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Dev Jain <dev.jain@arm.com>
To: Lorenzo Stoakes <ljs@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Suren Baghdasaryan <surenb@google.com>,
	"Liam R. Howlett" <liam@infradead.org>,
	Vlastimil Babka <vbabka@kernel.org>,
	Shakeel Butt <shakeel.butt@linux.dev>,
	David Hildenbrand <david@kernel.org>,
	Mike Rapoport <rppt@kernel.org>, Michal Hocko <mhocko@suse.com>,
	Uladzislau Rezki <urezki@gmail.com>,
	Toshi Kani <toshi.kani@hpe.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, David Carlier <devnexen@gmail.com>,
	Ryan Roberts <ryan.roberts@arm.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, stable@vger.kernel.org,
	syzbot+fd95a72470f5a44e464c@syzkaller.appspotmail.com
Subject: Re: [PATCH 0/2] mm: fix UAF caused by race between ptdump and vmap pgtable freeing
Date: Sun, 12 Jul 2026 17:04:17 +0530	[thread overview]
Message-ID: <8d109bba-4a8b-4d2e-9b3b-7c79441f7a39@arm.com> (raw)
In-Reply-To: <alNQccqtx5-QApup@lucifer>



On 12/07/26 2:16 pm, Lorenzo Stoakes wrote:
> On Sun, Jul 12, 2026 at 12:50:08PM +0530, Dev Jain wrote:
>> Will Deacon had pushed back on a similar approach:
>> https://lore.kernel.org/all/20250530123527.GA30463@willie-the-truck/
>>
>> Although now when I read back that thread, it feels more so like my
>> incompetency to convince :) because:
> 
> No haha not so, I think more like this stuff is fiddly.
> 
>>
>> 1. I don't think this pmd_free_pte_page() path is a hot path at all
> 
> Right, and we don't actually alter that path anyway
> 
>>
>> 2. We are doing a try lock which is almost guaranteed to succeed,
>>    so it's not like we are losing out on block mappings
> 
> Also it's specifically only on when vmap tries to make a mapping huge, and
> this path is being inconsistent with a convention that already existed - if
> you manipulate kernel page table mappings that can interact with other page
> table walkers, you have to take the init_mm mmap lock.
> 
>>
>> 3. Any overhead from the try lock will get dominated by the pgtable
>>    page free/TLB flush
> 
> Yup.
> 
>>
>> I guess you did not take the RCU approach because that would put code
>> into the generic kernel pgtable freeing path.
> 
> Well a number of reasons:
> 
> * firstly yes it makes the code path always RCU only to suit a specific
>   debug user as you say :)
> 
> * Importantly - we risk genuine RCU stall issues, because the ptdump then
>   has to be RCU too over vast ranges.
> 
>   To work around that you have to shard the ptdump walk, make an assumption
>   all callbacks are RCU-safe, and that the sharding suffices to avoid these
>   stalls.
> 
>   It's a ton of complexity and assumptions to account for... vmalloc doing
>   the wrong thing.
> 
> * It is an established precedent that we mmap lock init_mm for kernel page
>   table walking as per mm/pagewalk.c. It'd require significant rework there
>   and would disallow any future walkers like this if we were to require
>   RCU.
> 
> * The mmap lock approach is simple, safe, and as you say is only actually
>   required in code paths that manipulate page tables and thus are already
>   not hotpaths.
> 
> * If there's future work to free vmalloc page tables upon vunmap()
>   (currently it does not), we have a stable, established basis for doing so
>   that again puts the weight of the work on the operation being performed
>   rather than anything else.
> 
>>
>> I liked the RCU approach because I hate the fact that ptdump takes
>> an mmap_write_lock when it is literally only reading the pgtables.
> 
> Well you have to do that for the userland side, because there could be a
> concurrent downgraded mmap read lock during an munmap, and the same goes
> for non-VMA kernel ranges too, so it would have to keep doing that
> regardless.

Oh right, I didn't know x86 was using ptdump for user tables too.


> 
>> But your approach is simpler and fixes the problem at the particular spot
>> and not hammers the fix into a generic path. So overall, ACK.
> 
> Thanks!
> 
>>
>>
>>> Lorenzo Stoakes (2):
>>>       mm/vmalloc: acquire init_mm read lock on huge vmap promotion
>>>       Revert "arm64: Enable vmalloc-huge with ptdump"
>>>
>>>  arch/arm64/include/asm/ptdump.h |  2 --
>>>  arch/arm64/mm/mmu.c             | 43 ++++-------------------------------------
>>>  arch/arm64/mm/ptdump.c          | 11 ++---------
>>>  include/linux/mmap_lock.h       |  1 +
>>>  mm/pagewalk.c                   | 22 +++++++++++----------
>>>  mm/vmalloc.c                    | 41 ++++++++++++++++++++++++++++++---------
>>>  6 files changed, 51 insertions(+), 69 deletions(-)
>>> ---
>>> base-commit: a635d6748234582ea287c5ffeae28b9b23f91c7e
>>> change-id: 20260710-series-vmap-race-fix-2a4cac988938
>>>
>>> Cheers,
>>
> 
> Cheers, Lorenzo



      reply	other threads:[~2026-07-12 11:34 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10 10:49 [PATCH 0/2] mm: fix UAF caused by race between ptdump and vmap pgtable freeing Lorenzo Stoakes
2026-07-10 10:49 ` [PATCH 1/2] mm/vmalloc: acquire init_mm read lock on huge vmap promotion Lorenzo Stoakes
2026-07-10 12:57   ` Lorenzo Stoakes
2026-07-10 13:24     ` Lorenzo Stoakes
2026-07-11 10:23   ` Mike Rapoport
2026-07-12  7:43   ` Dev Jain
2026-07-12  8:01     ` Greg KH
2026-07-12  9:27       ` Lorenzo Stoakes
2026-07-12 11:15       ` Dev Jain
2026-07-12  8:22     ` Lorenzo Stoakes
2026-07-12 11:16       ` Dev Jain
2026-07-10 10:49 ` [PATCH 2/2] Revert "arm64: Enable vmalloc-huge with ptdump" Lorenzo Stoakes
2026-07-11 10:15   ` Mike Rapoport
2026-07-12  8:27     ` Lorenzo Stoakes
2026-07-12  7:46   ` Dev Jain
2026-07-12  8:28     ` Lorenzo Stoakes
2026-07-10 11:44 ` [PATCH 0/2] mm: fix UAF caused by race between ptdump and vmap pgtable freeing David CARLIER
2026-07-10 11:57   ` Lorenzo Stoakes
2026-07-12  7:20 ` Dev Jain
2026-07-12  8:46   ` Lorenzo Stoakes
2026-07-12 11:34     ` Dev Jain [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8d109bba-4a8b-4d2e-9b3b-7c79441f7a39@arm.com \
    --to=dev.jain@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=catalin.marinas@arm.com \
    --cc=david@kernel.org \
    --cc=devnexen@gmail.com \
    --cc=liam@infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=rppt@kernel.org \
    --cc=ryan.roberts@arm.com \
    --cc=shakeel.butt@linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.com \
    --cc=syzbot+fd95a72470f5a44e464c@syzkaller.appspotmail.com \
    --cc=toshi.kani@hpe.com \
    --cc=urezki@gmail.com \
    --cc=vbabka@kernel.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox