Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size

Hans Zhang
2025-01-07T11:28:19Z

On 2025/1/7 18:32, Niklas Cassel wrote:
>>>> ---
>>>> drivers/misc/pci_endpoint_test.c | 12 +++++++++---
>>>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
>>>> index 3aaaf47fa4ee..50d4616119af 100644
>>>> --- a/drivers/misc/pci_endpoint_test.c
>>>> +++ b/drivers/misc/pci_endpoint_test.c
>>>> @@ -280,10 +280,11 @@ static int pci_endpoint_test_bar_memcmp(struct pci_endpoint_test *test,
>>>> static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
>>>> enum pci_barno barno)
>>>> {
>>>> - int j, bar_size, buf_size, iters, remain;
>>>> void *write_buf __free(kfree) = NULL;
>>>> void *read_buf __free(kfree) = NULL;
>>>> struct pci_dev *pdev = test->pdev;
>>>> + int j, buf_size, iters, remain;
>>>> + resource_size_t bar_size;
>>
>> Fix resource_size_t to u64 bar_size.
>> u64 bar_size;
>>
>>>> if (!test->bar[barno])
>>>> return false;
>>>> @@ -307,13 +308,18 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
>>>> if (!read_buf)
>>>> return false;
>>>> - iters = bar_size / buf_size;
>>>> + if (IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT)) {
>>>> + remain = do_div(bar_size, buf_size);
>>>> + iters = bar_size;
>>>> + } else {
>>>> + iters = bar_size / buf_size;
>>>> + remain = bar_size % buf_size;
>>>> + }
>>
>> Removed IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT), Execute the following code.
>>
>> remain = do_div(bar_size, buf_size);
>> iters = bar_size;
>
> Perhaps keep it as resource_size_t and then cast it to u64 in the do_div()
> call?

Hi Niklas,

resource_size_t bar_size;
remain = do_div((u64)bar_size, buf_size);

It works for the arm platform.

arch/arm/include/asm/div64.h
static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
{
register unsigned int __base asm("r4") = base;
register unsigned long long __n asm("r0") = *n;
register unsigned long long __res asm("r2");
unsigned int __rem;
asm( __asmeq("%0", "r0")
__asmeq("%1", "r2")
__asmeq("%2", "r4")
"bl __do_div64"
: "+r" (__n), "=r" (__res)
: "r" (__base)
: "ip", "lr", "cc");
__rem = __n >> 32;
*n = __res;
return __rem;
}
#define __div64_32 __div64_32

#define do_div(n, base) __div64_32(&(n), base)

For X86 platforms, do_div is a macro definition, and the first parameter
does not define its type. If the macro definition is replaced directly,
an error will be reported in the ubuntu20.04 release.

resource_size_t bar_size;
remain = do_div((u64)bar_size, buf_size);

arch/x86/include/asm/div64.h
#define do_div(n, base) \
({ \
unsigned long __upper, __low, __high, __mod, __base; \
__base = (base); \
if (__builtin_constant_p(__base) && is_power_of_2(__base)) { \
__mod = n & (__base - 1); \
n >>= ilog2(__base); \
} else { \
asm("" : "=a" (__low), "=d" (__high) : "A" (n));\
__upper = __high; \
if (__high) { \
__upper = __high % (__base); \
__high = __high / (__base); \
} \
asm("divl %2" : "=a" (__low), "=d" (__mod) \
: "rm" (__base), "0" (__low), "1" (__upper)); \
asm("" : "=A" (n) : "a" (__low), "d" (__high)); \
} \
__mod; \
})

Best regards
Hans