diff mbox

drivers: net: xen-netfront: fix array initialization bug

Message ID 1397296540-3007-1-git-send-email-v.maffione@gmail.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Vincenzo Maffione April 12, 2014, 9:55 a.m. UTC
This patch fixes the initialization of an array used in the TX
datapath that was mistakenly initialized together with the
RX datapath arrays. An out of range array access could happen
when RX and TX rings had different sizes.

Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
---
 drivers/net/xen-netfront.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

David Miller April 12, 2014, 8:51 p.m. UTC | #1
From: Vincenzo Maffione <v.maffione@gmail.com>
Date: Sat, 12 Apr 2014 11:55:40 +0200

> This patch fixes the initialization of an array used in the TX
> datapath that was mistakenly initialized together with the
> RX datapath arrays. An out of range array access could happen
> when RX and TX rings had different sizes.
> 
> Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>

Good catch, applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Vrabel April 14, 2014, 9:42 a.m. UTC | #2
On 12/04/14 21:51, David Miller wrote:
> From: Vincenzo Maffione <v.maffione@gmail.com>
> Date: Sat, 12 Apr 2014 11:55:40 +0200
> 
>> This patch fixes the initialization of an array used in the TX
>> datapath that was mistakenly initialized together with the
>> RX datapath arrays. An out of range array access could happen
>> when RX and TX rings had different sizes.
>>
>> Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
> 
> Good catch, applied, thanks.

Thanks.  You can queue this for net-next since the Tx and Rx rings are
the same constant size.

David
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller April 14, 2014, 4:51 p.m. UTC | #3
From: David Vrabel <david.vrabel@citrix.com>
Date: Mon, 14 Apr 2014 10:42:20 +0100

> On 12/04/14 21:51, David Miller wrote:
>> From: Vincenzo Maffione <v.maffione@gmail.com>
>> Date: Sat, 12 Apr 2014 11:55:40 +0200
>> 
>>> This patch fixes the initialization of an array used in the TX
>>> datapath that was mistakenly initialized together with the
>>> RX datapath arrays. An out of range array access could happen
>>> when RX and TX rings had different sizes.
>>>
>>> Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
>> 
>> Good catch, applied, thanks.
> 
> Thanks.  You can queue this for net-next since the Tx and Rx rings are
> the same constant size.

I was able to determine when I reviewed this patch that the size in bytes
of the rings are the same (PAGE_SIZE), but I couldn't ascertain whether
the individual ring entries in the TX ring and RX ring are the same size.

Are they?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Vrabel April 14, 2014, 5:25 p.m. UTC | #4
On 14/04/14 17:51, David Miller wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> Date: Mon, 14 Apr 2014 10:42:20 +0100
> 
>> On 12/04/14 21:51, David Miller wrote:
>>> From: Vincenzo Maffione <v.maffione@gmail.com>
>>> Date: Sat, 12 Apr 2014 11:55:40 +0200
>>>
>>>> This patch fixes the initialization of an array used in the TX
>>>> datapath that was mistakenly initialized together with the
>>>> RX datapath arrays. An out of range array access could happen
>>>> when RX and TX rings had different sizes.
>>>>
>>>> Signed-off-by: Vincenzo Maffione <v.maffione@gmail.com>
>>>
>>> Good catch, applied, thanks.
>>
>> Thanks.  You can queue this for net-next since the Tx and Rx rings are
>> the same constant size.
> 
> I was able to determine when I reviewed this patch that the size in bytes
> of the rings are the same (PAGE_SIZE), but I couldn't ascertain whether
> the individual ring entries in the TX ring and RX ring are the same size.
> 
> Are they?

Yes.  It's not at all obvious, but each ends up with 256 entries.

Tx entries are 12 bytes and Rx entries are 8 bytes.  The ring macros
reserve some space in the shared page for the producer/consumer indexes
/and/ round down the number to the next power of two.

David

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

Patch

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 057b057..158b5e6 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1291,13 +1291,13 @@  static struct net_device *xennet_create_dev(struct xenbus_device *dev)
 	for (i = 0; i < NET_TX_RING_SIZE; i++) {
 		skb_entry_set_link(&np->tx_skbs[i], i+1);
 		np->grant_tx_ref[i] = GRANT_INVALID_REF;
+		np->grant_tx_page[i] = NULL;
 	}
 
 	/* Clear out rx_skbs */
 	for (i = 0; i < NET_RX_RING_SIZE; i++) {
 		np->rx_skbs[i] = NULL;
 		np->grant_rx_ref[i] = GRANT_INVALID_REF;
-		np->grant_tx_page[i] = NULL;
 	}
 
 	/* A grant for every tx ring slot */