Thread (8 messages) flat view 8 messages, 2 authors, 2017-01-05
STALE3497d

Revision v1 of 139 in this series.

Revisions (139)
  1. v1 [diff vs current]
  2. v1 [diff vs current]
  3. v1 [diff vs current]
  4. v1 [diff vs current]
  5. v1 [diff vs current]
  6. v1 [diff vs current]
  7. v1 [diff vs current]
  8. v1 [diff vs current]
  9. v1 [diff vs current]
  10. v1 [diff vs current]
  11. v1 [diff vs current]
  12. v1 [diff vs current]
  13. v1 [diff vs current]
  14. v1 [diff vs current]
  15. v1 [diff vs current]
  16. v1 [diff vs current]
  17. v1 [diff vs current]
  18. v1 [diff vs current]
  19. v1 [diff vs current]
  20. v1 [diff vs current]
  21. v1 [diff vs current]
  22. v1 [diff vs current]
  23. v1 current
  24. v1 [diff vs current]
  25. v1 [diff vs current]
  26. v1 [diff vs current]
  27. v1 [diff vs current]
  28. v1 [diff vs current]
  29. v1 [diff vs current]
  30. v1 [diff vs current]
  31. v1 [diff vs current]
  32. v1 [diff vs current]
  33. v1 [diff vs current]
  34. v1 [diff vs current]
  35. v1 [diff vs current]
  36. v1 [diff vs current]
  37. v1 [diff vs current]
  38. v1 [diff vs current]
  39. v1 [diff vs current]
  40. v1 [diff vs current]
  41. v1 [diff vs current]
  42. v1 [diff vs current]
  43. v1 [diff vs current]
  44. v1 [diff vs current]
  45. v1 [diff vs current]
  46. v1 [diff vs current]
  47. v1 [diff vs current]
  48. v1 [diff vs current]
  49. v1 [diff vs current]
  50. v1 [diff vs current]
  51. v1 [diff vs current]
  52. v1 [diff vs current]
  53. v1 [diff vs current]
  54. v1 [diff vs current]
  55. v1 [diff vs current]
  56. v1 [diff vs current]
  57. v1 [diff vs current]
  58. v1 [diff vs current]
  59. v1 [diff vs current]
  60. v1 [diff vs current]
  61. v1 [diff vs current]
  62. v1 [diff vs current]
  63. v1 [diff vs current]
  64. v1 [diff vs current]
  65. v1 [diff vs current]
  66. v1 [diff vs current]
  67. v1 [diff vs current]
  68. v1 [diff vs current]
  69. v1 [diff vs current]
  70. v1 [diff vs current]
  71. v1 [diff vs current]
  72. v1 [diff vs current]
  73. v1 [diff vs current]
  74. v1 [diff vs current]
  75. v1 [diff vs current]
  76. v1 [diff vs current]
  77. v1 [diff vs current]
  78. v1 [diff vs current]
  79. v1 [diff vs current]
  80. v1 [diff vs current]
  81. v2 [diff vs current]
  82. v1 [diff vs current]
  83. v1 [diff vs current]
  84. v1 [diff vs current]
  85. v1 [diff vs current]
  86. v1 [diff vs current]
  87. v1 [diff vs current]
  88. v1 [diff vs current]
  89. v1 [diff vs current]
  90. v1 [diff vs current]
  91. v2 [diff vs current]
  92. v1 [diff vs current]
  93. v1 [diff vs current]
  94. v1 [diff vs current]
  95. v1 [diff vs current]
  96. v1 [diff vs current]
  97. v1 [diff vs current]
  98. v1 [diff vs current]
  99. v1 [diff vs current]
  100. v1 [diff vs current]
  101. v1 [diff vs current]
  102. v1 [diff vs current]
  103. v1 [diff vs current]
  104. v1 [diff vs current]
  105. v1 [diff vs current]
  106. v1 [diff vs current]
  107. v1 [diff vs current]
  108. v1 [diff vs current]
  109. v1 [diff vs current]
  110. v1 [diff vs current]
  111. v2 [diff vs current]
  112. v1 [diff vs current]
  113. v1 [diff vs current]
  114. v1 [diff vs current]
  115. v2 [diff vs current]
  116. v1 [diff vs current]
  117. v1 [diff vs current]
  118. v1 [diff vs current]
  119. v1 [diff vs current]
  120. v1 [diff vs current]
  121. v2 [diff vs current]
  122. v3 [diff vs current]
  123. v1 [diff vs current]
  124. v1 [diff vs current]
  125. v1 [diff vs current]
  126. v2 [diff vs current]
  127. v1 [diff vs current]
  128. v1 [diff vs current]
  129. v1 [diff vs current]
  130. v1 [diff vs current]
  131. v2 [diff vs current]
  132. v1 [diff vs current]
  133. v2 [diff vs current]
  134. v1 [diff vs current]
  135. v1 [diff vs current]
  136. v1 [diff vs current]
  137. v1 [diff vs current]
  138. v2 [diff vs current]
  139. v1 [diff vs current]

