Re: [PATCH 1/3] mm/mmu_notifier: let interval notifiers block THP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Bypass of THP protections via VMA modification and logic bug in hugepage_madvise().
- [High] mmu_interval_notifier_range_block_thp() misses notifiers on the deferred_list, allowing a concurrent MADV_HUGEPAGE to re-enable THP.
- [Medium] MADV_HUGEPAGE requests are silently dropped for valid memory regions due to coarse VMA-level overlap checks.
- [Medium] VMAs with the VM_NO_KHUGEPAGED flag are incorrectly skipped during THP blocking, allowing synchronous THP allocations.
--

commit 70ad5b57ebfe62382bb716c4e65db9bc1f0ee638
Author: Yitao Jiang <jytscientist@hotmail.com>
Subject: mm/mmu_notifier: let interval notifiers block THP

This patch introduces MMU_INTERVAL_NOTIFIER_BLOCK_THP to allow interval
notifiers to request blocking of transparent huge pages on covered VMA ranges.
The MM core handles updating the VMA flags (setting VM_NOHUGEPAGE and clearing
VM_HUGEPAGE) during registration instead of drivers doing it directly.

> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 617bca76db49b..a9b05e71686c5 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
[ ... ]
> @@ -445,11 +445,16 @@ static unsigned int collapse_max_ptes_swap(struct collapse_control *cc,
>  	return khugepaged_max_ptes_swap;
>  }
>  
> -int hugepage_madvise(struct vm_area_struct *vma,
> -		     vm_flags_t *vm_flags, int advice)
> +int hugepage_madvise(struct vm_area_struct *vma, unsigned long start,
> +		     unsigned long end, vm_flags_t *vm_flags, int advice)
>  {
>  	switch (advice) {
>  	case MADV_HUGEPAGE:
> +		if ((*vm_flags & VM_NOHUGEPAGE) &&
> +		    mmu_interval_notifier_range_block_thp(vma->vm_mm,
> +							  start, end))
> +			return 0;
> +

[Severity: High]
If a new VMA is created within the notifier's range (e.g., via mremap or a
new mmap with MAP_FIXED), it will be created with default flags lacking
VM_NOHUGEPAGE, because mmu_interval_notifier_block_thp_locked() only
statically applies flags to existing VMAs. Because this check short-circuits
when VM_NOHUGEPAGE is absent, it seems it would evaluate to false and allow
MADV_HUGEPAGE to succeed. Does this allow userspace to bypass the THP blocking
protection for newly created VMAs?

[Severity: Medium]
Additionally, if a VMA was previously marked with VM_NOHUGEPAGE before the
notifier was registered, it may not have been split if its flags already
matched. If a user later calls MADV_HUGEPAGE on that entire VMA, this
condition will return 0 and ignore the hint completely. Should this logic
only reject the hint for the specific regions overlapping the notifier,
rather than dropping valid MADV_HUGEPAGE requests for the rest of the VMA?

> diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
> index 245b74f39f912..852a5682b07f2 100644
> --- a/mm/mmu_notifier.c
> +++ b/mm/mmu_notifier.c
[ ... ]
> +bool mmu_interval_notifier_range_block_thp(struct mm_struct *mm,
> +					   unsigned long start,
> +					   unsigned long end)
> +{
> +	struct mmu_notifier_subscriptions *subscriptions;
> +	struct mmu_interval_notifier *interval_sub;
> +	struct interval_tree_node *node;
> +	bool block_thp = false;
> +
> +	if (start >= end)
> +		return false;
> +
> +	/* Pairs with the store in mmu_notifier_register(). */
> +	subscriptions = smp_load_acquire(&mm->notifier_subscriptions);
> +	if (!subscriptions || !subscriptions->has_itree)
> +		return false;
> +
> +	spin_lock(&subscriptions->lock);
> +	for (node = interval_tree_iter_first(&subscriptions->itree, start,
> +					     end - 1);
> +	     node;
> +	     node = interval_tree_iter_next(node, start, end - 1)) {
> +		interval_sub = container_of(node, struct mmu_interval_notifier,
> +					    interval_tree);
> +		if (interval_sub->flags & MMU_INTERVAL_NOTIFIER_BLOCK_THP) {
> +			block_thp = true;
> +			break;
> +		}
> +	}
> +	spin_unlock(&subscriptions->lock);
> +
> +	return block_thp;
> +}

[Severity: High]
When an interval notifier is registered concurrently with an active page
table invalidation, __mmu_interval_notifier_insert() defers tree insertion
by placing the notifier on subscriptions->deferred_list. Since this loop
only inspects subscriptions->itree, could it miss active notifiers on the
deferred list and mistakenly allow a concurrent MADV_HUGEPAGE request to
re-enable THP?

[ ... ]
> +static int
> +mmu_interval_notifier_block_thp_locked(struct mm_struct *mm,
> +				       unsigned long start,
> +				       unsigned long end)
> +{
> +	struct vm_area_struct *vma, *prev;
> +	struct vma_iterator vmi;
> +
> +	mmap_assert_write_locked(mm);
> +
> +	vma_iter_init(&vmi, mm, start);
> +	vma = vma_iter_load(&vmi);
> +	prev = vma_prev(&vmi);
> +	if (vma && start > vma->vm_start)
> +		prev = vma;
> +
> +	for_each_vma_range(vmi, vma, end) {
> +		const unsigned long curr_start = max(vma->vm_start, start);
> +		const unsigned long curr_end = min(vma->vm_end, end);
> +		vma_flags_t new_flags;
> +
> +		if (vma->vm_flags & VM_NO_KHUGEPAGED)
> +			goto next;

[Severity: Medium]
The VM_NO_KHUGEPAGED flag only disables background scanning by khugepaged,
but it does not prevent transparent huge pages from being synchronously
allocated and mapped during page faults. By skipping these VMAs and not
setting VM_NOHUGEPAGE, does this leave them vulnerable to synchronous THP
allocations, thereby breaking the protection required by the notifier?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/SY1PR01MB10596EB75463208A8E1EBBA0FC0EC2@SY1PR01MB10596.ausprd01.prod.outlook.com?part=1