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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251231070029.79682-1-sj@kernel.org>
Date: Tue, 30 Dec 2025 23:00:22 -0800
From: SeongJae Park <sj@...nel.org>
To: Aaron Yang <yangqixiao@...pur.com>
Cc: SeongJae Park <sj@...nel.org>,
	linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org,
	linux-mm@...ck.org
Subject: Re: [PATCH v1] mm/damon/pa: initialize 'folio' to NULL to prevent use of uninitialized value

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@...pur.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

[...]