damos_new_filter() is returning a damos_filter struct without initializing its ->list field. And the users of the function uses the struct without initializing the field. As a result, uninitialized memory access error is possible. Actually, a kernel NULL pointer dereference BUG can be triggered using DAMON user-space tool, like below.
# damo start --damos_action stat --damos_filter anon matching # damo tune --damos_action stat --damos_filter anon matching --damos_filter anon nomatching # dmesg [...] [ 36.908136] BUG: kernel NULL pointer dereference, address: 0000000000000008 [ 36.910483] #PF: supervisor write access in kernel mode [ 36.912238] #PF: error_code(0x0002) - not-present page [ 36.913415] PGD 0 P4D 0 [ 36.913978] Oops: 0002 [#1] PREEMPT SMP PTI [ 36.914878] CPU: 32 PID: 1335 Comm: kdamond.0 Not tainted 6.5.0-rc3-mm-unstable-damon+ #1 [ 36.916621] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014 [ 36.919051] RIP: 0010:damos_destroy_filter (include/linux/list.h:114 include/linux/list.h:137 include/linux/list.h:148 mm/damon/core.c:345 mm/damon/core.c:355) [...] [ 36.938247] Call Trace: [ 36.938721] <TASK> [...] [ 36.950064] ? damos_destroy_filter (include/linux/list.h:114 include/linux/list.h:137 include/linux/list.h:148 mm/damon/core.c:345 mm/damon/core.c:355) [ 36.950883] ? damon_sysfs_set_scheme_filters.isra.0 (mm/damon/sysfs-schemes.c:1573) [ 36.952019] damon_sysfs_set_schemes (mm/damon/sysfs-schemes.c:1674 mm/damon/sysfs-schemes.c:1686) [ 36.952875] damon_sysfs_apply_inputs (mm/damon/sysfs.c:1312 mm/damon/sysfs.c:1298) [ 36.953757] ? damon_pa_check_accesses (mm/damon/paddr.c:168 mm/damon/paddr.c:179) [ 36.954648] damon_sysfs_cmd_request_callback (mm/damon/sysfs.c:1329 mm/damon/sysfs.c:1359) [...]
The first patch of this patchset fixes the bug by initializing the field in damos_new_filter(). The second patch adds a unit test for the problem. Note that the second patch Cc stable@ without Fixes: tag, since it would be better to be ingested together for avoiding any future regression.
SeongJae Park (2): mm/damon/core: initialize damo_filter->list from damos_new_filter() mm/damon/core-test: add a test for damos_new_filter()
mm/damon/core-test.h | 13 +++++++++++++ mm/damon/core.c | 1 + 2 files changed, 14 insertions(+)
damos_new_filter() is not initializing the list field of newly allocated filter object. However, DAMON sysfs interface and DAMON_RECLAIM are not initializing it after calling damos_new_filter(). As a result, accessing uninitialized memory is possible. Actually, adding multiple DAMOS filters via DAMON sysfs interface caused NULL pointer dereferencing. Initialize the field just after the allocation from damos_new_filter().
Fixes: 98def236f63c ("mm/damon/core: implement damos filter") Cc: stable@vger.kernel.org # 6.3.x- Signed-off-by: SeongJae Park sj@kernel.org --- mm/damon/core.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/mm/damon/core.c b/mm/damon/core.c index 91cff7f2997e..eb9580942a5c 100644 --- a/mm/damon/core.c +++ b/mm/damon/core.c @@ -273,6 +273,7 @@ struct damos_filter *damos_new_filter(enum damos_filter_type type, return NULL; filter->type = type; filter->matching = matching; + INIT_LIST_HEAD(&filter->list); return filter; }
damos_new_filter() was having a bug that not initializing ->list field of the returning damos_filter struct, which results in access to uninitialized memory. Add a unit test for the function.
Cc: stable@vger.kernel.org # 6.3.x- Signed-off-by: SeongJae Park sj@kernel.org --- mm/damon/core-test.h | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/mm/damon/core-test.h b/mm/damon/core-test.h index bb07721909e1..4bddbfe243c3 100644 --- a/mm/damon/core-test.h +++ b/mm/damon/core-test.h @@ -341,6 +341,18 @@ static void damon_test_set_attrs(struct kunit *test) KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL); }
+static void damos_test_new_filter(struct kunit *test) +{ + struct damos_filter *filter; + + filter = damos_new_filter(DAMOS_FILTER_TYPE_ANON, true); + KUNIT_EXPECT_EQ(test, filter->type, DAMOS_FILTER_TYPE_ANON); + KUNIT_EXPECT_EQ(test, filter->matching, true); + KUNIT_EXPECT_PTR_EQ(test, filter->list.prev, &filter->list); + KUNIT_EXPECT_PTR_EQ(test, filter->list.next, &filter->list); + damos_destroy_filter(filter); +} + static struct kunit_case damon_test_cases[] = { KUNIT_CASE(damon_test_target), KUNIT_CASE(damon_test_regions), @@ -353,6 +365,7 @@ static struct kunit_case damon_test_cases[] = { KUNIT_CASE(damon_test_set_regions), KUNIT_CASE(damon_test_update_monitoring_result), KUNIT_CASE(damon_test_set_attrs), + KUNIT_CASE(damos_test_new_filter), {}, };
linux-stable-mirror@lists.linaro.org
-
SeongJae Park