> -----Original Message----- > From: Pablo Neira Ayuso [mailto:pa...@netfilter.org] > Sent: Saturday, March 25, 2017 3:02 AM > To: f...@ikuai8.com > Cc: da...@davemloft.net; jo...@redhat.com; netfilter-de...@vger.kernel.org; > netdev@vger.kernel.org; gfree.w...@gmail.com > Subject: Re: [PATCH nf v3 1/1] netfilter: snmp: Fix one possible panic when > snmp_trap_helper fail to register > > On Tue, Mar 21, 2017 at 08:22:29AM +0800, f...@ikuai8.com wrote: > > From: Gao Feng <f...@ikuai8.com> > > > > In the commit 93557f53e1fb ("netfilter: nf_conntrack: nf_conntrack > > snmp helper"), the snmp_helper is replaced by nf_nat_snmp_hook. So the > > snmp_helper is never registered. But it still tries to unregister the > > snmp_helper, it could cause the panic. > > > > Now remove the useless snmp_helper and the unregister call in the > > error handler. > > > > Fixes: 93557f53e1fb ("netfilter: nf_conntrack: nf_conntrack snmp > > helper") > > > > Signed-off-by: Gao Feng <f...@ikuai8.com> > > --- > > v3: Remove the angle brackets in description, per Sergei > > v2: Add the SHA1 ID in the description, per Sergei > > v1: Initial version > > > > net/ipv4/netfilter/nf_nat_snmp_basic.c | 14 +------------- > > 1 file changed, 1 insertion(+), 13 deletions(-) > > > > diff --git a/net/ipv4/netfilter/nf_nat_snmp_basic.c > > b/net/ipv4/netfilter/nf_nat_snmp_basic.c > > index c9b52c3..5787364 100644 > > --- a/net/ipv4/netfilter/nf_nat_snmp_basic.c > > +++ b/net/ipv4/netfilter/nf_nat_snmp_basic.c > > @@ -1260,16 +1260,6 @@ static int help(struct sk_buff *skb, unsigned int > protoff, > > .timeout = 180, > > }; > > > > -static struct nf_conntrack_helper snmp_helper __read_mostly = { > > - .me = THIS_MODULE, > > - .help = help, > > - .expect_policy = &snmp_exp_policy, > > - .name = "snmp", > > - .tuple.src.l3num = AF_INET, > > - .tuple.src.u.udp.port = cpu_to_be16(SNMP_PORT), > > - .tuple.dst.protonum = IPPROTO_UDP, > > -}; > > - > > static struct nf_conntrack_helper snmp_trap_helper __read_mostly = { > > .me = THIS_MODULE, > > .help = help, > > @@ -1294,10 +1284,8 @@ static int __init nf_nat_snmp_basic_init(void) > > RCU_INIT_POINTER(nf_nat_snmp_hook, help); > > > > ret = nf_conntrack_helper_register(&snmp_trap_helper); > > - if (ret < 0) { > > - nf_conntrack_helper_unregister(&snmp_helper); > > + if (ret < 0) > > return ret; > > - } > > return ret; > > You can provide a more simplified version after this is removed, I > think: Ok. I thought it should not change the style for bug fix. I would sent the update patch.
Best Regards Feng > > @@ -1283,10 +1283,7 @@ static int __init nf_nat_snmp_basic_init(void) > BUG_ON(nf_nat_snmp_hook != NULL); > RCU_INIT_POINTER(nf_nat_snmp_hook, help); > > - ret = nf_conntrack_helper_register(&snmp_trap_helper); > - if (ret < 0) > - return ret; > - return ret; > + return nf_conntrack_helper_register(&snmp_trap_helper); > }
