mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 02:30:34 +02:00 
			
		
		
		
	net sched: fix race in mirred device removal
This fixes hang when target device of mirred packet classifier action is removed. If a mirror or redirection action is configured to cause packets to go to another device, the classifier holds a ref count, but was assuming the adminstrator cleaned up all redirections before removing. The fix is to add a notifier and cleanup during unregister. The new list is implicitly protected by RTNL mutex because it is held during filter add/delete as well as notifier. Signed-off-by: Stephen Hemminger <shemminger@vyatta.com> Acked-by: Jamal Hadi Salim <hadi@cyberus.ca> Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
		
							parent
							
								
									76ac21f5ef
								
							
						
					
					
						commit
						3b87956ea6
					
				
					 2 changed files with 41 additions and 3 deletions
				
			
		| 
						 | 
					@ -9,6 +9,7 @@ struct tcf_mirred {
 | 
				
			||||||
	int			tcfm_ifindex;
 | 
						int			tcfm_ifindex;
 | 
				
			||||||
	int			tcfm_ok_push;
 | 
						int			tcfm_ok_push;
 | 
				
			||||||
	struct net_device	*tcfm_dev;
 | 
						struct net_device	*tcfm_dev;
 | 
				
			||||||
 | 
						struct list_head	tcfm_list;
 | 
				
			||||||
};
 | 
					};
 | 
				
			||||||
#define to_mirred(pc) \
 | 
					#define to_mirred(pc) \
 | 
				
			||||||
	container_of(pc, struct tcf_mirred, common)
 | 
						container_of(pc, struct tcf_mirred, common)
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -33,6 +33,7 @@
 | 
				
			||||||
static struct tcf_common *tcf_mirred_ht[MIRRED_TAB_MASK + 1];
 | 
					static struct tcf_common *tcf_mirred_ht[MIRRED_TAB_MASK + 1];
 | 
				
			||||||
static u32 mirred_idx_gen;
 | 
					static u32 mirred_idx_gen;
 | 
				
			||||||
