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

[Kirill Tkhai]

В Вт, 09/06/2015 в 14:57 +0300, 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?

It's not need if so, because it's for via_phys_dev mode only. 

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