[Intel-wired-lan] [PATCH net 1/2] igb: Fix oops on changing number of rings

[Intel-wired-lan] [PATCH net 1/2] igb: Fix oops on changing number of rings

[Toshiaki Makita]

When changing the number of rings by ethtool -L, q_vectors are reused,
which causes oops because of uninitialized pointers.

- When an rx is reused as a tx, q_vector->rx.ring is not set to NULL, which
  misleads igb_poll() to determine that it has an rx ring although it
  actually points to the tx ring.
- When a tx is reused as an rx, q_vector->rx.ring->skb
  (q_vector->ring[0].skb) has a value that was used as tx_stats before.

Fix these problems by zeroing it out on reuseing it.

Fixes: 02ef6e1d0b00 ("igb: Fix queue allocation method to accommodate changing during runtime")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index f366b3b..010ddcb 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -1207,6 +1207,8 @@ static int igb_alloc_q_vector(struct igb_adapter *adapter,
 	q_vector = adapter->q_vector[v_idx];
 	if (!q_vector)
 		q_vector = kzalloc(size, GFP_KERNEL);
+	else
+		memset(q_vector, 0, size);
 	if (!q_vector)
 		return -ENOMEM;
 
-- 
1.8.1.2

[Intel-wired-lan] [PATCH net 2/2] igb: Fix NULL assignment to incorrect variable in igb_reset_q_vector

[Toshiaki Makita]

adapter->tx_ring is set to NULL where rx_ring should be.

Fixes: 5536d2102a2d ("igb: Combine q_vector and ring allocation into a single function")
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 010ddcb..b664512 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -1036,7 +1036,7 @@ static void igb_reset_q_vector(struct igb_adapter *adapter, int v_idx)
 		adapter->tx_ring[q_vector->tx.ring->queue_index] = NULL;
 
 	if (q_vector->rx.ring)
-		adapter->tx_ring[q_vector->rx.ring->queue_index] = NULL;
+		adapter->rx_ring[q_vector->rx.ring->queue_index] = NULL;
 
 	netif_napi_del(&q_vector->napi);
 
-- 
1.8.1.2

[Intel-wired-lan] [PATCH net 1/2] igb: Fix oops on changing number of rings

[Jeff Kirsher]

On Mon, 2015-04-13 at 18:15 +0900, Toshiaki Makita wrote:
> When changing the number of rings by ethtool -L, q_vectors are reused,
> which causes oops because of uninitialized pointers.
> 
> - When an rx is reused as a tx, q_vector->rx.ring is not set to NULL,
> which
>   misleads igb_poll() to determine that it has an rx ring although it
>   actually points to the tx ring.
> - When a tx is reused as an rx, q_vector->rx.ring->skb
>   (q_vector->ring[0].skb) has a value that was used as tx_stats
> before.
> 
> Fix these problems by zeroing it out on reuseing it.
> 
> Fixes: 02ef6e1d0b00 ("igb: Fix queue allocation method to accommodate
> changing during runtime")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
>  1 file changed, 2 insertions(+)

Thanks Toshiaki, I have applied your patch to my queue.
-- 
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-queue.git
dev-queue
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20150414/fcb21064/attachment.asc>

[Intel-wired-lan] [PATCH net 2/2] igb: Fix NULL assignment to incorrect variable in igb_reset_q_vector

[Jeff Kirsher]

On Mon, 2015-04-13 at 18:15 +0900, Toshiaki Makita wrote:
> adapter->tx_ring is set to NULL where rx_ring should be.
> 
> Fixes: 5536d2102a2d ("igb: Combine q_vector and ring allocation into a
> single function")
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks Toshiaki, I have applied your patch to my queue.
-- 
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-queue.git
dev-queue
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20150414/0d351407/attachment.asc>

[Intel-wired-lan] [PATCH net 1/2] igb: Fix oops on changing number of rings

[Brown, Aaron F]

> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Jeff Kirsher
> Sent: Tuesday, April 14, 2015 6:02 AM
> To: Toshiaki Makita
> Cc: intel-wired-lan at lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH net 1/2] igb: Fix oops on changing
> number of rings
> 
> On Mon, 2015-04-13 at 18:15 +0900, Toshiaki Makita wrote:
> > When changing the number of rings by ethtool -L, q_vectors are reused,
> > which causes oops because of uninitialized pointers.
> >
> > - When an rx is reused as a tx, q_vector->rx.ring is not set to NULL,
> > which
> >   misleads igb_poll() to determine that it has an rx ring although it
> >   actually points to the tx ring.
> > - When a tx is reused as an rx, q_vector->rx.ring->skb
> >   (q_vector->ring[0].skb) has a value that was used as tx_stats
> > before.
> >
> > Fix these problems by zeroing it out on reuseing it.
> >
> > Fixes: 02ef6e1d0b00 ("igb: Fix queue allocation method to accommodate
> > changing during runtime")
> > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> > ---
> >  drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> Thanks Toshiaki, I have applied your patch to my queue.
> --

Tested-by: Aaron Brown <aaron.f.brown@intel.com>

[Intel-wired-lan] [PATCH net 2/2] igb: Fix NULL assignment to incorrect variable in igb_reset_q_vector

[Brown, Aaron F]

> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On
> Behalf Of Jeff Kirsher
> Sent: Tuesday, April 14, 2015 6:03 AM
> To: Toshiaki Makita
> Cc: intel-wired-lan at lists.osuosl.org
> Subject: Re: [Intel-wired-lan] [PATCH net 2/2] igb: Fix NULL assignment to
> incorrect variable in igb_reset_q_vector
> 
> On Mon, 2015-04-13 at 18:15 +0900, Toshiaki Makita wrote:
> > adapter->tx_ring is set to NULL where rx_ring should be.
> >
> > Fixes: 5536d2102a2d ("igb: Combine q_vector and ring allocation into a
> > single function")
> > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> > ---
> >  drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Thanks Toshiaki, I have applied your patch to my queue.

Tested-by: Aaron Brown <aaron.f.brown@intel.com>