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

[Hans Zhang]

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