NETFILTER: replace list_for_each with list_for_each_entry

PATCH Superseded Li Zefan 2 participants 5 messages Nov 14, 2007

Subsystems (2)

Networking netfilter

lore

Share & tools

lore

Li Zefan

author

2007-11-14 03:02 UTC #0 lore raw

PATCH 1/1 NETFILTER: replace list_for_each with list_for_each_entry

Replace list_for_each with list_for_each_entry when possible.

Signed-off-by Li Zefan <[email protected]>

---
 net/ipv4/netfilter/ipt_CLUSTERIP.c |    6 ++----
 net/netfilter/nf_sockopt.c         |    8 ++------
 2 files changed, 4 insertions(+), 10 deletions(-)

net/ipv4/netfilter/ipt_CLUSTERIP.c +2 -4

index 2f544da..311361e 100644
@@ -109,11 +109,9 @@ clusterip_config_entry_put(struct clusterip_config *c)
 static struct clusterip_config *
 __clusterip_config_find(__be32 clusterip)
 {
-	struct list_head *pos;
+	struct clusterip_config *c;
 
-	list_for_each(pos, &clusterip_configs) {
-		struct clusterip_config *c = list_entry(pos,
-					struct clusterip_config, list);
+	list_for_each_entry(c, &clusterip_configs, list) {
 		if (c->clusterip == clusterip)
 			return c;
 	}

net/netfilter/nf_sockopt.c +2 -6

index aa28315..06bbccb 100644
@@ -65,7 +65,6 @@ EXPORT_SYMBOL(nf_unregister_sockopt);
 static int nf_sockopt(struct sock *sk, int pf, int val,
 		      char __user *opt, int *len, int get)
 {
-	struct list_head *i;
 	struct nf_sockopt_ops *ops;
 	int ret;

@@ -75,8 +74,7 @@ static int nf_sockopt(struct sock *sk, int pf, int val,
 	if (mutex_lock_interruptible(&nf_sockopt_mutex) != 0)
 		return -EINTR;
 
-	list_for_each(i, &nf_sockopts) {
-		ops = (struct nf_sockopt_ops *)i;
+	list_for_each_entry(ops, &nf_sockopts, list) {
 		if (ops->pf == pf) {
 			if (!try_module_get(ops->owner))
 				goto out_nosup;

@@ -124,7 +122,6 @@ EXPORT_SYMBOL(nf_getsockopt);
 static int compat_nf_sockopt(struct sock *sk, int pf, int val,
 			     char __user *opt, int *len, int get)
 {
-	struct list_head *i;
 	struct nf_sockopt_ops *ops;
 	int ret;

@@ -135,8 +132,7 @@ static int compat_nf_sockopt(struct sock *sk, int pf, int val,
 	if (mutex_lock_interruptible(&nf_sockopt_mutex) != 0)
 		return -EINTR;
 
-	list_for_each(i, &nf_sockopts) {
-		ops = (struct nf_sockopt_ops *)i;
+	list_for_each_entry(ops, &nf_sockopts, list) {
 		if (ops->pf == pf) {
 			if (!try_module_get(ops->owner))
 				goto out_nosup;

Signature · 1 line

1.5.3.rc7

Li Zefan

author

2007-11-14 06:50 UTC #1 lore raw

Quoted 9 lines · Li Zefan

Replace list_for_each with list_for_each_entry when possible.
Signed-off-by Li Zefan <[email protected]>
---
 net/ipv4/netfilter/ipt_CLUSTERIP.c |    6 ++----
 net/netfilter/nf_sockopt.c         |    8 ++------
 2 files changed, 4 insertions(+), 10 deletions(-)

Please ignore this patch, I'll send a new one.

Li Zefan

author

2007-11-14 07:21 UTC #2 lore raw

PATCH v2 1/1 NETFILTER: replace list_for_each with list_for_each_entry

This patch replaces list_for_each with list_for_each_entry, and consequently fixes a bug too.

Things like this is wrong:

	list_for_each(i, &nf_sockopts) {
		struct nf_sockopt_ops *ops = (struct nf_sockopt_ops *)i;

It makes the assumption that list is the first member of struct nf_sockopt_ops, so the ptr casting works. But if one day the structure changes, for example, like this:

	struct nf_sockopt_ops {
		foo_t a_new_member;
		struct list_head list;
	};

Then we get corrupted data.

Signed-off-by Li Zefan <[email protected]>

---
 net/ipv4/netfilter/ipt_CLUSTERIP.c |    6 ++----
 net/netfilter/core.c               |    8 ++++----
 net/netfilter/nf_sockopt.c         |   13 ++++---------
 3 files changed, 10 insertions(+), 17 deletions(-)

net/ipv4/netfilter/ipt_CLUSTERIP.c +2 -4

index 2f544da..311361e 100644
@@ -109,11 +109,9 @@ clusterip_config_entry_put(struct clusterip_config *c)
 static struct clusterip_config *
 __clusterip_config_find(__be32 clusterip)
 {
-	struct list_head *pos;
+	struct clusterip_config *c;
 
-	list_for_each(pos, &clusterip_configs) {
-		struct clusterip_config *c = list_entry(pos,
-					struct clusterip_config, list);
+	list_for_each_entry(c, &clusterip_configs, list) {
 		if (c->clusterip == clusterip)
 			return c;
 	}

net/netfilter/core.c +4 -4

index bed9ba0..ed58922 100644
@@ -62,17 +62,17 @@ static DEFINE_MUTEX(nf_hook_mutex);
 
 int nf_register_hook(struct nf_hook_ops *reg)
 {
-	struct list_head *i;
+	struct nf_hook_ops *elem;
 	int err;
 
 	err = mutex_lock_interruptible(&nf_hook_mutex);
 	if (err < 0)
 		return err;
-	list_for_each(i, &nf_hooks[reg->pf][reg->hooknum]) {
-		if (reg->priority < ((struct nf_hook_ops *)i)->priority)
+	list_for_each_entry(elem, &nf_hooks[reg->pf][reg->hooknum], list) {
+		if (reg->priority < elem->priority)
 			break;
 	}
-	list_add_rcu(&reg->list, i->prev);
+	list_add_rcu(&reg->list, elem->list.prev);
 	mutex_unlock(&nf_hook_mutex);
 	return 0;
 }

net/netfilter/nf_sockopt.c +4 -9

index aa28315..2dfac32 100644
@@ -23,14 +23,13 @@ static inline int overlap(int min1, int max1, int min2, int max2)
 /* Functions to register sockopt ranges (exclusive). */
 int nf_register_sockopt(struct nf_sockopt_ops *reg)
 {
-	struct list_head *i;
+	struct nf_sockopt_ops *ops;
 	int ret = 0;
 
 	if (mutex_lock_interruptible(&nf_sockopt_mutex) != 0)
 		return -EINTR;
 
-	list_for_each(i, &nf_sockopts) {
-		struct nf_sockopt_ops *ops = (struct nf_sockopt_ops *)i;
+	list_for_each_entry(ops, &nf_sockopts, list) {
 		if (ops->pf == reg->pf
 		    && (overlap(ops->set_optmin, ops->set_optmax,
 				reg->set_optmin, reg->set_optmax)

@@ -65,7 +64,6 @@ EXPORT_SYMBOL(nf_unregister_sockopt);
 static int nf_sockopt(struct sock *sk, int pf, int val,
 		      char __user *opt, int *len, int get)
 {
-	struct list_head *i;
 	struct nf_sockopt_ops *ops;
 	int ret;

@@ -75,8 +73,7 @@ static int nf_sockopt(struct sock *sk, int pf, int val,
 	if (mutex_lock_interruptible(&nf_sockopt_mutex) != 0)
 		return -EINTR;
 
-	list_for_each(i, &nf_sockopts) {
-		ops = (struct nf_sockopt_ops *)i;
+	list_for_each_entry(ops, &nf_sockopts, list) {
 		if (ops->pf == pf) {
 			if (!try_module_get(ops->owner))
 				goto out_nosup;

@@ -124,7 +121,6 @@ EXPORT_SYMBOL(nf_getsockopt);
 static int compat_nf_sockopt(struct sock *sk, int pf, int val,
 			     char __user *opt, int *len, int get)
 {
-	struct list_head *i;
 	struct nf_sockopt_ops *ops;
 	int ret;

@@ -135,8 +131,7 @@ static int compat_nf_sockopt(struct sock *sk, int pf, int val,
 	if (mutex_lock_interruptible(&nf_sockopt_mutex) != 0)
 		return -EINTR;
 
-	list_for_each(i, &nf_sockopts) {
-		ops = (struct nf_sockopt_ops *)i;
+	list_for_each_entry(ops, &nf_sockopts, list) {
 		if (ops->pf == pf) {
 			if (!try_module_get(ops->owner))
 				goto out_nosup;

Signature · 1 line

1.5.3.rc7

Patrick McHardy

2007-11-14 08:37 UTC #3 lore raw

Quoted 11 lines · Li Zefan

This patch replaces list_for_each with list_for_each_entry, and consequently fixes a bug too.
Things like this is wrong:
	list_for_each(i, &nf_sockopts) {
		struct nf_sockopt_ops *ops = (struct nf_sockopt_ops *)i;
It makes the assumption that list is the first member of struct nf_sockopt_ops, so the ptr casting works. But if one day the structure changes, for example, like this:

So far its not a bug, so I've applied it without that description. Thanks.

Quoted 1 line

 net/netfilter/nf_sockopt.c         |   13 ++++---------

And removed that part since you're the third person within two weeks to send an identical patch :) Please always base your patches on Linus' latest -git tree or Dave's networking tree for the next version once it opens.

Li Zefan

author

2007-11-14 09:00 UTC #4 lore raw

Quoted 10 lines · Patrick McHardy

Quoted 1 line · Li Zefan
 net/netfilter/nf_sockopt.c         |   13 ++++---------
And removed that part since you're the third person within two weeks to send an identical patch :) Please always base your patches on Linus' latest -git tree or Dave's networking tree for the next version once it opens.

OK, I will. :)