On 16/12/2024 13:07, Arunpravin Paneer Selvam wrote:
- Added a testcase to verify the multiroot force merge fini.
- Added a new field in_use to track the mm freed status.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@xxxxxxx>
Signed-off-by: Lin.Cao <lincao12@xxxxxxx>
---
drivers/gpu/drm/drm_buddy.c | 20 ++++++++++++++++-
drivers/gpu/drm/tests/drm_buddy_test.c | 30 ++++++++++++++++++--------
include/drm/drm_buddy.h | 2 ++
3 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
index ca42e6081d27..39ce918b3a65 100644
--- a/drivers/gpu/drm/drm_buddy.c
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -102,6 +102,18 @@ static inline bool contains(u64 s1, u64 e1, u64 s2, u64 e2)
return s1 <= s2 && e1 >= e2;
}
+static bool is_roots_freed(struct drm_buddy *mm)
+{
+ int i;
+
+ for (i = 0; i < mm->n_roots; ++i) {
+ if (!drm_buddy_block_is_free(mm->roots[i]))
+ return false;
+ }
+
+ return true;
+}
+
static struct drm_buddy_block *
__get_buddy(struct drm_buddy_block *block)
{
@@ -303,6 +315,8 @@ int drm_buddy_init(struct drm_buddy *mm, u64 size, u64 chunk_size)
i++;
} while (size);
+ mm->in_use = true;
+
return 0;
out_free_roots:
@@ -335,13 +349,17 @@ void drm_buddy_fini(struct drm_buddy *mm)
start = drm_buddy_block_offset(mm->roots[i]);
__force_merge(mm, start, start + size, order);
- WARN_ON(!drm_buddy_block_is_free(mm->roots[i]));
So does this warn not pop? Or it does but kunit ignores it or something?
drm_block_free(mm, mm->roots[i]);
root_size = mm->chunk_size << order;
size -= root_size;
}
+ mm->in_use = false;
+
+ if (WARN_ON(!is_roots_freed(mm)))
This looks like UAF under normal operation, since each root pointer
within mm->roots is already gone.
How about something like this:
+ #include <kunit/test-bug.h>
+
#include <linux/kmemleak.h>
#include <linux/module.h>
#include <linux/sizes.h>
@@ -335,7 +337,9 @@ void drm_buddy_fini(struct drm_buddy *mm)
start = drm_buddy_block_offset(mm->roots[i]);
__force_merge(mm, start, start + size, order);
- WARN_ON(!drm_buddy_block_is_free(mm->roots[i]));
+ if (WARN_ON(!drm_buddy_block_is_free(mm->roots[i])))
+ kunit_fail_current_test("buddy_fini() root");
+
drm_block_free(mm, mm->roots[i]);
root_size = mm->chunk_size << order;
And then also drop the in_use stuff. As a follow up could do that for
all warnings in this file that don't result in error being returned to
the caller...
+ mm->in_use = true;
+
WARN_ON(mm->avail != mm->size);
...like this one.
> > kfree(mm->roots);
diff --git a/drivers/gpu/drm/tests/drm_buddy_test.c b/drivers/gpu/drm/tests/drm_buddy_test.c
index 9662c949d0e3..694b058ddd6d 100644
--- a/drivers/gpu/drm/tests/drm_buddy_test.c
+++ b/drivers/gpu/drm/tests/drm_buddy_test.c
@@ -385,19 +385,31 @@ static void drm_test_buddy_alloc_clear(struct kunit *test)
drm_buddy_fini(&mm);
/*
- * Create a new mm with a non power-of-two size. Allocate a random size, free as
- * cleared and then call fini. This will ensure the multi-root force merge during
- * fini.
+ * Create a new mm with a non power-of-two size. Allocate a random size from each
+ * root, free as cleared and then call fini. This will ensure the multi-root
+ * force merge during fini.
*/
- mm_size = 12 * SZ_4K;
- size = max(round_up(prandom_u32_state(&prng) % mm_size, ps), ps);
+ mm_size = (SZ_4K << max_order) + (SZ_4K << (max_order - 2));
+
KUNIT_EXPECT_FALSE(test, drm_buddy_init(&mm, mm_size, ps));
- KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, mm_size,
- size, ps, &allocated,
- DRM_BUDDY_TOPDOWN_ALLOCATION),
- "buddy_alloc hit an error size=%u\n", size);
+ KUNIT_EXPECT_EQ(test, mm.max_order, max_order);
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, SZ_4K << max_order,
+ 4 * ps, ps, &allocated,
+ DRM_BUDDY_RANGE_ALLOCATION),
+ "buddy_alloc hit an error size=%lu\n", 4 * ps);
+ drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, 0, SZ_4K << max_order,
+ 2 * ps, ps, &allocated,
+ DRM_BUDDY_CLEAR_ALLOCATION),
+ "buddy_alloc hit an error size=%lu\n", 2 * ps);
+ drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
+ KUNIT_ASSERT_FALSE_MSG(test, drm_buddy_alloc_blocks(&mm, SZ_4K << max_order, mm_size,
+ ps, ps, &allocated,
+ DRM_BUDDY_RANGE_ALLOCATION),
+ "buddy_alloc hit an error size=%lu\n", ps);
drm_buddy_free_list(&mm, &allocated, DRM_BUDDY_CLEARED);
drm_buddy_fini(&mm);
+ KUNIT_EXPECT_EQ(test, mm.in_use, false);
}
static void drm_test_buddy_alloc_contiguous(struct kunit *test)
diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h
index 9689a7c5dd36..d692d831ffdd 100644
--- a/include/drm/drm_buddy.h
+++ b/include/drm/drm_buddy.h
@@ -86,6 +86,8 @@ struct drm_buddy {
unsigned int n_roots;
unsigned int max_order;
+ bool in_use;
+
/* Must be at least SZ_4K */
u64 chunk_size;
u64 size;
