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

From: Hans Zhang
Date: Tue Jan 07 2025 - 07:11:38 EST




On 2025/1/7 19:47, Niklas Cassel wrote:
On Tue, Jan 07, 2025 at 07:43:26PM +0800, Hans Zhang wrote:


On 2025/1/7 19:33, Niklas Cassel wrote:
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.

What is the error?

We don't need to use do_div().
The current code that does normal / and % works fine on both
32-bit and 64-bit if you just do:

static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
enum pci_barno barno)
{
- int j, bar_size, buf_size, iters, remain;
+ int j, buf_size, iters, remain;
void *write_buf __free(kfree) = NULL;
void *read_buf __free(kfree) = NULL;
struct pci_dev *pdev = test->pdev;
+ u64 bar_size;

No?


Hi Niklas,

Please look at the robot compilation error.

https://patchwork.kernel.org/project/linux-pci/patch/20241231065500.168799-1-18255117159@xxxxxxx/

drivers/misc/pci_endpoint_test.c:315: undefined reference to `__udivmoddi4'

That error was for your patch that changed bar_size to resource_size_t,
which is typedefed to phys_addr_t, which can be either 32-bits or 64-bits
on 32-bit systems (depending on CONFIG_X86_PAE).

I was suggesting to just change it to u64, so it will unconditionally be
64-bits.

Hi Niklas,

The robot has been compiled with CONFIG_PHYS_ADDR_T_64BIT=y, so resource_size_t=u64


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;


Is my understanding wrong? Could you correct me, please? Thank you very much.

config: i386-randconfig-003-20250101 (https://download.01.org/0day-ci/archive/20250101/202501011917.ugP1ywJV-lkp@xxxxxxxxx/config)


I compiled it as a KO module for an experiment.
__umoddi3 and __udivdi3 is similar to __udivmoddi4.

u64 bar_size;

iters = bar_size / buf_size;
remain = bar_size % buf_size;

zhb@zhb:/media/zhb/hans/code/kernel_org/hans$ make
make -C /media/zhb/hans/code/kernel_org/linux/ M=/media/zhb/hans/code/kernel_org/hans modules
make[1]: Entering directory '/media/zhb/hans/code/kernel_org/linux'
make[2]: Entering directory '/media/zhb/hans/code/kernel_org/hans'
CC [M] pci_endpoint_test.o
MODPOST Module.symvers
ERROR: modpost: "__umoddi3" [pci_endpoint_test.ko] undefined!
ERROR: modpost: "__udivdi3" [pci_endpoint_test.ko] undefined!
make[4]: *** [/media/zhb/hans/code/kernel_org/linux/scripts/Makefile.modpost:145: Module.symvers] Error 1
make[3]: *** [/media/zhb/hans/code/kernel_org/linux/Makefile:1939: modpost] Error 2
make[2]: *** [/media/zhb/hans/code/kernel_org/linux/Makefile:251: __sub-make] Error 2
make[2]: Leaving directory '/media/zhb/hans/code/kernel_org/hans'
make[1]: *** [Makefile:251: __sub-make] Error 2
make[1]: Leaving directory '/media/zhb/hans/code/kernel_org/linux'
make: *** [Makefile:10: kernel_modules] Error 2


Best regards
Hans