diff mbox

[net,1/2] igb: Fix oops on changing number of rings

Message ID 1428916512-25559-1-git-send-email-makita.toshiaki@lab.ntt.co.jp
State Accepted
Delegated to: Jeff Kirsher
Headers show

Commit Message

Toshiaki Makita April 13, 2015, 9:15 a.m. UTC
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(+)

Comments

Kirsher, Jeffrey T April 14, 2015, 1:01 p.m. UTC | #1
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.
Brown, Aaron F May 2, 2015, 1:32 a.m. UTC | #2
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> Behalf Of Jeff Kirsher
> Sent: Tuesday, April 14, 2015 6:02 AM
> To: Toshiaki Makita
> Cc: intel-wired-lan@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>
diff mbox

Patch

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;