Thread (5 messages) 5 messages, 1 author, 2014-08-11
DORMANTno replies REVIEWED: 1 (1M)
Revisions (202)
  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 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 [diff vs 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. v1 [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. v1 [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. v1 [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. v1 [diff vs current]
  122. v1 [diff vs current]
  123. v2 [diff vs current]
  124. v1 [diff vs current]
  125. v1 [diff vs current]
  126. v1 [diff vs current]
  127. v1 [diff vs current]
  128. v1 [diff vs current]
  129. v1 [diff vs current]
  130. v1 [diff vs current]
  131. v1 [diff vs current]
  132. v1 [diff vs current]
  133. v1 [diff vs current]
  134. v1 [diff vs current]
  135. v1 [diff vs current]
  136. v1 [diff vs current]
  137. v1 [diff vs current]
  138. v1 [diff vs current]
  139. v1 [diff vs current]
  140. v1 [diff vs current]
  141. v1 [diff vs current]
  142. v1 [diff vs current]
  143. v1 [diff vs current]
  144. v1 [diff vs current]
  145. v1 [diff vs current]
  146. v1 [diff vs current]
  147. v1 [diff vs current]
  148. v1 [diff vs current]
  149. v1 [diff vs current]
  150. v1 [diff vs current]
  151. v1 [diff vs current]
  152. v1 [diff vs current]
  153. v1 [diff vs current]
  154. v1 [diff vs current]
  155. v1 [diff vs current]
  156. v1 [diff vs current]
  157. v1 [diff vs current]
  158. v1 [diff vs current]
  159. v1 [diff vs current]
  160. v2 [diff vs current]
  161. v1 [diff vs current]
  162. v1 [diff vs current]
  163. v1 [diff vs current]
  164. v2 [diff vs current]
  165. v1 [diff vs current]
  166. v1 [diff vs current]
  167. v1 [diff vs current]
  168. v1 [diff vs current]
  169. v2 [diff vs current]
  170. v1 [diff vs current]
  171. v1 [diff vs current]
  172. v1 [diff vs current]
  173. v1 [diff vs current]
  174. v3 [diff vs current]
  175. v1 [diff vs current]
  176. v1 [diff vs current]
  177. v1 [diff vs current]
  178. v1 [diff vs current]
  179. v1 [diff vs current]
  180. v2 [diff vs current]
  181. v1 [diff vs current]
  182. v1 [diff vs current]
  183. v1 [diff vs current]
  184. v1 [diff vs current]
  185. v1 [diff vs current]
  186. v2 [diff vs current]
  187. v1 [diff vs current]
  188. v1 [diff vs current]
  189. v1 [diff vs current]
  190. v1 [diff vs current]
  191. v1 [diff vs current]
  192. v1 [diff vs current]
  193. v1 [diff vs current]
  194. v2 [diff vs current]
  195. v1 [diff vs current]
  196. v2 [diff vs current]
  197. v1 [diff vs current]
  198. v1 [diff vs current]
  199. v1 [diff vs current]
  200. v1 [diff vs current]
  201. v2 [diff vs current]
  202. v1 [diff vs current]

[PATCH 3/4] netfilter: don't use mutex_lock_interruptible()

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: 2014-08-11 17:06:18
Also in: netfilter-devel
Subsystem: ethernet bridge, ipvs, netfilter, networking [general], the rest · Maintainers: Nikolay Aleksandrov, Ido Schimmel, Simon Horman, Julian Anastasov, Pablo Neira Ayuso, Florian Westphal, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

Eric Dumazet reports that getsockopt() or setsockopt() sometimes
returns -EINTR instead of -ENOPROTOOPT, causing headaches to
application developers.

This patch replaces all the mutex_lock_interruptible() by mutex_lock()
in the netfilter tree, as there is no reason we should sleep for a
long time there.

Reported-by: Eric Dumazet <edumazet@google.com>
Suggested-by: Patrick McHardy <redacted>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Acked-by: Julian Anastasov <ja@ssi.bg>
---
 net/bridge/netfilter/ebtables.c |   10 ++-------
 net/netfilter/core.c            |   11 ++-------
 net/netfilter/ipvs/ip_vs_ctl.c  |   19 ++++------------
 net/netfilter/nf_sockopt.c      |    8 ++-----
 net/netfilter/x_tables.c        |   47 ++++++++++-----------------------------
 5 files changed, 22 insertions(+), 73 deletions(-)
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 1059ed3..6d69631 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -327,10 +327,7 @@ find_inlist_lock_noload(struct list_head *head, const char *name, int *error,
 		char name[EBT_FUNCTION_MAXNAMELEN];
 	} *e;
 
-	*error = mutex_lock_interruptible(mutex);
-	if (*error != 0)
-		return NULL;
-
+	mutex_lock(mutex);
 	list_for_each_entry(e, head, list) {
 		if (strcmp(e->name, name) == 0)
 			return e;
@@ -1203,10 +1200,7 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table)
 
 	table->private = newinfo;
 	rwlock_init(&table->lock);
-	ret = mutex_lock_interruptible(&ebt_mutex);
-	if (ret != 0)
-		goto free_chainstack;
-
+	mutex_lock(&ebt_mutex);
 	list_for_each_entry(t, &net->xt.tables[NFPROTO_BRIDGE], list) {
 		if (strcmp(t->name, table->name) == 0) {
 			ret = -EEXIST;
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 1fbab0c..a93c97f 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -35,11 +35,7 @@ EXPORT_SYMBOL_GPL(nf_ipv6_ops);
 
 int nf_register_afinfo(const struct nf_afinfo *afinfo)
 {
-	int err;
-
-	err = mutex_lock_interruptible(&afinfo_mutex);
-	if (err < 0)
-		return err;
+	mutex_lock(&afinfo_mutex);
 	RCU_INIT_POINTER(nf_afinfo[afinfo->family], afinfo);
 	mutex_unlock(&afinfo_mutex);
 	return 0;
@@ -68,11 +64,8 @@ static DEFINE_MUTEX(nf_hook_mutex);
 int nf_register_hook(struct nf_hook_ops *reg)
 {
 	struct nf_hook_ops *elem;
-	int err;
 
-	err = mutex_lock_interruptible(&nf_hook_mutex);
-	if (err < 0)
-		return err;
+	mutex_lock(&nf_hook_mutex);
 	list_for_each_entry(elem, &nf_hooks[reg->pf][reg->hooknum], list) {
 		if (reg->priority < elem->priority)
 			break;
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 8416307..fd3f444 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -2271,10 +2271,7 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, void __user *user, unsigned int len)
 	    cmd == IP_VS_SO_SET_STOPDAEMON) {
 		struct ip_vs_daemon_user *dm = (struct ip_vs_daemon_user *)arg;
 
-		if (mutex_lock_interruptible(&ipvs->sync_mutex)) {
-			ret = -ERESTARTSYS;
-			goto out_dec;
-		}
+		mutex_lock(&ipvs->sync_mutex);
 		if (cmd == IP_VS_SO_SET_STARTDAEMON)
 			ret = start_sync_thread(net, dm->state, dm->mcast_ifn,
 						dm->syncid);
@@ -2284,11 +2281,7 @@ do_ip_vs_set_ctl(struct sock *sk, int cmd, void __user *user, unsigned int len)
 		goto out_dec;
 	}
 
-	if (mutex_lock_interruptible(&__ip_vs_mutex)) {
-		ret = -ERESTARTSYS;
-		goto out_dec;
-	}
-
+	mutex_lock(&__ip_vs_mutex);
 	if (cmd == IP_VS_SO_SET_FLUSH) {
 		/* Flush the virtual service */
 		ret = ip_vs_flush(net, false);
@@ -2573,9 +2566,7 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
 		struct ip_vs_daemon_user d[2];
 
 		memset(&d, 0, sizeof(d));
-		if (mutex_lock_interruptible(&ipvs->sync_mutex))
-			return -ERESTARTSYS;
-
+		mutex_lock(&ipvs->sync_mutex);
 		if (ipvs->sync_state & IP_VS_STATE_MASTER) {
 			d[0].state = IP_VS_STATE_MASTER;
 			strlcpy(d[0].mcast_ifn, ipvs->master_mcast_ifn,
@@ -2594,9 +2585,7 @@ do_ip_vs_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
 		return ret;
 	}
 
-	if (mutex_lock_interruptible(&__ip_vs_mutex))
-		return -ERESTARTSYS;
-
+	mutex_lock(&__ip_vs_mutex);
 	switch (cmd) {
 	case IP_VS_SO_GET_VERSION:
 	{
diff --git a/net/netfilter/nf_sockopt.c b/net/netfilter/nf_sockopt.c
index f042ae5..c68c1e5 100644
--- a/net/netfilter/nf_sockopt.c
+++ b/net/netfilter/nf_sockopt.c
@@ -26,9 +26,7 @@ int nf_register_sockopt(struct nf_sockopt_ops *reg)
 	struct nf_sockopt_ops *ops;
 	int ret = 0;
 
-	if (mutex_lock_interruptible(&nf_sockopt_mutex) != 0)
-		return -EINTR;
-
+	mutex_lock(&nf_sockopt_mutex);
 	list_for_each_entry(ops, &nf_sockopts, list) {
 		if (ops->pf == reg->pf
 		    && (overlap(ops->set_optmin, ops->set_optmax,
@@ -65,9 +63,7 @@ static struct nf_sockopt_ops *nf_sockopt_find(struct sock *sk, u_int8_t pf,
 {
 	struct nf_sockopt_ops *ops;
 
-	if (mutex_lock_interruptible(&nf_sockopt_mutex) != 0)
-		return ERR_PTR(-EINTR);
-
+	mutex_lock(&nf_sockopt_mutex);
 	list_for_each_entry(ops, &nf_sockopts, list) {
 		if (ops->pf == pf) {
 			if (!try_module_get(ops->owner))
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 47b978b..272ae4d 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -71,18 +71,14 @@ static const char *const xt_prefix[NFPROTO_NUMPROTO] = {
 static const unsigned int xt_jumpstack_multiplier = 2;
 
 /* Registration hooks for targets. */
-int
-xt_register_target(struct xt_target *target)
+int xt_register_target(struct xt_target *target)
 {
 	u_int8_t af = target->family;
-	int ret;
 
-	ret = mutex_lock_interruptible(&xt[af].mutex);
-	if (ret != 0)
-		return ret;
+	mutex_lock(&xt[af].mutex);
 	list_add(&target->list, &xt[af].target);
 	mutex_unlock(&xt[af].mutex);
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL(xt_register_target);
 
@@ -125,20 +121,14 @@ xt_unregister_targets(struct xt_target *target, unsigned int n)
 }
 EXPORT_SYMBOL(xt_unregister_targets);
 
-int
-xt_register_match(struct xt_match *match)
+int xt_register_match(struct xt_match *match)
 {
 	u_int8_t af = match->family;
-	int ret;
-
-	ret = mutex_lock_interruptible(&xt[af].mutex);
-	if (ret != 0)
-		return ret;
 
+	mutex_lock(&xt[af].mutex);
 	list_add(&match->list, &xt[af].match);
 	mutex_unlock(&xt[af].mutex);
-
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL(xt_register_match);
 
@@ -194,9 +184,7 @@ struct xt_match *xt_find_match(u8 af, const char *name, u8 revision)
 	struct xt_match *m;
 	int err = -ENOENT;
 
-	if (mutex_lock_interruptible(&xt[af].mutex) != 0)
-		return ERR_PTR(-EINTR);
-
+	mutex_lock(&xt[af].mutex);
 	list_for_each_entry(m, &xt[af].match, list) {
 		if (strcmp(m->name, name) == 0) {
 			if (m->revision == revision) {
@@ -239,9 +227,7 @@ struct xt_target *xt_find_target(u8 af, const char *name, u8 revision)
 	struct xt_target *t;
 	int err = -ENOENT;
 
-	if (mutex_lock_interruptible(&xt[af].mutex) != 0)
-		return ERR_PTR(-EINTR);
-
+	mutex_lock(&xt[af].mutex);
 	list_for_each_entry(t, &xt[af].target, list) {
 		if (strcmp(t->name, name) == 0) {
 			if (t->revision == revision) {
@@ -323,10 +309,7 @@ int xt_find_revision(u8 af, const char *name, u8 revision, int target,
 {
 	int have_rev, best = -1;
 
-	if (mutex_lock_interruptible(&xt[af].mutex) != 0) {
-		*err = -EINTR;
-		return 1;
-	}
+	mutex_lock(&xt[af].mutex);
 	if (target == 1)
 		have_rev = target_revfn(af, name, revision, &best);
 	else
@@ -732,9 +715,7 @@ struct xt_table *xt_find_table_lock(struct net *net, u_int8_t af,
 {
 	struct xt_table *t;
 
-	if (mutex_lock_interruptible(&xt[af].mutex) != 0)
-		return ERR_PTR(-EINTR);
-
+	mutex_lock(&xt[af].mutex);
 	list_for_each_entry(t, &net->xt.tables[af], list)
 		if (strcmp(t->name, name) == 0 && try_module_get(t->me))
 			return t;
@@ -883,10 +864,7 @@ struct xt_table *xt_register_table(struct net *net,
 		goto out;
 	}
 
-	ret = mutex_lock_interruptible(&xt[table->af].mutex);
-	if (ret != 0)
-		goto out_free;
-
+	mutex_lock(&xt[table->af].mutex);
 	/* Don't autoload: we'd eat our tail... */
 	list_for_each_entry(t, &net->xt.tables[table->af], list) {
 		if (strcmp(t->name, table->name) == 0) {
@@ -911,9 +889,8 @@ struct xt_table *xt_register_table(struct net *net,
 	mutex_unlock(&xt[table->af].mutex);
 	return table;
 
- unlock:
+unlock:
 	mutex_unlock(&xt[table->af].mutex);
-out_free:
 	kfree(table);
 out:
 	return ERR_PTR(ret);
-- 
1.7.10.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