All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic when snmp_trap_helper fail to register
@ 2017-03-25  2:24 gfree.wind
  2017-03-25  8:16 ` Liping Zhang
  2017-03-27 14:01 ` kbuild test robot
  0 siblings, 2 replies; 5+ messages in thread
From: gfree.wind @ 2017-03-25  2:24 UTC (permalink / raw)
  To: pablo, netfilter-devel, gfree.wind; +Cc: Gao Feng

From: Gao Feng <fgao@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 <fgao@ikuai8.com>
---
 v4: Simplify the codes, per Pablo
 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 | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/net/ipv4/netfilter/nf_nat_snmp_basic.c b/net/ipv4/netfilter/nf_nat_snmp_basic.c
index c9b52c3..36d16e5 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,
@@ -1293,12 +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) {
-		nf_conntrack_helper_unregister(&snmp_helper);
-		return ret;
-	}
-	return ret;
+	return nf_conntrack_helper_register(&snmp_trap_helper);
 }
 
 static void __exit nf_nat_snmp_basic_fini(void)
-- 
1.9.1





^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic when snmp_trap_helper fail to register
  2017-03-25  2:24 [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic when snmp_trap_helper fail to register gfree.wind
@ 2017-03-25  8:16 ` Liping Zhang
  2017-03-25 10:00   ` Gao Feng
  2017-03-27 14:01 ` kbuild test robot
  1 sibling, 1 reply; 5+ messages in thread
From: Liping Zhang @ 2017-03-25  8:16 UTC (permalink / raw)
  To: gfree.wind; +Cc: Pablo Neira Ayuso, Netfilter Developer Mailing List, Gao Feng

Hi Feng,

2017-03-25 10:24 GMT+08:00  <gfree.wind@foxmail.com>:
[...]
> @@ -1293,12 +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) {
> -               nf_conntrack_helper_unregister(&snmp_helper);
> -               return ret;
> -       }
> -       return ret;
> +       return nf_conntrack_helper_register(&snmp_trap_helper);

I think you should also remove the "int ret = 0;" statement,

static int __init nf_nat_snmp_basic_init(void)
{
     int ret = 0;  <--- Here

Otherwise, GCC will warn "unused variable".

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic when snmp_trap_helper fail to register
  2017-03-25  8:16 ` Liping Zhang
@ 2017-03-25 10:00   ` Gao Feng
  0 siblings, 0 replies; 5+ messages in thread
From: Gao Feng @ 2017-03-25 10:00 UTC (permalink / raw)
  To: 'Liping Zhang', gfree.wind
  Cc: 'Pablo Neira Ayuso', 'Netfilter Developer Mailing List'

Hi Liping,

> -----Original Message-----
> From: Liping Zhang [mailto:zlpnobody@gmail.com]
> Sent: Saturday, March 25, 2017 4:17 PM
> To: gfree.wind@foxmail.com
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>; Netfilter Developer Mailing List
> <netfilter-devel@vger.kernel.org>; Gao Feng <fgao@ikuai8.com>
> Subject: Re: [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic when
> snmp_trap_helper fail to register
> 
> Hi Feng,
> 
> 2017-03-25 10:24 GMT+08:00  <gfree.wind@foxmail.com>:
> [...]
> > @@ -1293,12 +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) {
> > -               nf_conntrack_helper_unregister(&snmp_helper);
> > -               return ret;
> > -       }
> > -       return ret;
> > +       return nf_conntrack_helper_register(&snmp_trap_helper);
> 
> I think you should also remove the "int ret = 0;" statement,
> 
> static int __init nf_nat_snmp_basic_init(void) {
>      int ret = 0;  <--- Here
> 
> Otherwise, GCC will warn "unused variable".

Thanks.
I didn't see the warning during compiling.
I think it should be overlapped because I pull the upstream.
There are other changes.

Next time, I would only touch the changed files and compile.
Thanks again.

Feng






^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic when snmp_trap_helper fail to register
  2017-03-25  2:24 [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic when snmp_trap_helper fail to register gfree.wind
  2017-03-25  8:16 ` Liping Zhang
@ 2017-03-27 14:01 ` kbuild test robot
  2017-03-27 14:08   ` Gao Feng
  1 sibling, 1 reply; 5+ messages in thread
From: kbuild test robot @ 2017-03-27 14:01 UTC (permalink / raw)
  To: gfree.wind; +Cc: kbuild-all, pablo, netfilter-devel, gfree.wind, Gao Feng

[-- Attachment #1: Type: text/plain, Size: 2942 bytes --]

Hi Gao,

[auto build test WARNING on nf/master]

url:    https://github.com/0day-ci/linux/commits/gfree-wind-foxmail-com/netfilter-snmp-Fix-one-possible-panic-when-snmp_trap_helper-fail-to-register/20170327-081220
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
config: x86_64-randconfig-s2-03272042 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   net/ipv4//netfilter/nf_nat_snmp_basic.c: In function 'nf_nat_snmp_basic_init':
>> net/ipv4//netfilter/nf_nat_snmp_basic.c:1281: warning: unused variable 'ret'

vim +/ret +1281 net/ipv4//netfilter/nf_nat_snmp_basic.c

807467c22 Patrick McHardy   2006-12-02  1265  	.help			= help,
6002f266b Patrick McHardy   2008-03-25  1266  	.expect_policy		= &snmp_exp_policy,
807467c22 Patrick McHardy   2006-12-02  1267  	.name			= "snmp_trap",
807467c22 Patrick McHardy   2006-12-02  1268  	.tuple.src.l3num	= AF_INET,
09640e636 Harvey Harrison   2009-02-01  1269  	.tuple.src.u.udp.port	= cpu_to_be16(SNMP_TRAP_PORT),
807467c22 Patrick McHardy   2006-12-02  1270  	.tuple.dst.protonum	= IPPROTO_UDP,
807467c22 Patrick McHardy   2006-12-02  1271  };
807467c22 Patrick McHardy   2006-12-02  1272  
807467c22 Patrick McHardy   2006-12-02  1273  /*****************************************************************************
807467c22 Patrick McHardy   2006-12-02  1274   *
807467c22 Patrick McHardy   2006-12-02  1275   * Module stuff.
807467c22 Patrick McHardy   2006-12-02  1276   *
807467c22 Patrick McHardy   2006-12-02  1277   *****************************************************************************/
807467c22 Patrick McHardy   2006-12-02  1278  
807467c22 Patrick McHardy   2006-12-02  1279  static int __init nf_nat_snmp_basic_init(void)
807467c22 Patrick McHardy   2006-12-02  1280  {
807467c22 Patrick McHardy   2006-12-02 @1281  	int ret = 0;
807467c22 Patrick McHardy   2006-12-02  1282  
93557f53e Jiri Olsa         2011-01-18  1283  	BUG_ON(nf_nat_snmp_hook != NULL);
a9b3cd7f3 Stephen Hemminger 2011-08-01  1284  	RCU_INIT_POINTER(nf_nat_snmp_hook, help);
93557f53e Jiri Olsa         2011-01-18  1285  
9b25a2987 Gao Feng          2017-03-25  1286  	return nf_conntrack_helper_register(&snmp_trap_helper);
807467c22 Patrick McHardy   2006-12-02  1287  }
807467c22 Patrick McHardy   2006-12-02  1288  
807467c22 Patrick McHardy   2006-12-02  1289  static void __exit nf_nat_snmp_basic_fini(void)

:::::: The code at line 1281 was first introduced by commit
:::::: 807467c22a12381985878be23c778657fc0364d0 [NETFILTER]: nf_nat: add SNMP NAT helper port

:::::: TO: Patrick McHardy <kaber@trash.net>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31288 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic when snmp_trap_helper fail to register
  2017-03-27 14:01 ` kbuild test robot
@ 2017-03-27 14:08   ` Gao Feng
  0 siblings, 0 replies; 5+ messages in thread
From: Gao Feng @ 2017-03-27 14:08 UTC (permalink / raw)
  To: 'kbuild test robot'
  Cc: kbuild-all, pablo, netfilter-devel, 'Gao Feng'


> -----Original Message-----
> From: netfilter-devel-owner@vger.kernel.org
> [mailto:netfilter-devel-owner@vger.kernel.org] On Behalf Of kbuild test
robot
> Sent: Monday, March 27, 2017 10:01 PM
> To: gfree.wind@foxmail.com
> Cc: kbuild-all@01.org; pablo@netfilter.org;
netfilter-devel@vger.kernel.org;
> gfree.wind@foxmail.com; Gao Feng <fgao@ikuai8.com>
> Subject: Re: [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic
when
> snmp_trap_helper fail to register
> 
> Hi Gao,
> 
> [auto build test WARNING on nf/master]
> 
> url:
>
https://github.com/0day-ci/linux/commits/gfree-wind-foxmail-com/netfilter-s
> nmp-Fix-one-possible-panic-when-snmp_trap_helper-fail-to-register/2017032
> 7-081220
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
master
> config: x86_64-randconfig-s2-03272042 (attached as .config)
> compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64
> 
> All warnings (new ones prefixed by >>):
> 
>    net/ipv4//netfilter/nf_nat_snmp_basic.c: In function
> 'nf_nat_snmp_basic_init':
> >> net/ipv4//netfilter/nf_nat_snmp_basic.c:1281: warning: unused variable
> 'ret'
> 
> vim +/ret +1281 net/ipv4//netfilter/nf_nat_snmp_basic.c
> 
> 807467c22 Patrick McHardy   2006-12-02  1265  	.help
=
> help,
> 6002f266b Patrick McHardy   2008-03-25  1266  	.expect_policy
=
> &snmp_exp_policy,
> 807467c22 Patrick McHardy   2006-12-02  1267  	.name
=
> "snmp_trap",
> 807467c22 Patrick McHardy   2006-12-02  1268  	.tuple.src.l3num
=
> AF_INET,
> 09640e636 Harvey Harrison   2009-02-01  1269
.tuple.src.u.udp.port	=
> cpu_to_be16(SNMP_TRAP_PORT),
> 807467c22 Patrick McHardy   2006-12-02  1270  	.tuple.dst.protonum
=
> IPPROTO_UDP,
> 807467c22 Patrick McHardy   2006-12-02  1271  };
> 807467c22 Patrick McHardy   2006-12-02  1272
> 807467c22 Patrick McHardy   2006-12-02  1273
> /***************************************************************
> **************
> 807467c22 Patrick McHardy   2006-12-02  1274   *
> 807467c22 Patrick McHardy   2006-12-02  1275   * Module stuff.
> 807467c22 Patrick McHardy   2006-12-02  1276   *
> 807467c22 Patrick McHardy   2006-12-02  1277
> ****************************************************************
> *************/
> 807467c22 Patrick McHardy   2006-12-02  1278
> 807467c22 Patrick McHardy   2006-12-02  1279  static int __init
> nf_nat_snmp_basic_init(void)
> 807467c22 Patrick McHardy   2006-12-02  1280  {
> 807467c22 Patrick McHardy   2006-12-02 @1281  	int ret = 0;
> 807467c22 Patrick McHardy   2006-12-02  1282
> 93557f53e Jiri Olsa         2011-01-18  1283
> 	BUG_ON(nf_nat_snmp_hook != NULL);
> a9b3cd7f3 Stephen Hemminger 2011-08-01  1284
> 	RCU_INIT_POINTER(nf_nat_snmp_hook, help);
> 93557f53e Jiri Olsa         2011-01-18  1285
> 9b25a2987 Gao Feng          2017-03-25  1286  	return
> nf_conntrack_helper_register(&snmp_trap_helper);
> 807467c22 Patrick McHardy   2006-12-02  1287  }
> 807467c22 Patrick McHardy   2006-12-02  1288
> 807467c22 Patrick McHardy   2006-12-02  1289  static void __exit
> nf_nat_snmp_basic_fini(void)
> 
> :::::: The code at line 1281 was first introduced by commit
> :::::: 807467c22a12381985878be23c778657fc0364d0 [NETFILTER]: nf_nat: add
> SNMP NAT helper port
> 
> :::::: TO: Patrick McHardy <kaber@trash.net>
> :::::: CC: David S. Miller <davem@davemloft.net>
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology
> Center
> https://lists.01.org/pipermail/kbuild-all                   Intel
> Corporation

Thanks.
It is fixed in v5 patch.

Regards
Feng




^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-03-27 14:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-25  2:24 [PATCH nf v4 1/1] netfilter: snmp: Fix one possible panic when snmp_trap_helper fail to register gfree.wind
2017-03-25  8:16 ` Liping Zhang
2017-03-25 10:00   ` Gao Feng
2017-03-27 14:01 ` kbuild test robot
2017-03-27 14:08   ` Gao Feng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.