[PATCH v1] mm/damon/pa: initialize 'folio' to NULL to prevent use of uninitialized value

[PATCH v1] mm/damon/pa: initialize 'folio' to NULL to prevent use of uninitialized value

[Aaron Yang]

In damon_pa_mark_accessed_or_deactivate(), the local variable 'folio'
is declared but not initialized. If the region [r->ar.start, r->ar.end)
is empty or invalid such that the while-loop body is never entered,
'folio' retains an indeterminate (garbage) value. The function then
unconditionally assigns this uninitialized pointer to s->last_applied
(line 239), resulting in undefined behavior. Subsequent dereference or
folio_put() on s->last_applied may cause crashes or memory corruption.

Although DAMON regions are typically non-empty, zero-length regions
can arise during region merging/splitting or due to address unit
alignment — making this path reachable in practice.

Fix by initializing 'folio' to NULL. Assigning NULL to s->last_applied
is safe and semantically correct: it cleanly indicates "no folio was
processed in this invocation", and callers are expected to check for
NULL before use (as per common kernel practice).

No functional change for non-empty regions; only hardens error/edge
case handling.

Signed-off-by: Aaron Yang <yangqixiao@inspur.com>
---
 mm/damon/paddr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index 07a8aead439e..32d8024d130e 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -212,7 +212,7 @@ static inline unsigned long damon_pa_mark_accessed_or_deactivate(
 		unsigned long *sz_filter_passed)
 {
 	phys_addr_t addr, applied = 0;
-	struct folio *folio;
+	struct folio *folio = NULL;
 
 	addr = damon_pa_phys_addr(r->ar.start, addr_unit);
 	while (addr < damon_pa_phys_addr(r->ar.end, addr_unit)) {
-- 
2.47.3

Re: [PATCH v1] mm/damon/pa: initialize 'folio' to NULL to prevent use of uninitialized value

[SeongJae Park]

Hello Aaron,


Thank you for sending this patch!

For the patch subject, let's use 'mm/damon/paddr:' as the prefix of the
subject, for making it consistent with other past commits for the paddr.c file.

On Wed, 31 Dec 2025 13:57:37 +0800 Aaron Yang <yangqixiao@inspur.com> wrote:

> In damon_pa_mark_accessed_or_deactivate(), the local variable 'folio'
> is declared but not initialized. If the region [r->ar.start, r->ar.end)
> is empty or invalid such that the while-loop body is never entered,
> 'folio' retains an indeterminate (garbage) value. The function then
> unconditionally assigns this uninitialized pointer to s->last_applied
> (line 239), resulting in undefined behavior. Subsequent dereference or
> folio_put() on s->last_applied may cause crashes or memory corruption.

Nice finding!

> 
> Although DAMON regions are typically non-empty, zero-length regions
> can arise during region merging/splitting or due to address unit
> alignment — making this path reachable in practice.

But, can zero-length regions be made in real?  damon_ctx->min_sz_region
disallows DAMON having regions of size less that it, and all DAMON API callers
set it at least 1.  That is, DAMON code is at least intentionally designed to
prohibit zero-length regions.  And many parts of DAMON including
damon_pa_mark_accessed_or_deactivate() are written under the assumption that
there is no zero-length regions.  If there is a case that allows zero-length
regions, all such parts could trigger problems.

> 
> Fix by initializing 'folio' to NULL. Assigning NULL to s->last_applied
> is safe and semantically correct: it cleanly indicates "no folio was
> processed in this invocation", and callers are expected to check for
> NULL before use (as per common kernel practice).

For the above mentioned reason, I'd like to fix the code that allows
zero-length regions, rather than making only this single function safe from the
bug.

So, if you found a case that allows zero-length DAMON regions, could you please
share the reproduction steps of it, or fix of it?

If you didn't find a real case that allows zero-length DAMON regions but was
thinking it might be theoretically possible, maybe the poor documentation of
DAMON has confused you to believe so.  If that's the case, how about making it
clear on the documentation?  Specifically, I think damon_region kernel-doc
comment in include/linux/damon.h could be a good place to make this explicitly
explained.


Thanks,
SJ

[...]