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: <Z30CywAKGRYE_p28@ryzen>
Date: Tue, 7 Jan 2025 11:32:43 +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

Hello Hans,

On Mon, Jan 06, 2025 at 11:32:33PM +0800, Hans Zhang wrote:
> 
> 
> On 2025/1/6 19:49, Niklas Cassel wrote:
> > Doing a:
> > $ git grep -A 10 "IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT"
> > does not show very many hits, which suggests that this is not the proper
> > way to solve this.
> > 
> > I don't know the proper solution to this. How is resource_size_t handled
> > in other PCI driver when being built on with 32-bit PHYS_ADDR_T ?
> > 
> > Can't you just cast the resource_size_t to u64 before doing the division?
> 
> Hi Niklas,
> 
> Modify as follows, if you have no opinion, I will fix the next version.
> 
> > > ---
> > >   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?


Kind regards,
Niklas