[patch net 0/3] team: couple of fixes

[patch net 0/3] team: couple of fixes

[Jiri Pirko]

Jiri Pirko (3):
  team: check return value of team_get_port_by_index_rcu() for NULL
  team: move add to port list before port enablement
  team: fix checks in team_get_first_port_txable_rcu()

 drivers/net/team/team.c                 | 2 +-
 drivers/net/team/team_mode_random.c     | 2 ++
 drivers/net/team/team_mode_roundrobin.c | 2 ++
 include/linux/if_team.h                 | 4 ++--
 4 files changed, 7 insertions(+), 3 deletions(-)

-- 
1.7.11.7

[patch net 1/3] team: check return value of team_get_port_by_index_rcu() for NULL

[Jiri Pirko]

team_get_port_by_index_rcu() might return NULL due to race between port
removal and skb tx path. Panic is easily triggeable when txing packets
and adding/removing port in a loop.

introduced by commit 3d249d4ca "net: introduce ethernet teaming device"
and commit 753f993911b "team: introduce random mode" (for random mode)

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/team/team_mode_random.c     | 2 ++
 drivers/net/team/team_mode_roundrobin.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/team/team_mode_random.c b/drivers/net/team/team_mode_random.c
index 5ca14d4..7f032e2 100644
--- a/drivers/net/team/team_mode_random.c
+++ b/drivers/net/team/team_mode_random.c
@@ -28,6 +28,8 @@ static bool rnd_transmit(struct team *team, struct sk_buff *skb)
 
 	port_index = random_N(team->en_port_count);
 	port = team_get_port_by_index_rcu(team, port_index);
+	if (unlikely(!port))
+		goto drop;
 	port = team_get_first_port_txable_rcu(team, port);
 	if (unlikely(!port))
 		goto drop;
diff --git a/drivers/net/team/team_mode_roundrobin.c b/drivers/net/team/team_mode_roundrobin.c
index d268e4d..472623f 100644
--- a/drivers/net/team/team_mode_roundrobin.c
+++ b/drivers/net/team/team_mode_roundrobin.c
@@ -32,6 +32,8 @@ static bool rr_transmit(struct team *team, struct sk_buff *skb)
 
 	port_index = rr_priv(team)->sent_packets++ % team->en_port_count;
 	port = team_get_port_by_index_rcu(team, port_index);
+	if (unlikely(!port))
+		goto drop;
 	port = team_get_first_port_txable_rcu(team, port);
 	if (unlikely(!port))
 		goto drop;
-- 
1.7.11.7

[patch net 2/3] team: move add to port list before port enablement

[Jiri Pirko]

team_port_enable() adds port to port_hashlist. Reader sees port
in team_get_port_by_index_rcu() and returns it, but
team_get_first_port_txable_rcu() tries to go through port_list, where the
port is not inserted yet -> NULL pointer dereference.
Fix this by reordering port_list and port_hashlist insertion.
Panic is easily triggeable when txing packets and adding/removing port
in a loop.

Introduced by commit 3d249d4c "net: introduce ethernet teaming device"

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/team/team.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index d016a76..b305105 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1092,8 +1092,8 @@ static int team_port_add(struct team *team, struct net_device *port_dev)
 	}
 
 	port->index = -1;
-	team_port_enable(team, port);
 	list_add_tail_rcu(&port->list, &team->port_list);
+	team_port_enable(team, port);
 	__team_compute_features(team);
 	__team_port_change_port_added(port, !!netif_carrier_ok(port_dev));
 	__team_options_change_check(team);
-- 
1.7.11.7

[patch net 3/3] team: fix checks in team_get_first_port_txable_rcu()

[Jiri Pirko]

should be checked if "cur" is txable, not "port".

Introduced by commit 6e88e1357c "team: use function team_port_txable()
for determing enabled and up port"

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/linux/if_team.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index 4474557..16fae64 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -249,12 +249,12 @@ team_get_first_port_txable_rcu(struct team *team, struct team_port *port)
 		return port;
 	cur = port;
 	list_for_each_entry_continue_rcu(cur, &team->port_list, list)
-		if (team_port_txable(port))
+		if (team_port_txable(cur))
 			return cur;
 	list_for_each_entry_rcu(cur, &team->port_list, list) {
 		if (cur == port)
 			break;
-		if (team_port_txable(port))
+		if (team_port_txable(cur))
 			return cur;
 	}
 	return NULL;
-- 
1.7.11.7

Re: [patch net 0/3] team: couple of fixes

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Sat,  8 Jun 2013 15:00:52 +0200

> Jiri Pirko (3):
>   team: check return value of team_get_port_by_index_rcu() for NULL
>   team: move add to port list before port enablement
>   team: fix checks in team_get_first_port_txable_rcu()

Series applied, thanks Jiri.

Re: [patch net 0/3] team: couple of fixes

[Jiri Pirko]

Wed, Jun 12, 2013 at 09:57:24AM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Sat,  8 Jun 2013 15:00:52 +0200
>
>> Jiri Pirko (3):
>>   team: check return value of team_get_port_by_index_rcu() for NULL
>>   team: move add to port list before port enablement
>>   team: fix checks in team_get_first_port_txable_rcu()
>
>Series applied, thanks Jiri.

Thanks Dave. Please note that this is suitable for stable trees as well.

Re: [patch net 0/3] team: couple of fixes

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 12 Jun 2013 10:58:40 +0200

> Wed, Jun 12, 2013 at 09:57:24AM CEST, davem@davemloft.net wrote:
>>From: Jiri Pirko <jiri@resnulli.us>
>>Date: Sat,  8 Jun 2013 15:00:52 +0200
>>
>>> Jiri Pirko (3):
>>>   team: check return value of team_get_port_by_index_rcu() for NULL
>>>   team: move add to port list before port enablement
>>>   team: fix checks in team_get_first_port_txable_rcu()
>>
>>Series applied, thanks Jiri.
> 
> Thanks Dave. Please note that this is suitable for stable trees as well.

Queued up, thanks for letting me know.