linux-kernel - Re: [PATCH v3 2/2] block-dma: properly take MMIO path

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 for Android: free password hash cracker in your pocket

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

Message-ID: <aRSmZ6IisXDmI9Yg@kbusch-mbp>
Date: Wed, 12 Nov 2025 10:23:19 -0500
From: Keith Busch <kbusch@...nel.org>
To: Leon Romanovsky <leon@...nel.org>
Cc: Jens Axboe <axboe@...nel.dk>, Christoph Hellwig <hch@....de>,
	Sagi Grimberg <sagi@...mberg.me>, linux-block@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-nvme@...ts.infradead.org
Subject: Re: [PATCH v3 2/2] block-dma: properly take MMIO path

On Wed, Nov 12, 2025 at 05:16:04PM +0200, Leon Romanovsky wrote:
> On Wed, Nov 12, 2025 at 09:38:07AM -0500, Keith Busch wrote:
> > On Mon, Oct 27, 2025 at 09:30:21AM +0200, Leon Romanovsky wrote:
> > > @@ -732,13 +746,20 @@ static void nvme_unmap_metadata(struct request *req)
> > >  		return;
> > >  	}
> > >  
> > > -	if (!blk_rq_integrity_dma_unmap(req, dma_dev, &iod->meta_dma_state,
> > > -					iod->meta_total_len)) {
> > > +	if (iod->flags & IOD_META_P2P)
> > > +		map = PCI_P2PDMA_MAP_BUS_ADDR;
> > > +	if (iod->flags & IOD_META_MMIO) {
> > > +		map = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
> > > +		attrs |= DMA_ATTR_MMIO;
> > > +	}
> > 
> > This should be an 'else if' no?
> 
> We can't have both IOD_META_P2P and IOD_META_MMIO. It can be only one or
> IOD_META_P2P or IOD_META_MMIO. In this case "else if' is not necessary.

I get that and I'm not saying this is a bug, but it just looks odd.
You're checking the conditions as if they're independent of each other
when they're not. If the first 'if' is true, there's no need to test the
second one as we know it's false, but this code flow will test it
anyway.