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

Hans Zhang
2025-01-09T03:00:22Z

On 2025/1/8 22:13, Niklas Cassel wrote:
Hi Niklas,

>>> Ok. Looking at do_div(), it seems to be the correct API to use
>>> for this problem. Just change bar_size type to u64 (instead of casting)
>>> and use do_div() ? That is how it is seems to be used in other drivers.
>>
>> I think using div_u64_rem() instead of do_div() would make this
>> more readable as this is always an inline function, so the type can
>> remain resource_size_t, and the division gets optimized well when
>> that is a 32-bit type.
>
> After patch 1/2, we no longer care about the remainder, so I guess
> div64_u64() is the correct function to use then?
>

include/asm-generic/bitsperlong.h
#ifdef CONFIG_64BIT
#define BITS_PER_LONG 64
#else
#define BITS_PER_LONG 32
#endif /* CONFIG_64BIT */

include/linux/types.h
#ifdef CONFIG_PHYS_ADDR_T_64BIT
typedef u64 phys_addr_t;
#else
typedef u32 phys_addr_t;
#endif

typedef phys_addr_t resource_size_t;

include/linux/math64.h
#if BITS_PER_LONG == 64
......
static inline u64 div64_u64(u64 dividend, u64 divisor)
{
return dividend / divisor;
}
......
#elif BITS_PER_LONG == 32
#ifndef div64_u64
extern u64 div64_u64(u64 dividend, u64 divisor);
#endif
......
#endif /* BITS_PER_LONG */

lib/math/div64.c
u64 div64_u64(u64 dividend, u64 divisor)
{
u32 high = divisor >> 32;
u64 quot;

if (high == 0) {
quot = div_u64(dividend, divisor);
} else {
int n = fls(high);
quot = div_u64(dividend >> n, divisor >> n);

if (quot != 0)
quot--;
if ((dividend - quot * divisor) >= divisor)
quot++;
}

return quot;
}
EXPORT_SYMBOL(div64_u64);

If CONFIG_64BIT and CONFIG_PHYS_ADDR_T_64BIT are not configured. The
resource_size_t=u32, and the first parameter of div64_u64 is u64.The
same question is as follows:

https://patchwork.kernel.org/project/linux-pci/patch/20250102120222.1403906-1-18255117159@163.com/

config: arm-defconfig
(https://download.01.org/0day-ci/archive/20250103/202501030414.p0DE9xNK-lkp@intel.com/config)

All error/warnings (new ones prefixed by >>):

>> drivers/misc/pci_endpoint_test.c:311:11: warning: comparison of
distinct pointer types ('typeof ((bar_size)) *' (aka 'unsigned int *')
and 'uint64_t *' (aka 'unsigned long long *'))
[-Wcompare-distinct-pointer-types]
311 | remain = do_div(bar_size, buf_size);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
include/asm-generic/div64.h:183:28: note: expanded from macro 'do_div'
183 | (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
| ~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~
>> drivers/misc/pci_endpoint_test.c:311:11: error: incompatible pointer
types passing 'resource_size_t *' (aka 'unsigned int *') to parameter of
type 'uint64_t *' (aka 'unsigned long long *')
[-Werror,-Wincompatible-pointer-types]
311 | remain = do_div(bar_size, buf_size);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
include/asm-generic/div64.h:199:22: note: expanded from macro 'do_div'
199 | __rem = __div64_32(&(n), __base); \
| ^~~~
arch/arm/include/asm/div64.h:24:45: note: passing argument to
parameter 'n' here
24 | static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
| ^
>> drivers/misc/pci_endpoint_test.c:311:11: warning: shift count >=
width of type [-Wshift-count-overflow]
311 | remain = do_div(bar_size, buf_size);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
include/asm-generic/div64.h:195:25: note: expanded from macro 'do_div'
195 | } else if (likely(((n) >> 32) == 0)) { \
| ^ ~~
include/linux/compiler.h:76:40: note: expanded from macro 'likely'
76 | # define likely(x) __builtin_expect(!!(x), 1)
| ^
2 warnings and 1 error generated.

Best regards
Hans