netdev - Re: [RFC PATCH] net/mlx4: Get rid of page operation after dma_alloc

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]

Date:   Tue, 18 Dec 2018 16:32:10 +0000
From:   Jason Gunthorpe <jgg@...lanox.com>
To:     Stephen Warren <swarren@...dotorg.org>
CC:     Tariq Toukan <tariqt@...lanox.com>,
        "Wei@...dotorg.org" <Wei@...dotorg.org>,
        "Hu@...dotorg.org" <Hu@...dotorg.org>,
        "\\ (Xavier\\)" <xavier.huwei@...wei.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        Doug Ledford <dledford@...hat.com>,
        Stephen Warren <swarren@...dia.com>
Subject: Re: [RFC PATCH] net/mlx4: Get rid of page operation after
 dma_alloc_coherent

On Fri, Dec 14, 2018 at 04:32:54PM -0700, Stephen Warren wrote:
> From: Stephen Warren <swarren@...dia.com>
> 
> This is a port of commit 378efe798ecf ("RDMA/hns: Get rid of page
> operation after dma_alloc_coherent") to the mlx4 driver. That change was
> described as:
> 
> > In general, dma_alloc_coherent() returns a CPU virtual address and
> > a DMA address, and we have no guarantee that the underlying memory
> > even has an associated struct page at all.
> >
> > This patch gets rid of the page operation after dma_alloc_coherent,
> > and records the VA returned form dma_alloc_coherent in the struct
> > of hem in hns RoCE driver.
> 
> Differences in this port relative to the hns patch:
> 
> 1) The hns patch only needed to fix a dma_alloc_coherent path, but this
> patch also needs to fix an alloc_pages path. This appears to be simple
> except for the next point.
> 
> 2) The hns patch converted a bunch of code to consistently use
> sg_dma_len(mem) rather than a mix of that and mem->length However, it
> seems that sg_dma_len(mem) can be modified or zeroed at runtime, and so
> using it when calling e.g. __free_pages is problematic.

dma_len should only ever be used when programming a HW device to do
DMA. It certainly should never be used for anything else, so I'm not
sure why this description veered off into talking about alloc_pages?

If pages were allocated and described in a sg list then the CPU side
must use the pages/len part of the SGL to walk that list of pages.

I also don't really see a practical problem with putting the virtual
address pointer of DMA coherent memory in the SGL, so long as it is
never used in a DMA map operation or otherwise.

.. so again, what is it this is actually trying to fix in mlx4?

Jason