netdev - Re: [PATCH net-next 3/3] net/mlx4_en: Fix handling of 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]

Date:	Mon, 19 Aug 2013 22:42:00 +0200
From:	Francois Romieu <romieu@...zoreil.com>
To:	Amir Vadai <amirv@...lanox.com>
Cc:	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 3/3] net/mlx4_en: Fix handling of dma_map failure

Amir Vadai <amirv@...lanox.com> :
[...]
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> index 157bcd1..92d7097 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
> @@ -673,6 +673,64 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
>  	tx_info->skb = skb;
>  	tx_info->nr_txbb = nr_txbb;
>  
> +	if (lso_header_size)
> +		data = ((void *)&tx_desc->lso + ALIGN(lso_header_size + 4,
> +						      DS_SIZE));
> +	else
> +		data = &tx_desc->data;
> +
> +	/* valid only for none inline segments */
> +	tx_info->data_offset = (void *)data - (void *)tx_desc;
> +
> +	tx_info->linear = (lso_header_size < skb_headlen(skb) &&
> +			   !is_inline(skb, NULL)) ? 1 : 0;
> +
> +	data += skb_shinfo(skb)->nr_frags + tx_info->linear - 1;
> +
> +	if (is_inline(skb, &fragptr)) {
> +		tx_info->inl = 1;
> +	} else {
> +		/* Map fragments */
> +		for (i = skb_shinfo(skb)->nr_frags - 1; i >= 0; i--) {
> +			frag = &skb_shinfo(skb)->frags[i];
> +			dma = skb_frag_dma_map(priv->ddev, frag,
> +					       0, skb_frag_size(frag),
> +					       DMA_TO_DEVICE);
> +			if (dma_mapping_error(priv->ddev, dma)) {

				goto err_unmap_frags;

You have a lot of huge scope variables. At least use these to hide
the 80 cols problems.

> +				for (i++; i < skb_shinfo(skb)->nr_frags; i++) {
> +					frag = &skb_shinfo(skb)->frags[i];
> +					en_err(priv, "DMA mapping error\n");
> +					dma_unmap_page(priv->ddev,
> +						       (dma_addr_t) be64_to_cpu(data[i].addr),
> +						       skb_frag_size(frag), PCI_DMA_TODEVICE);
> +				}
> +				goto tx_drop;
> +			}
> +
> +			data->addr = cpu_to_be64(dma);
> +			data->lkey = cpu_to_be32(mdev->mr.key);
> +			wmb();
> +			data->byte_count = cpu_to_be32(skb_frag_size(frag));
> +			--data;
> +		}
> +
> +		/* Map linear part */
> +		if (tx_info->linear) {
> +			u32 byte_count = skb_headlen(skb) - lso_header_size;
> +			dma = dma_map_single(priv->ddev, skb->data +
> +					     lso_header_size, byte_count,
> +					     PCI_DMA_TODEVICE);
> +			if (dma_mapping_error(priv->ddev, dma))
> +				goto tx_drop;

(frags dma leak)
				goto err_unmap_frags;

You may consider a local variable for 'priv->ddev' btw.

-- 
Ueimor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html