linux-kernel - Re: [PATCH v4 2/4] s390/pci: store DMA offset in bus_dma

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] [day] [month] [year] [list]

Message-ID: <467237b118aff01c95486afb733a79220f732388.camel@linux.ibm.com>
Date: Wed, 12 Feb 2025 17:15:47 +0100
From: Niklas Schnelle <schnelle@...ux.ibm.com>
To: Matthew Rosato <mjrosato@...ux.ibm.com>, joro@...tes.org, will@...nel.org,
        robin.murphy@....com, gerald.schaefer@...ux.ibm.com
Cc: hca@...ux.ibm.com, gor@...ux.ibm.com, agordeev@...ux.ibm.com,
        svens@...ux.ibm.com, borntraeger@...ux.ibm.com, farman@...ux.ibm.com,
        clegoate@...hat.com, jgg@...dia.com, iommu@...ts.linux.dev,
        linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
        Halil Pasic
	 <pasic@...ux.ibm.com>
Subject: Re: [PATCH v4 2/4] s390/pci: store DMA offset in bus_dma_region

On Wed, 2025-02-12 at 10:23 -0500, Matthew Rosato wrote:
> > > +static void pci_dma_range_setup(struct pci_dev *pdev)
> > > +{
> > > +	struct zpci_dev *zdev = to_zpci(pdev);
> > > +	struct bus_dma_region *map;
> > > +
> > > +	map = kzalloc(sizeof(*map), GFP_KERNEL);
> > > +	if (!map)
> > > +		return;
> > > +
> > > +	map->cpu_start = 0;
> > > +	map->dma_start = PAGE_ALIGN(zdev->start_dma);
> > > +	map->size = max(PAGE_ALIGN_DOWN(zdev->end_dma + 1) - map->dma_start, 0);
> > 
> > Ugh, this is my fault as I suggested it, but this max() doesn't work
> > here. The zdev->end_dma is unsigned and so is map->dma_start so if the
> > former is smaller underflow will occur and the max() won't save us.
> > It's largely a theoretical issue since zdev->end_dma + 1 should always
> > be larger than zdev->start_dma, but now the max() looks like we thought
> > of that, but then it doesn't work.
> > 
> > If we handle it maybe just go with:
> > 
> > aligned_end = PAGE_ALIGN_DOWN(zdev->end_dma + 1);
> > if (aligned_end >= map->dma_start)
> >      map->size = aligned_end - map->dma_start;
> > else
> >      map->size = 0;
> > 
> 
> 
> Given that it's not really something that's supposed to happen, would it make sense then to add a 
> 
> WARN_ON_ONCE(map->size == 0);
> 
> At the end of this?


Yes that makes sense to me