On 2021-08-22, at 13:19:46 +0200, Jan Engelhardt wrote:
> On Friday 2021-08-20 20:24, Grzegorz Kuczyński wrote:
> > A few years ago I add network namespace to extension condition.
> > I review this changes again and make changes again.
> > This is better version.
>
> It does not apply. Your mail software mangled the patch.
>
> I would also wish for a more precise description what causes
> these lines to need removal.
>
> >- if (cnet->after_clear)
> >- return;
>
> >- if (--var->refcount == 0) {
> >+ if (--var->refcount == 0 && !list_empty_careful(&cnet->conditions_list)) {
I've been looking at this, and I don't think it works.
The idea is to do away with a separate flag whose purpose is to indicate
that condition_mt_destroy is being called during the tear-down of a
name-space.
Currently we have:
static void __net_exit condition_net_exit(struct net *net)
{
struct condition_net *condition_net = condition_pernet(net);
struct list_head *pos, *q;
struct condition_variable *var = NULL;
remove_proc_subtree(dir_name, net->proc_net);
mutex_lock(&proc_lock);
list_for_each_safe(pos, q, &condition_net->conditions_list) {
var = list_entry(pos, struct condition_variable, list);
list_del(pos);
kfree(var);
}
mutex_unlock(&proc_lock);
condition_net->after_clear = true;
}
and:
static void condition_mt_destroy(const struct xt_mtdtor_param *par)
{
const struct xt_condition_mtinfo *info = par->matchinfo;
struct condition_variable *var = info->condvar;
struct condition_net *cnet = condition_pernet(par->net);
if (cnet->after_clear)
return;
mutex_lock(&proc_lock);
if (--var->refcount == 0) {
list_del(&var->list);
remove_proc_entry(var->name, cnet->proc_net_condition);
mutex_unlock(&proc_lock);
kfree(var);
return;
}
mutex_unlock(&proc_lock);
}
Since pernet_operations destructors are called in the reverse of the
order in which they are registered, the destructor for xt_condition will
be called before the destructor for the table which deletes all the
rules. The `after_clear` flag serves to indicate during the call of
condition_mt_destroy that condition_net_exit has already been called and
freed all the variables. Replacing it with a check whether list of
variables is empty could work, but not after we've checked the ref-count
of a variable that we have already freed. Based on what I've seen in
other modules, I'd be more inclined to do something like this:
static void __net_exit condition_net_exit(struct net *net)
{
struct condition_net *condition_net = condition_pernet(net);
remove_proc_subtree(dir_name, net->proc_net);
condition_net->proc_net_condition = NULL;
}
and:
static void condition_mt_destroy(const struct xt_mtdtor_param *par)
{
const struct xt_condition_mtinfo *info = par->matchinfo;
struct condition_variable *var = info->condvar;
struct condition_net *cnet = condition_pernet(par->net);
mutex_lock(&proc_lock);
if (--var->refcount == 0) {
list_del(&var->list);
if (condition_net->proc_net_condition)
remove_proc_entry(var->name, cnet->proc_net_condition);
kfree(var);
}
mutex_unlock(&proc_lock);
}
J.
Attachment:
signature.asc
Description: PGP signature
