Re: [PATCH 02/17] netfilter: add namespace support for l4proto

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, May 24, 2012 at 09:52:51AM +0800, Gao feng wrote:
> 于 2012年05月23日 18:25, Pablo Neira Ayuso 写道:
> > On Mon, May 14, 2012 at 04:52:12PM +0800, Gao feng wrote:
> >> From: Gao feng <gaofeng@cn.fujitus.com>
[...]
> >> @@ -243,137 +253,172 @@ void nf_conntrack_l3proto_unregister(struct nf_conntrack_l3proto *proto)
> >>  }
> >>  EXPORT_SYMBOL_GPL(nf_conntrack_l3proto_unregister);
> >>  
> >> -static int nf_ct_l4proto_register_sysctl(struct nf_conntrack_l4proto *l4proto)
> >> +static struct nf_proto_net *nf_ct_l4proto_net(struct net *net,
> >> +					      struct nf_conntrack_l4proto *l4proto)
> >>  {
> >> -	int err = 0;
> >> +	if (l4proto->net_id)
> >> +		return net_generic(net, *l4proto->net_id);
> >> +	else
> >> +		return NULL;
> >> +}
> >>  
> >> +int nf_ct_l4proto_register_sysctl(struct net *net,
> >> +				  struct nf_conntrack_l4proto *l4proto)
> >> +{
> >> +	int err = 0;
> >> +	struct nf_proto_net *pn = nf_ct_l4proto_net(net, l4proto);
> >> +	if (pn == NULL)
> >> +		return 0;
> >>  #ifdef CONFIG_SYSCTL
> >> -	if (l4proto->ctl_table != NULL) {
> >> -		err = nf_ct_register_sysctl(l4proto->ctl_table_header,
> >> +	if (pn->ctl_table != NULL) {
> >> +		err = nf_ct_register_sysctl(net,
> >> +					    &pn->ctl_table_header,
> >>  					    "net/netfilter",
> >> -					    l4proto->ctl_table,
> >> -					    l4proto->ctl_table_users);
> >> -		if (err < 0)
> >> +					    pn->ctl_table,
> >> +					    &pn->users);
> >> +		if (err < 0) {
> >> +			kfree(pn->ctl_table);
> >> +			pn->ctl_table = NULL;
> >                                ^^^^^^^^^^^
> > Do you really need to set this above to NULL? Is there any existing
> > bug trap? If not, it's superfluous, please, remove it.
> > 
> yes,l4proto_tcp(udp,icmp)'s ctl_table is stored in netns_ct.proto,
> so when we register l4proto_tcp's sysctl failed,ctl_table will still
> point to the kfreed memory. this will cause panic the next
> time we register l4proto_tcp's sysctl.

I see, thanks for the clarification.