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

[Hans Zhang <18255117159@163.com>]

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