[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z30RFBcOI61784bI@ryzen>
Date: Tue, 7 Jan 2025 12:33:40 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Hans Zhang <18255117159@....com>
Cc: manivannan.sadhasivam@...aro.org, kw@...ux.com, kishon@...nel.org,
arnd@...db.de, 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 Tue, Jan 07, 2025 at 07:27:24PM +0800, Hans Zhang wrote:
>
>
> 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.
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?
Kind regards,
Niklas
Powered by blists - more mailing lists