[PATCH] drivers: net: xen-netfront: fix array initialization bug

[PATCH] drivers: net: xen-netfront: fix array initialization bug

[Vincenzo Maffione]

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(-)

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 */
-- 
1.9.1

Re: [PATCH] drivers: net: xen-netfront: fix array initialization bug

[David Miller]

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.

Re: [Xen-devel] [PATCH] drivers: net: xen-netfront: fix array initialization bug

[David Vrabel]

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

Re: [Xen-devel] [PATCH] drivers: net: xen-netfront: fix array initialization bug

[David Miller]

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?

Re: [Xen-devel] [PATCH] drivers: net: xen-netfront: fix array initialization bug

[David Vrabel]

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