netdev - Re: [PATCH net-next v3 1/2] net: implement dma cache skb allocator

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]

Date:	Sat, 14 Mar 2015 20:08:45 +0000
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Govindarajulu Varadarajan <_govind@....com>
Cc:	davem@...emloft.net, netdev@...r.kernel.org, ssujith@...co.com,
	benve@...co.com
Subject: Re: [PATCH net-next v3 1/2] net: implement dma cache skb allocator

On Tue, 2015-03-10 at 23:13 +0530, Govindarajulu Varadarajan wrote:
[...]
> +static struct netdev_dma_node *netdev_dma_alloc(struct netdev_dma_head *nc_head,
> +						size_t sz)
> +{
> +	struct netdev_dma_node *nc = nc_head->nc;
> +	int offset;
> +
> +	if (unlikely(!nc)) {
> +refill:
> +		__dma_cache_refill(nc_head, sz);
> +		nc = nc_head->nc;
> +		if (unlikely(!nc))
> +			return NULL;
> +	}
> +
> +	offset = nc->frag.offset - sz;
> +	if (offset < 0) {
> +		if (!atomic_sub_and_test(nc->pagecnt_bias,
> +					 &nc->frag.page->_count)) {
> +			/* the caller has processed all the frags belonging to
> +			 * this page. Since page->_count is not 0 and
> +			 * nc->dma_count is 0 these frags should be in stack.
> +			 * We should unmap the page here.

Unmapping potentially copies data back from a bounce buffer (even if
it's already been synchronised).  This is OK in the case of a buffer
that's attached to the skb as a page fragment, because in that case it's
read-only for the stack.  It's not OK for a buffer that has been passed
to build_skb(), as this may legitimately have been changed by the stack
and this will overwrite the changes.

So you have to choose between DMA buffer recycling and build_skb().

> +			 */
> +			if (!nc->dma_count) {
> +				dma_unmap_single(nc_head->dev, nc->dma_addr,
> +						 nc->frag.size,
> +						 DMA_FROM_DEVICE);
[...]

Don't mix dma_map_page() and dma_unmap_single().

Ben.

-- 
Ben Hutchings
Q.  Which is the greater problem in the world today, ignorance or apathy?
A.  I don't know and I couldn't care less.

Download attachment "signature.asc" of type "application/pgp-signature" (812 bytes)