Re: [PATCH 9/9] netfilter: conntrack: replace notify chain by function pointer

[Patrick McHardy <kaber@trash.net>]

Pablo Neira Ayuso wrote:
> -int nf_ct_expect_unregister_notifier(struct notifier_block *nb)
> +int nf_ct_expect_unregister_notifier(struct nf_exp_event_notifier *new)
>  {
> -	return atomic_notifier_chain_unregister(&nf_ct_expect_chain, nb);
> +	int ret = 0;
> +	struct nf_exp_event_notifier *notify;
> +
> +	mutex_lock(&nf_ct_ecache_mutex);
> +	notify = rcu_dereference(nf_expect_event_cb);
> +	if (notify != new) {
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}

I think these unregistration functions should return void. The only
reason why they don't currently is because the notifier_chain_unregister
function for some unknown reasons don't return void, but there's
a) nothing the caller could possibly do to handle this and b) a bug
anyways. So I'd suggest to just unconditionally assign NULL.

Sorry for not bringing this up earlier.

BTW, you might also consider marking the callback pointers read_mostly.

> +	rcu_assign_pointer(nf_expect_event_cb, NULL);
> +	mutex_unlock(&nf_ct_ecache_mutex);
> +	return ret;
> +
> +out_unlock:
> +	mutex_unlock(&nf_ct_ecache_mutex);
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(nf_ct_expect_unregister_notifier);