All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Lian Wang" <lianux.mm@gmail.com>
Cc: damon@lists.linux.dev
Subject: Re: [RESEND RFC PATCH v2 5/5] mm/damon/vaddr: implement DAMOS_SPLIT handler
Date: Thu, 02 Jul 2026 10:30:13 +0000	[thread overview]
Message-ID: <20260702103014.7B1581F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260702095227.75866-6-lianux.mm@gmail.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect address range expansion and iteration logic in `damos_va_split()` causes out-of-bounds folio processing and skips memory.
- [High] `damos_va_split()` calls `split_folio_to_order()` on mapped compound pages without verifying they are valid LRU folios, leading to kernel crashes.
--

commit 1aa5abadd8058bacb6959d3c7864cfd3e927e6df
Author: Lian Wang <lianux.mm@gmail.com>

mm/damon/vaddr: implement DAMOS_SPLIT handler

This commit implements the vaddr operations layer handler for DAMOS_SPLIT.
It iterates over the target region and attempts to split any large folios
that exceed the scheme's target order.

> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index 8bfc6dcc635d5..73cb61c655fc7 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -940,6 +940,83 @@ static unsigned long damos_va_collapse(struct damon_target *target,
>  	return applied;
>  }
>  
> +static unsigned long damos_va_split(struct damon_target *target,
> +		struct damon_region *r, struct damos *s,
> +		unsigned long *sz_filter_passed)
> +{
> +	unsigned long addr, end, chunk_sz;
> +	unsigned int target_order = s->target_order;
> +	unsigned long applied = 0;
> +	struct mm_struct *mm;
> +	struct vm_area_struct *vma;
> +	struct folio *folio;
> +	struct folio_walk fw;
> +
> +	mm = damon_get_mm(target);
> +	if (!mm)
> +		return 0;
> +
> +	chunk_sz = PAGE_SIZE << HPAGE_PMD_ORDER;
> +	addr = ALIGN_DOWN(r->ar.start, chunk_sz);
> +	end = ALIGN(r->ar.end, chunk_sz);

[Severity: High]
Does aligning the start and end to 2MB boundaries risk processing memory outside
the requested target region? It seems this could expand the iteration beyond
the boundaries defined by r->ar.start and r->ar.end.

> +	if (end < addr)
> +		goto out_mmput;
> +
> +	while (addr < end) {
> +		unsigned long folio_sz;
> +
> +		if (addr + chunk_sz < addr)
> +			break;
> +
> +		mmap_read_lock(mm);
> +		vma = find_vma(mm, addr);
> +
> +		if (!vma || addr < vma->vm_start ||
> +		    vma->vm_flags & (VM_HUGETLB | VM_MIXEDMAP))
> +			goto unlock;
> +
> +		folio = folio_walk_start(&fw, vma, addr, 0);
> +		if (!folio)
> +			goto unlock;
> +
> +		folio_sz = PAGE_SIZE << folio_order(folio);
> +
> +		if (folio_order(folio) > target_order) {
> +			if (!folio_trylock(folio)) {
> +				folio_walk_end(&fw, vma);
> +				goto unlock;
> +			}
> +			folio_get(folio);
> +			folio_walk_end(&fw, vma);
> +
> +			if (!split_folio_to_order(folio, target_order))

[Severity: High]
Could this call to split_folio_to_order() encounter driver-allocated compound
pages? 

While VM_MIXEDMAP and VM_HUGETLB are filtered out, other normal VMAs
might still be backed by compound pages that are not fully initialized LRU
folios. Without checking if the folio is actually a valid LRU folio (e.g., via
folio_test_lru()), this might dereference uninitialized fields during the split
and cause kernel crashes.

> +				applied += folio_sz;
> +
> +			folio_unlock(folio);
> +			folio_put(folio);
> +			*sz_filter_passed += folio_sz;
> +			addr += folio_sz;
> +		} else {
> +			folio_walk_end(&fw, vma);
> +			*sz_filter_passed += chunk_sz;
> +			addr += chunk_sz;

[Severity: High]
When the folio order is smaller than or equal to the target order, does advancing
addr by a full 2MB chunk miss the remaining folios within that chunk? 

If addr was advanced by folio_sz in a previous loop iteration, adding chunk_sz
here might overshoot and skip memory.

> +		}
> +		mmap_read_unlock(mm);
> +		cond_resched();
> +		continue;
> +
> +unlock:
> +		*sz_filter_passed += chunk_sz;
> +		addr += chunk_sz;

[Severity: High]
Similarly, does advancing by chunk_sz on error paths (like missing VMAs, holes,
or lock failures) skip intermediate memory and artificially inflate the
sz_filter_passed statistic?

> +		mmap_read_unlock(mm);
> +		cond_resched();
> +	}
> +
> +out_mmput:
> +	mmput(mm);
> +	return applied;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702095227.75866-1-lianux.mm@gmail.com?part=5

  reply	other threads:[~2026-07-02 10:30 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-02  9:52 [RESEND RFC PATCH v2 0/5] mm/damon: add mTHP collapse and split actions Lian Wang
2026-07-02  9:52 ` [RESEND RFC PATCH v2 1/5] mm/damon: add target_order field for DAMOS_COLLAPSE Lian Wang
2026-07-02 10:02   ` sashiko-bot
2026-07-02  9:52 ` [RESEND RFC PATCH v2 2/5] mm/khugepaged: add damon_collapse_folio_range() for external callers Lian Wang
2026-07-02 10:08   ` sashiko-bot
2026-07-02  9:52 ` [RESEND RFC PATCH v2 3/5] mm/damon/vaddr: implement mTHP-aware DAMOS_COLLAPSE handler Lian Wang
2026-07-02 10:27   ` sashiko-bot
2026-07-02  9:52 ` [RESEND RFC PATCH v2 4/5] mm/damon: introduce DAMOS_SPLIT action Lian Wang
2026-07-02 10:21   ` sashiko-bot
2026-07-02  9:52 ` [RESEND RFC PATCH v2 5/5] mm/damon/vaddr: implement DAMOS_SPLIT handler Lian Wang
2026-07-02 10:30   ` sashiko-bot [this message]
2026-07-02 16:39 ` [RESEND RFC PATCH v2 0/5] mm/damon: add mTHP collapse and split actions SJ Park
  -- strict thread matches above, loose matches on Subject: below --
2026-07-02  9:46 Lian Wang
2026-07-02  9:46 ` [RESEND RFC PATCH v2 5/5] mm/damon/vaddr: implement DAMOS_SPLIT handler Lian Wang
2026-07-02 10:49   ` sashiko-bot
2026-07-02 20:18   ` SJ Park

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=20260702103014.7B1581F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=lianux.mm@gmail.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.