static DEFINE_RWLOCK(mirred_lock);
 | 
					static DEFINE_RWLOCK(mirred_lock);
 | 
				
			||||||
 | 
					static LIST_HEAD(mirred_list);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static struct tcf_hashinfo mirred_hash_info = {
 | 
					static struct tcf_hashinfo mirred_hash_info = {
 | 
				
			||||||
	.htab	=	tcf_mirred_ht,
 | 
						.htab	=	tcf_mirred_ht,
 | 
				
			||||||
| 
						 | 
					@ -47,7 +48,9 @@ static inline int tcf_mirred_release(struct tcf_mirred *m, int bind)
 | 
				
			||||||
			m->tcf_bindcnt--;
 | 
								m->tcf_bindcnt--;
 | 
				
			||||||
		m->tcf_refcnt--;
 | 
							m->tcf_refcnt--;
 | 
				
			||||||
		if(!m->tcf_bindcnt && m->tcf_refcnt <= 0) {
 | 
							if(!m->tcf_bindcnt && m->tcf_refcnt <= 0) {
 | 
				
			||||||
			dev_put(m->tcfm_dev);
 | 
								list_del(&m->tcfm_list);
 | 
				
			||||||
 | 
								if (m->tcfm_dev)
 | 
				
			||||||
 | 
									dev_put(m->tcfm_dev);
 | 
				
			||||||
			tcf_hash_destroy(&m->common, &mirred_hash_info);
 | 
								tcf_hash_destroy(&m->common, &mirred_hash_info);
 | 
				
			||||||
			return 1;
 | 
								return 1;
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
| 
						 | 
					@ -134,8 +137,10 @@ static int tcf_mirred_init(struct nlattr *nla, struct nlattr *est,
 | 
				
			||||||
		m->tcfm_ok_push = ok_push;
 | 
							m->tcfm_ok_push = ok_push;
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	spin_unlock_bh(&m->tcf_lock);
 | 
						spin_unlock_bh(&m->tcf_lock);
 | 
				
			||||||
	if (ret == ACT_P_CREATED)
 | 
						if (ret == ACT_P_CREATED) {
 | 
				
			||||||
 | 
							list_add(&m->tcfm_list, &mirred_list);
 | 
				
			||||||
		tcf_hash_insert(pc, &mirred_hash_info);
 | 
							tcf_hash_insert(pc, &mirred_hash_info);
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	return ret;
 | 
						return ret;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
| 
						 | 
					@ -162,9 +167,14 @@ static int tcf_mirred(struct sk_buff *skb, struct tc_action *a,
 | 
				
			||||||
	m->tcf_tm.lastuse = jiffies;
 | 
						m->tcf_tm.lastuse = jiffies;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	dev = m->tcfm_dev;
 | 
						dev = m->tcfm_dev;
 | 
				
			||||||
 | 
						if (!dev) {
 | 
				
			||||||
 | 
							printk_once(KERN_NOTICE "tc mirred: target device is gone\n");
 | 
				
			||||||
 | 
							goto out;
 | 
				
			||||||
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	if (!(dev->flags & IFF_UP)) {
 | 
						if (!(dev->flags & IFF_UP)) {
 | 
				
			||||||
		if (net_ratelimit())
 | 
							if (net_ratelimit())
 | 
				
			||||||
			pr_notice("tc mirred to Houston: device %s is gone!\n",
 | 
								pr_notice("tc mirred to Houston: device %s is down\n",
 | 
				
			||||||
				  dev->name);
 | 
									  dev->name);
 | 
				
			||||||
		goto out;
 | 
							goto out;
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
| 
						 | 
					@ -232,6 +242,28 @@ static int tcf_mirred_dump(struct sk_buff *skb, struct tc_action *a, int bind, i
 | 
				
			||||||
	return -1;
 | 
						return -1;
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					static int mirred_device_event(struct notifier_block *unused,
 | 
				
			||||||
 | 
								       unsigned long event, void *ptr)
 | 
				
			||||||
 | 
					{
 | 
				
			||||||
 | 
						struct net_device *dev = ptr;
 | 
				
			||||||
 | 
						struct tcf_mirred *m;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						if (event == NETDEV_UNREGISTER)
 | 
				
			||||||
 | 
							list_for_each_entry(m, &mirred_list, tcfm_list) {
 | 
				
			||||||
 | 
								if (m->tcfm_dev == dev) {
 | 
				
			||||||
 | 
									dev_put(dev);
 | 
				
			||||||
 | 
									m->tcfm_dev = NULL;
 | 
				
			||||||
 | 
								}
 | 
				
			||||||
 | 
							}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						return NOTIFY_DONE;
 | 
				
			||||||
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					static struct notifier_block mirred_device_notifier = {
 | 
				
			||||||
 | 
						.notifier_call = mirred_device_event,
 | 
				
			||||||
 | 
					};
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static struct tc_action_ops act_mirred_ops = {
 | 
					static struct tc_action_ops act_mirred_ops = {
 | 
				
			||||||
	.kind		=	"mirred",
 | 
						.kind		=	"mirred",
 | 
				
			||||||
	.hinfo		=	&mirred_hash_info,
 | 
						.hinfo		=	&mirred_hash_info,
 | 
				
			||||||
| 
						 | 
					@ -252,12 +284,17 @@ MODULE_LICENSE("GPL");
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static int __init mirred_init_module(void)
 | 
					static int __init mirred_init_module(void)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
 | 
						int err = register_netdevice_notifier(&mirred_device_notifier);
 | 
				
			||||||
 | 
						if (err)
 | 
				
			||||||
 | 
							return err;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	pr_info("Mirror/redirect action on\n");
 | 
						pr_info("Mirror/redirect action on\n");
 | 
				
			||||||
	return tcf_register_action(&act_mirred_ops);
 | 
						return tcf_register_action(&act_mirred_ops);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
static void __exit mirred_cleanup_module(void)
 | 
					static void __exit mirred_cleanup_module(void)
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
 | 
						unregister_netdevice_notifier(&mirred_device_notifier);
 | 
				
			||||||
	tcf_unregister_action(&act_mirred_ops);
 | 
						tcf_unregister_action(&act_mirred_ops);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue