On Tuesday 2010-04-06 16:12, Patrick McHardy wrote: >Jan Engelhardt wrote: >> +/* Defaults, these can be overridden on the module command-line. */ >> +static unsigned int condition_list_perms = S_IRUSR | S_IWUSR; >> +static unsigned int condition_uid_perms; >> +static unsigned int condition_gid_perms; > >I think it might be useful to make them overridable on a per-rule base >if it doesn't cause inconsistent behaviour when sharing a condition >variable. That does not work; a condition variable can only be owned by one uid. >> +/* proc_lock is a user context only semaphore used for write access */ >> +/* to the conditions' list. */ >> +static struct mutex proc_lock; >DEFINE_MUTEX() ? >> + rcu_read_lock(); >> + x = rcu_dereference(var->enabled); >> + rcu_read_unlock(); >That looks unnecessary. What exactly is that rcu lock trying to protect? >Where is the corresponding rcu assignment? >> + if (mutex_lock_interruptible(&proc_lock) != 0) >> + return -EINTR; >No need for interruptible locking, the section is very short and >usually there's only a single iptables process running at a time. > >> + var->status_proc->write_proc = condition_proc_write; >> + list_add_rcu(&var->list, &conditions_list); > >Why are you using the RCU variant? The list is only walked while >holding the mutex, without using the _rcu list functions. Removal >also happens while holding the mutex. removed the rcu things. >> + proc_net_condition = proc_mkdir(dir_name, net->proc_net); >> + if (proc_net_condition == NULL) >> + return -EACCES; >> + >> + ret = xt_register_match(&condition_mt_reg); > >Matches are registered globally, not once per namespace. fixed. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