[PATCH 5/6] netfilter: ipt_CLUSTERIP: check duplicate config when initializing

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: 2017-01-05 11:20:12
Also in: netfilter-devel
Subsystem: networking [general], the rest · Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds 

From: Xin Long <lucien.xin@gmail.com>

Now when adding an ipt_CLUSTERIP rule, it only checks duplicate config in
clusterip_config_find_get(). But after that, there may be still another
thread to insert a config with the same ip, then it leaves proc_create_data
to do duplicate check.

It's more reasonable to check duplicate config by ipt_CLUSTERIP itself,
instead of checking it by proc fs duplicate file check. Before, when proc
fs allowed duplicate name files in a directory, It could even crash kernel
because of use-after-free.

This patch is to check duplicate config under the protection of clusterip
net lock when initializing a new config and correct the return err.

Note that it also moves proc file node creation after adding new config, as
proc_create_data may sleep, it couldn't be called under the clusterip_net
lock. clusterip_config_find_get returns NULL if c->pde is null to make sure
it can't be used until the proc file node creation is done.

Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv4/netfilter/ipt_CLUSTERIP.c | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index 21db00d0362b..a6b8c1a4102b 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -144,7 +144,7 @@ clusterip_config_find_get(struct net *net, __be32 clusterip, int entry)
 	rcu_read_lock_bh();
 	c = __clusterip_config_find(net, clusterip);
 	if (c) {
-		if (unlikely(!atomic_inc_not_zero(&c->refcount)))
+		if (!c->pde || unlikely(!atomic_inc_not_zero(&c->refcount)))
 			c = NULL;
 		else if (entry)
 			atomic_inc(&c->entries);
@@ -166,14 +166,15 @@ clusterip_config_init_nodelist(struct clusterip_config *c,
 
 static struct clusterip_config *
 clusterip_config_init(const struct ipt_clusterip_tgt_info *i, __be32 ip,
-			struct net_device *dev)
+		      struct net_device *dev)
 {
+	struct net *net = dev_net(dev);
 	struct clusterip_config *c;
-	struct clusterip_net *cn = net_generic(dev_net(dev), clusterip_net_id);
+	struct clusterip_net *cn = net_generic(net, clusterip_net_id);
 
 	c = kzalloc(sizeof(*c), GFP_ATOMIC);
 	if (!c)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	c->dev = dev;
 	c->clusterip = ip;
@@ -185,6 +186,17 @@ clusterip_config_init(const struct ipt_clusterip_tgt_info *i, __be32 ip,
 	atomic_set(&c->refcount, 1);
 	atomic_set(&c->entries, 1);
 
+	spin_lock_bh(&cn->lock);
+	if (__clusterip_config_find(net, ip)) {
+		spin_unlock_bh(&cn->lock);
+		kfree(c);
+
+		return ERR_PTR(-EBUSY);
+	}
+
+	list_add_rcu(&c->list, &cn->configs);
+	spin_unlock_bh(&cn->lock);
+
 #ifdef CONFIG_PROC_FS
 	{
 		char buffer[16];
@@ -195,16 +207,16 @@ clusterip_config_init(const struct ipt_clusterip_tgt_info *i, __be32 ip,
 					  cn->procdir,
 					  &clusterip_proc_fops, c);
 		if (!c->pde) {
+			spin_lock_bh(&cn->lock);
+			list_del_rcu(&c->list);
+			spin_unlock_bh(&cn->lock);
 			kfree(c);
-			return NULL;
+
+			return ERR_PTR(-ENOMEM);
 		}
 	}
 #endif
 
-	spin_lock_bh(&cn->lock);
-	list_add_rcu(&c->list, &cn->configs);
-	spin_unlock_bh(&cn->lock);
-
 	return c;
 }
 
@@ -410,9 +422,9 @@ static int clusterip_tg_check(const struct xt_tgchk_param *par)
 
 			config = clusterip_config_init(cipinfo,
 							e->ip.dst.s_addr, dev);
-			if (!config) {
+			if (IS_ERR(config)) {
 				dev_put(dev);
-				return -ENOMEM;
+				return PTR_ERR(config);
 			}
 			dev_mc_add(config->dev, config->clustermac);
 		}
-- 
2.1.4
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help