[Devel] [PATCH 04/14] Subject: ve/net/bridge: don't set NULL in skb->dev

[Andrew Vagin]

On Mon, Jun 08, 2015 at 05:20:39PM +0300, Kirill Tkhai wrote:
> Porting patches diff-ve-net-bridge-dont-set-NULL-in-skb-dev
> and diff-ve-net-bridge-dont-forget-to-init-master_dev from 2.6.32:
> 
> skb->dev can't be NULL, because it is accessed from netfilters
> without testing.
> 
> In the upstream code the bridge devices is used. We have
> the via_phys_dev feature, but the master_dev is accesses
> without locks, so here is a race.
> 
> This patch allows safely assess to master_dev in rcu context.

We are going to remove the via_phys_dev mode. Will we need this patch in
this case?

> 
> https://jira.sw.ru/browse/PSBM-24056
> 
> Signed-off-by: Andrey Vagin <avagin at openvz.org>
> Acked-by: Stanislav Kinsbursky <skinsbursky at parallels.com>
> Signed-off-by: Kirill Tkhai <ktkhai at odin.com>
> ---
>  net/bridge/br_if.c        |    8 ++++----
>  net/bridge/br_input.c     |   14 ++++++++++----
>  net/bridge/br_netfilter.c |    4 +++-
>  3 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index a357ce8..21d815b 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -171,7 +171,7 @@ void br_dev_delete(struct net_device *dev, struct list_head *head)
>  
>  	if (br->master_dev) {
>  		dev_put(br->master_dev);
> -		br->master_dev = NULL;
> +		rcu_assign_pointer(br->master_dev, NULL);
>  	}
>  
>  	list_for_each_entry_safe(p, n, &br->port_list, list) {
> @@ -399,7 +399,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
>  		br_stp_enable_port(p);
>  	if (!(dev->features & NETIF_F_VIRTUAL) && !br->master_dev) {
>  		dev_hold(dev);
> -		br->master_dev = dev;
> +		rcu_assign_pointer(br->master_dev, dev);
>  	}
>  	spin_unlock_bh(&br->lock);
>  
> @@ -453,12 +453,12 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
>  	spin_lock_bh(&br->lock);
>  	changed_addr = br_stp_recalculate_bridge_id(br);
>  	if (br->master_dev == dev) {
> -		br->master_dev = NULL;
> +		rcu_assign_pointer(br->master_dev, NULL);
>  		dev_put(dev);
>  		list_for_each_entry(p, &br->port_list, list)
>  			if (!(p->dev->features & NETIF_F_VIRTUAL)) {
>  				dev_hold(p->dev);
> -				br->master_dev = p->dev;
> +				rcu_assign_pointer(br->master_dev, p->dev);
>  				break;
>  			}
>  	}
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 15f99fe..82f020a 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -28,6 +28,7 @@ static int br_pass_frame_up(struct net_bridge *br, struct sk_buff *skb)
>  {
>  	struct net_device *indev, *brdev = BR_INPUT_SKB_CB(skb)->brdev;
>  	struct br_cpu_netstats *brstats = this_cpu_ptr(br->stats);
> +	struct net_device *master_dev = NULL;
>  
>  	u64_stats_update_begin(&brstats->syncp);
>  	brstats->rx_packets++;
> @@ -49,12 +50,14 @@ static int br_pass_frame_up(struct net_bridge *br, struct sk_buff *skb)
>  		return NET_RX_DROP;
>  
>  	indev = skb->dev;
> -	if (!br->via_phys_dev)
> +
> +	if (br->via_phys_dev)
> +		master_dev = rcu_dereference(br->master_dev);
> +	if (!master_dev)
>  		skb->dev = brdev;
>  	else {
>  		skb->brmark = BR_ALREADY_SEEN;
> -		if (br->master_dev)
> -			skb->dev = br->master_dev;
> +		skb->dev = master_dev;
>  	}
>  
>  	return NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN, skb, indev, NULL,
> @@ -239,7 +242,10 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
>  		if (skb->brmark == BR_ALREADY_SEEN)
>  			return RX_HANDLER_PASS;
>  
> -		out = p->br->via_phys_dev ? p->br->master_dev : p->br->dev;
> +		if (p->br->via_phys_dev)
> +			out = rcu_dereference(p->br->master_dev);
> +		else
> +			out = p->br->dev;
>  
>  		if (out && ether_addr_equal(out->dev_addr, dest))
>  			skb->pkt_type = PACKET_HOST;
> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
> index 0afdf3e..7ddb9ad 100644
> --- a/net/bridge/br_netfilter.c
> +++ b/net/bridge/br_netfilter.c
> @@ -181,6 +181,7 @@ static inline struct rtable *bridge_parent_rtable(const struct net_device *dev)
>  
>  static inline struct net_device *bridge_parent(const struct net_device *dev)
>  {
> +	struct net_device *master_dev;
>  	struct net_bridge_port *port;
>  	struct net_bridge *br;
>  
> @@ -189,7 +190,8 @@ static inline struct net_device *bridge_parent(const struct net_device *dev)
>  		return NULL;
>  
>  	br = port->br;
> -	if (br->via_phys_dev && br->master_dev)
> +	master_dev = rcu_dereference(br->master_dev);
> +	if (br->via_phys_dev && master_dev)
>  		return br->master_dev;
>  	else
>  		return br->dev;
> 
> _______________________________________________
> Devel mailing list
> Devel at openvz.org
> https://lists.openvz.org/mailman/listinfo/devel