diff options
author | Rusty Russell <rusty@rustcorp.com.au> | 2005-01-04 04:13:30 -0800 |
---|---|---|
committer | Linus Torvalds <torvalds@ppc970.osdl.org> | 2005-01-04 04:13:30 -0800 |
commit | a22405d433bbdc3131c5327e9b2d2fbcf4744705 (patch) | |
tree | bf5e2a7e4cb1aaa952a1e88b28aca455a2418860 /net | |
parent | 65a4385dda9524223a367660bbb829818bf48090 (diff) | |
download | history-a22405d433bbdc3131c5327e9b2d2fbcf4744705.tar.gz |
[PATCH] Clean up the kmod handling code in iptables.c
We now have a convenient kmod macro try_then_request_module().
1) Split our current "generic" function into separate
find_target_lock(), find_table_lock() and find_match_lock()
functions explicitly.
2) Have those functions do try_module_get() for us, and fix up the one
caller who didn't do that anyway.
3) Have the caller use try_then_request_module().
4) Remove __ipt_mutex_up() and __ipt_find_target_lock() which weren't
used (even in patch-o-matic AFAICT).
This cleanup takes us closer to using standard list macros everywhere,
and makes the version patch simpler.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Diffstat (limited to 'net')
-rw-r--r-- | net/ipv4/netfilter/ip_tables.c | 171 |
1 files changed, 67 insertions, 104 deletions
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index c77a55ebe54615..c0f6d4081e1bab 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c @@ -26,6 +26,7 @@ #include <asm/uaccess.h> #include <asm/semaphore.h> #include <linux/proc_fs.h> +#include <linux/err.h> #include <linux/netfilter_ipv4/ip_tables.h> @@ -408,85 +409,55 @@ ipt_do_table(struct sk_buff **pskb, #endif } -/* If it succeeds, returns element and locks mutex */ -static inline void * -find_inlist_lock_noload(struct list_head *head, - const char *name, - int *error, - struct semaphore *mutex) -{ - void *ret; - -#if 0 - duprintf("find_inlist: searching for `%s' in %s.\n", - name, head == &ipt_target ? "ipt_target" - : head == &ipt_match ? "ipt_match" - : head == &ipt_tables ? "ipt_tables" : "UNKNOWN"); -#endif - - *error = down_interruptible(mutex); - if (*error != 0) - return NULL; - - ret = list_named_find(head, name); - if (!ret) { - *error = -ENOENT; - up(mutex); - } - return ret; -} +/* + * These are weird, but module loading must not be done with mutex + * held (since they will register), and we have to have a single + * function to use try_then_request_module(). + */ -#ifndef CONFIG_KMOD -#define find_inlist_lock(h,n,p,e,m) find_inlist_lock_noload((h),(n),(e),(m)) -#else -static void * -find_inlist_lock(struct list_head *head, - const char *name, - const char *prefix, - int *error, - struct semaphore *mutex) +/* Find table by name, grabs mutex & ref. Returns ERR_PTR() on error. */ +static inline struct ipt_table *find_table_lock(const char *name) { - void *ret; + struct ipt_table *t; - ret = find_inlist_lock_noload(head, name, error, mutex); - if (!ret) { - duprintf("find_inlist: loading `%s%s'.\n", prefix, name); - request_module("%s%s", prefix, name); - ret = find_inlist_lock_noload(head, name, error, mutex); - } + if (down_interruptible(&ipt_mutex) != 0) + return ERR_PTR(-EINTR); - return ret; + list_for_each_entry(t, &ipt_tables, list) + if (strcmp(t->name, name) == 0 && try_module_get(t->me)) + return t; + up(&ipt_mutex); + return NULL; } -#endif -static inline struct ipt_table * -ipt_find_table_lock(const char *name, int *error, struct semaphore *mutex) +/* Find match by name, grabs mutex & ref. Returns ERR_PTR() on error. */ +static inline struct ipt_match *find_match_lock(const char *name) { - return find_inlist_lock(&ipt_tables, name, "iptable_", error, mutex); -} + struct ipt_match *m; -static inline struct ipt_match * -find_match_lock(const char *name, int *error, struct semaphore *mutex) -{ - return find_inlist_lock(&ipt_match, name, "ipt_", error, mutex); -} + if (down_interruptible(&ipt_mutex) != 0) + return ERR_PTR(-EINTR); -static struct ipt_target * -ipt_find_target_lock(const char *name, int *error, struct semaphore *mutex) -{ - return find_inlist_lock(&ipt_target, name, "ipt_", error, mutex); + list_for_each_entry(m, &ipt_match, list) + if (strcmp(m->name, name) == 0 && try_module_get(m->me)) + return m; + up(&ipt_mutex); + return NULL; } -struct ipt_target * -__ipt_find_target_lock(const char *name, int *error) +/* Find target by name, grabs mutex & ref. Returns ERR_PTR() on error. */ +static inline struct ipt_target *find_target_lock(const char *name) { - return ipt_find_target_lock(name,error,&ipt_mutex); -} + struct ipt_target *t; -void -__ipt_mutex_up(void) -{ + if (down_interruptible(&ipt_mutex) != 0) + return ERR_PTR(-EINTR); + + list_for_each_entry(t, &ipt_target, list) + if (strcmp(t->name, name) == 0 && try_module_get(t->me)) + return t; up(&ipt_mutex); + return NULL; } /* All zeroes == unconditional rule. */ @@ -648,17 +619,13 @@ check_match(struct ipt_entry_match *m, unsigned int hookmask, unsigned int *i) { - int ret; struct ipt_match *match; - match = find_match_lock(m->u.user.name, &ret, &ipt_mutex); - if (!match) { + match = try_then_request_module(find_match_lock(m->u.user.name), + "ipt_%s", m->u.user.name); + if (IS_ERR(match) || !match) { duprintf("check_match: `%s' not found\n", m->u.user.name); - return ret; - } - if (!try_module_get(match->me)) { - up(&ipt_mutex); - return -ENOENT; + return match ? PTR_ERR(match) : -ENOENT; } m->u.kernel.match = match; up(&ipt_mutex); @@ -699,14 +666,11 @@ check_entry(struct ipt_entry *e, const char *name, unsigned int size, goto cleanup_matches; t = ipt_get_target(e); - target = ipt_find_target_lock(t->u.user.name, &ret, &ipt_mutex); - if (!target) { + target = try_then_request_module(find_target_lock(t->u.user.name), + "ipt_%s", t->u.user.name); + if (IS_ERR(target) || !target) { duprintf("check_entry: `%s' not found\n", t->u.user.name); - goto cleanup_matches; - } - if (!try_module_get(target->me)) { - up(&ipt_mutex); - ret = -ENOENT; + ret = target ? PTR_ERR(target) : -ENOENT; goto cleanup_matches; } t->u.kernel.target = target; @@ -1036,8 +1000,8 @@ get_entries(const struct ipt_get_entries *entries, int ret; struct ipt_table *t; - t = ipt_find_table_lock(entries->name, &ret, &ipt_mutex); - if (t) { + t = find_table_lock(entries->name); + if (t && !IS_ERR(t)) { duprintf("t->private->number = %u\n", t->private->number); if (entries->size == t->private->size) @@ -1049,10 +1013,10 @@ get_entries(const struct ipt_get_entries *entries, entries->size); ret = -EINVAL; } + module_put(t->me); up(&ipt_mutex); } else - duprintf("get_entries: Can't find %s!\n", - entries->name); + ret = t ? PTR_ERR(t) : -ENOENT; return ret; } @@ -1103,25 +1067,21 @@ do_replace(void __user *user, unsigned int len) duprintf("ip_tables: Translated table\n"); - t = ipt_find_table_lock(tmp.name, &ret, &ipt_mutex); - if (!t) + t = try_then_request_module(find_table_lock(tmp.name), + "iptable_%s", tmp.name); + if (!t || IS_ERR(t)) { + ret = t ? PTR_ERR(t) : -ENOENT; goto free_newinfo_counters_untrans; + } /* You lied! */ if (tmp.valid_hooks != t->valid_hooks) { duprintf("Valid hook crap: %08X vs %08X\n", tmp.valid_hooks, t->valid_hooks); ret = -EINVAL; - goto free_newinfo_counters_untrans_unlock; - } - - /* Get a reference in advance, we're not allowed fail later */ - if (!try_module_get(t->me)) { - ret = -EBUSY; - goto free_newinfo_counters_untrans_unlock; + goto put_module; } - oldinfo = replace_table(t, tmp.num_counters, newinfo, &ret); if (!oldinfo) goto put_module; @@ -1150,7 +1110,6 @@ do_replace(void __user *user, unsigned int len) put_module: module_put(t->me); - free_newinfo_counters_untrans_unlock: up(&ipt_mutex); free_newinfo_counters_untrans: IPT_ENTRY_ITERATE(newinfo->entries, newinfo->size, cleanup_entry,NULL); @@ -1189,7 +1148,7 @@ do_add_counters(void __user *user, unsigned int len) unsigned int i; struct ipt_counters_info tmp, *paddc; struct ipt_table *t; - int ret; + int ret = 0; if (copy_from_user(&tmp, user, sizeof(tmp)) != 0) return -EFAULT; @@ -1206,9 +1165,11 @@ do_add_counters(void __user *user, unsigned int len) goto free; } - t = ipt_find_table_lock(tmp.name, &ret, &ipt_mutex); - if (!t) + t = find_table_lock(tmp.name); + if (!t || IS_ERR(t)) { + ret = t ? PTR_ERR(t) : -ENOENT; goto free; + } write_lock_bh(&t->lock); if (t->private->number != paddc->num_counters) { @@ -1225,6 +1186,7 @@ do_add_counters(void __user *user, unsigned int len) unlock_up_free: write_unlock_bh(&t->lock); up(&ipt_mutex); + module_put(t->me); free: vfree(paddc); @@ -1281,8 +1243,10 @@ do_ipt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len) break; } name[IPT_TABLE_MAXNAMELEN-1] = '\0'; - t = ipt_find_table_lock(name, &ret, &ipt_mutex); - if (t) { + + t = try_then_request_module(find_table_lock(name), + "iptable_%s", name); + if (t && !IS_ERR(t)) { struct ipt_getinfo info; info.valid_hooks = t->valid_hooks; @@ -1298,9 +1262,10 @@ do_ipt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len) ret = -EFAULT; else ret = 0; - up(&ipt_mutex); - } + module_put(t->me); + } else + ret = t ? PTR_ERR(t) : -ENOENT; } break; @@ -1889,8 +1854,6 @@ EXPORT_SYMBOL(ipt_unregister_match); EXPORT_SYMBOL(ipt_do_table); EXPORT_SYMBOL(ipt_register_target); EXPORT_SYMBOL(ipt_unregister_target); -EXPORT_SYMBOL_GPL(__ipt_find_target_lock); -EXPORT_SYMBOL_GPL(__ipt_mutex_up); module_init(init); module_exit(fini); |