netdev - Re: [PATCH v2 03/24] page_pool: Add netmem_set_dma_addr() and netmem_get_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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y72rTrFzDiMJLfc3@casper.infradead.org>
Date:   Tue, 10 Jan 2023 18:15:42 +0000
From:   Matthew Wilcox <willy@...radead.org>
To:     Ilias Apalodimas <ilias.apalodimas@...aro.org>
Cc:     Jesper Dangaard Brouer <hawk@...nel.org>, netdev@...r.kernel.org,
        linux-mm@...ck.org, Shakeel Butt <shakeelb@...gle.com>
Subject: Re: [PATCH v2 03/24] page_pool: Add netmem_set_dma_addr() and
 netmem_get_dma_addr()

On Mon, Jan 09, 2023 at 07:30:30PM +0200, Ilias Apalodimas wrote:
> Hi Matthew

Hey Ilias.  Thanks for all the review!

> > -static inline dma_addr_t page_pool_get_dma_addr(struct page *page)
> > +static inline dma_addr_t netmem_get_dma_addr(struct netmem *nmem)
> 
> Ideally, we'd like to avoid having people call these directly and use
> the page_pool_(get|set)_dma_addr wrappers.  Can we add a comment in
> v3?

I don't think this is what we want.  Currently drivers call
page_pool_get_dma_addr() on pages that are presumably from the page
pool, but the compiler isn't going to help them out if they just
get the struct page from somewhere random.  They'll get garbage and
presumably crash.

By returning a netmem pointer from page_pool, we help drivers ensure
that they're only passing around memory that was actually allocated
from the page_pool and so they won't get garbage if they pass it to
netmem_get_dma_addr().  The page_pool_get_dma_addr() wrapper is
a temporary measure until we have all the drivers converted to
use netmem alone.

Does that all make sense, or have I misunderstood what you wanted
from a comment?