linux-kernel - Re: [v8] misc: pci_endpoint_test: Fix overflow of bar

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f2901d4f-52c8-496d-9939-3b0e113cba4b@163.com>
Date: Thu, 9 Jan 2025 10:59:00 +0800
From: Hans Zhang <18255117159@....com>
To: Niklas Cassel <cassel@...nel.org>, Arnd Bergmann <arnd@...db.de>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
 Krzysztof Wilczyński <kw@...ux.com>,
 Kishon Vijay Abraham I <kishon@...nel.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-pci@...r.kernel.org,
 linux-kernel@...r.kernel.org, rockswang7@...il.com
Subject: Re: [v8] misc: pci_endpoint_test: Fix overflow of bar_size



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