netdev - RE: BUG in netxen_release_tx_buffers when TSO enabled on kernels >= 3.3 and <= 3.6

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: <C562D0E12CBDE44E8A0CCAB77060096705CE85@AVMB1.qlogic.org>
Date:	Tue, 22 Jan 2013 18:46:41 +0000
From:	Rajesh Borundia <rajesh.borundia@...gic.com>
To:	Eric Dumazet <eric.dumazet@...il.com>,
	"christoph.paasch@...ouvain.be" <christoph.paasch@...ouvain.be>
CC:	Ian Campbell <Ian.Campbell@...rix.com>,
	Sony Chacko <sony.chacko@...gic.com>,
	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>
Subject: RE: BUG in netxen_release_tx_buffers when TSO enabled on kernels >=
 3.3 and <= 3.6

>-----Original Message-----
>From: Eric Dumazet [mailto:eric.dumazet@...il.com]
>Sent: Tuesday, January 22, 2013 10:03 PM
>To: christoph.paasch@...ouvain.be
>Cc: Ian Campbell; Sony Chacko; Rajesh Borundia; David Miller; netdev
>Subject: Re: BUG in netxen_release_tx_buffers when TSO enabled on
>kernels >= 3.3 and <= 3.6
>
>From: Eric Dumazet <edumazet@...gle.com>
>
>On Tue, 2013-01-22 at 16:43 +0100, Christoph Paasch wrote:
>> In netxen_map_tx_skb() I think we also have to set nf->dma to 0ULL
>(like the
>> diff below).
>>
>> Otherwise, netxen_release_tx_buffer() may try to unmap something that
>has
>> already been unmapped.
>>
>> I'm not sure - I don't feel very comfortable in driver-code...
>
>It seems fine to me, here is the official combined patch, feel
>free to add your 'Signed-off-by'
>
>Thanks !
>
>[PATCH] netxen: fix off by one bug in netxen_release_tx_buffer()
>
>Christoph Paasch found netxen could trigger a BUG in its dismantle
>phase, in netxen_release_tx_buffer(), using full size TSO packets.
>
>cmd_buf->frag_count includes the skb->data part, so the loop must
>start at index 1 instead of 0, or else we can make an out
>of bound access to cmd_buff->frag_array[MAX_SKB_FRAGS + 2]
>
>Christoph provided the fixes in netxen_map_tx_skb() function.
>In case of a dma mapping error, its better to clear the dma fields
>so that we don't try to unmap them again in netxen_release_tx_buffer()
>
>Reported-by: Christoph Paasch <christoph.paasch@...ouvain.be>
>Signed-off-by: Eric Dumazet <edumazet@...gle.com>
>Tested-by: Christoph Paasch <christoph.paasch@...ouvain.be>
>Cc: Sony Chacko <sony.chacko@...gic.com>
>Cc: Rajesh Borundia <rajesh.borundia@...gic.com>
>---
> drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c |    2 +-
> drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c |    2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
>b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
>index bc165f4..695667d 100644
>--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
>+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
>@@ -144,7 +144,7 @@ void netxen_release_tx_buffers(struct netxen_adapter
>*adapter)
> 					 buffrag->length, PCI_DMA_TODEVICE);
> 			buffrag->dma = 0ULL;
> 		}
>-		for (j = 0; j < cmd_buf->frag_count; j++) {
>+		for (j = 1; j < cmd_buf->frag_count; j++) {
> 			buffrag++;
> 			if (buffrag->dma) {
> 				pci_unmap_page(adapter->pdev, buffrag->dma,
>diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>index 6098fd4a..69e321a 100644
>--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
>@@ -1963,10 +1963,12 @@ unwind:
> 	while (--i >= 0) {
> 		nf = &pbuf->frag_array[i+1];
> 		pci_unmap_page(pdev, nf->dma, nf->length, PCI_DMA_TODEVICE);
>+		nf->dma = 0ULL;
> 	}
>
> 	nf = &pbuf->frag_array[0];
> 	pci_unmap_single(pdev, nf->dma, skb_headlen(skb),
>PCI_DMA_TODEVICE);
>+	nf->dma = 0ULL;
>
> out_err:
> 	return -ENOMEM;
>
>

Acked-by: Rajesh Borundia <rajesh.borundia@...gic.com