[PATCH v3] drm_buddy: fix power-of-2 rounding errs

[PATCH v3] drm_buddy: fix power-of-2 rounding errs

[David Gow]

The standard roundup_pow_of_two() and rounddown_pow_of_two() macros use
unsigned long internally, which on 32-bit architectures (like arm32) is
a 32-bit type.

drm_test_buddy_alloc_exceeds_max_order() uses these on a u64 value,
where they silently truncate the 10GB allocation, giving unexpected
success in DRM-CI.  (see below the snip).

Fix this by replacing the those macros with safe 64-bit power-of-two
calculations using ilog2().

Fixes: 0a1844bf0b532 ("drm/buddy: Improve contiguous memory allocation")
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: David Gow <david@davidgow.net>
---

This is a straightforward rebase of patch 39 of v12 of this series:
https://lore.kernel.org/all/20260326185413.1205870-40-jim.cromie@gmail.com/

The only changes are updating the buddy path (as it's no longer a part of
DRM), and adding the Fixes tag.

This is a fairly hacky solution: I've got a nicer series which adds clearer
helpers and generally cleans up all of these macros on 32-bit systems, but
it's much larger (and growing as more things need cleaning up), so I'd
rather have this more urgent fix go in immediately, and save the cleanup
for a less-urgent follow-up series:
https://lore.kernel.org/all/20260830103321.2042968-1-david@davidgow.net/

As noted in the original series, this is causing KUnit test failures on
all 32-bit architectures:
[09:01:26]     # gpu_test_buddy_alloc_exceeds_max_order: EXPECTATION FAILED at drivers/gpu/tests/gpu_buddy_test.c:1429
[09:01:26]     Expected err == -22, but
[09:01:26]         err == 0 (0x0)
[09:01:26] WARNING: drivers/gpu/buddy.c:508 at gpu_buddy_fini+0x244/0x2e0, CPU#0: kunit_try_catch/1595
[09:01:26]     # gpu_test_buddy_alloc_exceeds_max_order: drivers/gpu/buddy.c:508: gpu_buddy_assert(gpu_buddy_block_is_free(mm->roots[i]))
[09:01:26] WARNING: drivers/gpu/buddy.c:516 at gpu_buddy_fini+0x284/0x2e0, CPU#0: kunit_try_catch/1595
[09:01:26]     # gpu_test_buddy_alloc_exceeds_max_order: drivers/gpu/buddy.c:508: gpu_buddy_assert(gpu_buddy_block_is_free(mm->roots[i]))
[09:01:26] WARNING: drivers/gpu/buddy.c:516 at gpu_buddy_fini+0x284/0x2e0, CPU#0: kunit_try_catch/1595
[09:01:26]     # gpu_test_buddy_alloc_exceeds_max_order: drivers/gpu/buddy.c:519: gpu_buddy_assert(!mm->used_scoreboard[i])
[09:01:26] [FAILED] gpu_test_buddy_alloc_exceeds_max_order

Cheers,
-- David

---
 drivers/gpu/buddy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
index 26e7a48b55f3..964a5ae78f1b 100644
--- a/drivers/gpu/buddy.c
+++ b/drivers/gpu/buddy.c
@@ -1188,7 +1188,7 @@ static int __alloc_contig_try_harder(struct gpu_buddy *mm,
 	u64 modify_size;
 	int err;
 
-	modify_size = rounddown_pow_of_two(size);
+	modify_size = 1ULL << ilog2(size);
 	order = ilog2(modify_size) - ilog2(mm->chunk_size);
 	if (order == 0)
 		return -ENOSPC;
@@ -1435,7 +1435,7 @@ int gpu_buddy_alloc_blocks(struct gpu_buddy *mm,
 
 	/* Roundup the size to power of 2 */
 	if (flags & GPU_BUDDY_CONTIGUOUS_ALLOCATION) {
-		size = roundup_pow_of_two(size);
+		size = 1ULL << (ilog2(size - 1) + 1);
 		min_block_size = size;
 		/*
 		 * Normalize the requested size to min_block_size for regular allocations.
-- 
2.55.0

Re: [PATCH v3] drm_buddy: fix power-of-2 rounding errs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The `is_power_of_2()` macro truncates 64-bit sizes on 32-bit platforms, undermining the patch's fix.
- [High] `__alloc_contig_try_harder()` breaks list contracts, leaking local allocations and freeing caller-owned blocks.
--

commit 2e4e9330a58d876de4c3883a4c7315a7270524e2
Author: David Gow <david@davidgow.net>

drm_buddy: fix power-of-2 rounding errs

This commit replaces roundup_pow_of_two and rounddown_pow_of_two macros
with 64-bit safe ilog2 equivalents in the buddy allocator to fix
truncation of 64-bit values on 32-bit architectures.

> diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c
> index 26e7a48b55f33..964a5ae78f1be 100644
> --- a/drivers/gpu/buddy.c
> +++ b/drivers/gpu/buddy.c

[ ... ]

> @@ -1435,7 +1435,7 @@ int gpu_buddy_alloc_blocks(struct gpu_buddy *mm,
>  
>  	/* Roundup the size to power of 2 */
>  	if (flags & GPU_BUDDY_CONTIGUOUS_ALLOCATION) {
> -		size = roundup_pow_of_two(size);
> +		size = 1ULL << (ilog2(size - 1) + 1);
>  		min_block_size = size;
>  		/*
>  		 * Normalize the requested size to min_block_size for regular allocations.

[Severity: High]
This is a pre-existing issue, but does the is_power_of_2() macro still
silently truncate 64-bit sizes on 32-bit platforms, undermining this fix?

Earlier in gpu_buddy_alloc_blocks() there is a validation check:

    if (!is_power_of_2(min_block_size))
        return -EINVAL;

If min_block_size is a 64-bit value > 4GB, the unsigned long argument
silently truncates the upper 32 bits on 32-bit architectures. For example,
0x100000000 (4GB) truncates to 0 (failing the check), and 0x100001000
(4GB + 4KB) truncates to 4KB (falsely passing the check).

Similarly, gpu_buddy_can_offset_align() does this:

    return size < min_block_size && is_power_of_2(size);

Can this lead to allocation failures or memory corruption because buddy
alignment constraints are bypassed?

[Severity: High]
This is a pre-existing issue, but does __alloc_contig_try_harder() break list
contracts and cause a memory leak when used as a fallback here?

Later in gpu_buddy_alloc_blocks() during a fragmented allocation loop:

    if (flags & GPU_BUDDY_CONTIGUOUS_ALLOCATION &&
        !(flags & GPU_BUDDY_RANGE_ALLOCATION))
            return __alloc_contig_try_harder(mm,
                                             original_size,
                                             original_min_size,
                                             blocks);

If the loop partially succeeded before falling back, the blocks in the local
allocated list are never freed or spliced, causing a permanent memory leak.

Additionally, the fallback function is passed the caller's blocks list
directly instead of the local list. If it fails internally, it calls:

    gpu_buddy_free_list_internal(mm, blocks);

Doesn't this unconditionally empty the caller's entire list, destroying
any pre-existing blocks the caller might have had before calling
gpu_buddy_alloc_blocks()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913084907.520877-2-david@davidgow.net?part=1