Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] The `pages` variable is declared as `unsigned long`, and the `fls()` and `BIT()` macros are used, leading to silent truncation and potential infinite loops or crashes for large allocations. - [High] Calling the `is_power_of_2()` macro with 64-bit variables (`min_block_size` and `size`) silently truncates them to 32 bits on 32-bit architectures, breaking allocator validation. -- --- Patch [2]: [PATCH 2/2] drm_buddy: fix power-of-2 rounding errs --- commit 6e9e2fbe5490ccf9c40ea5dcdf1e897c00f63b30 Author: Jim Cromie <[email protected]> drm_buddy: fix power-of-2 rounding errs This patch replaces standard roundup_pow_of_two() and rounddown_pow_of_two() macros with their 64-bit equivalents in the GPU buddy allocator. This fixes silent truncation of 64-bit allocation sizes on 32-bit architectures, which was causing tests to fail. Link: https://lore.kernel.org/all/[email protected]/ > diff --git a/drivers/gpu/buddy.c b/drivers/gpu/buddy.c > index a5553fcec28c2..64f41f1fe9089 100644 > --- a/drivers/gpu/buddy.c > +++ b/drivers/gpu/buddy.c [ ... ] > @@ -1440,7 +1440,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 = roundup_pow_of_two64(size); > min_block_size = size; > /* > * Normalize the requested size to min_block_size for regular > allocations. [Severity: High] This is a pre-existing issue, but since you are fixing 32-bit truncation errors in this function, are there similar truncation issues with the pages variable later in gpu_buddy_alloc_blocks()? The pages variable is declared as unsigned long: unsigned long pages; And later initialized and used with fls() and BIT(): pages = size >> ilog2(mm->chunk_size); order = fls(pages) - 1; ... pages -= BIT(order); Since unsigned long is 32-bit on 32-bit architectures, pages truncates for sizes > 16TB (assuming 4KB chunks). Furthermore, fls() takes a 32-bit integer, so it will truncate pages on 64-bit systems. BIT(order) also uses 1UL, which overflows on 32-bit architectures if order >= 32. Can this lead to infinite loops or crashes for large allocations? Similar truncation might also occur in gpu_buddy_offset_aligned_allocation(), which also uses unsigned long pages. [Severity: High] This is another pre-existing issue, but does is_power_of_2() also silently truncate 64-bit variables on 32-bit architectures? Earlier in gpu_buddy_alloc_blocks(): if (!is_power_of_2(min_block_size)) return -EINVAL; The is_power_of_2() macro casts its argument to unsigned long. If a 64-bit value like 4GB + 4KB is passed, it truncates to 4KB on 32-bit systems, which is a valid power of two. This bypasses the validation and allows non-power-of-two sizes to reach round_up(), which could corrupt sizes and allocator memory. A similar check exists in gpu_buddy_can_offset_align(): return size < min_block_size && is_power_of_2(size); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
