* re: module_param: make bool parameters really bool (net & drivers/net)
@ 2012-03-22 18:26 Dan Carpenter
2012-03-22 18:30 ` Dan Carpenter
2012-03-22 22:07 ` Rusty Russell
0 siblings, 2 replies; 5+ messages in thread
From: Dan Carpenter @ 2012-03-22 18:26 UTC (permalink / raw)
To: rusty; +Cc: netdev
Hi Rusty,
The patch eb93992207da: "module_param: make bool parameters really
bool (net & drivers/net)" from Dec 19, 2011, leads to the following
warning:
net/ipv4/netfilter/iptable_filter.c:90 iptable_filter_init()
warn: 5 is more than 1 (max 'forward' can be) so this is
always the same.
It's declared like this:
54 /* Default to forward because I got too much mail already. */
55 static bool forward = NF_ACCEPT;
56 module_param(forward, bool, 0000);
It's used like this:
66 ((struct ipt_standard *)repl->entries)[1].target.verdict = -forward - 1;
Smatch complains when check that it's larger than 5.
90 if (forward < 0 || forward > NF_MAX_VERDICT) {
91 pr_err("iptables forward must be 0 or 1\n");
92 return -EINVAL;
93 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: module_param: make bool parameters really bool (net & drivers/net)
2012-03-22 18:26 module_param: make bool parameters really bool (net & drivers/net) Dan Carpenter
@ 2012-03-22 18:30 ` Dan Carpenter
2012-03-22 22:07 ` Rusty Russell
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2012-03-22 18:30 UTC (permalink / raw)
To: rusty; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 97 bytes --]
The same thing happens in net/ipv6/netfilter/ip6table_filter.c
as well.
regards,
dan carpenter
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* re: module_param: make bool parameters really bool (net & drivers/net)
2012-03-22 18:26 module_param: make bool parameters really bool (net & drivers/net) Dan Carpenter
2012-03-22 18:30 ` Dan Carpenter
@ 2012-03-22 22:07 ` Rusty Russell
2012-03-22 22:27 ` [PATCH] netfilter: remove forward module param confusion Rusty Russell
1 sibling, 1 reply; 5+ messages in thread
From: Rusty Russell @ 2012-03-22 22:07 UTC (permalink / raw)
To: Dan Carpenter; +Cc: netdev, netfilter-devel
On Thu, 22 Mar 2012 21:26:23 +0300, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Hi Rusty,
>
> The patch eb93992207da: "module_param: make bool parameters really
> bool (net & drivers/net)" from Dec 19, 2011, leads to the following
> warning:
> net/ipv4/netfilter/iptable_filter.c:90 iptable_filter_init()
> warn: 5 is more than 1 (max 'forward' can be) so this is
> always the same.
>
> It's declared like this:
>
> 54 /* Default to forward because I got too much mail already. */
> 55 static bool forward = NF_ACCEPT;
> 56 module_param(forward, bool, 0000);
>
>
> It's used like this:
> 66 ((struct ipt_standard *)repl->entries)[1].target.verdict = -forward - 1;
>
> Smatch complains when check that it's larger than 5.
>
> 90 if (forward < 0 || forward > NF_MAX_VERDICT) {
> 91 pr_err("iptables forward must be 0 or 1\n");
> 92 return -EINVAL;
> 93 }
Thanks Dan!
This was obviously initially an arbitrary value, but someone
made it a bool module parameter (me?). It works for accept and drop, so
let's make it official. Patch coming.
Thanks,
Rusty.
--
How could I marry someone with more hair than me? http://baldalex.org
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] netfilter: remove forward module param confusion.
2012-03-22 22:07 ` Rusty Russell
@ 2012-03-22 22:27 ` Rusty Russell
2012-03-23 2:36 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Rusty Russell @ 2012-03-22 22:27 UTC (permalink / raw)
To: Dan Carpenter; +Cc: netdev, netfilter-devel
It used to be an int, and it got changed to a bool parameter at least
7 years ago. It happens that NF_ACCEPT and NF_DROP are 0 and 1, so
this works, but it's unclear, and the check that it's in range is not
required.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
net/ipv4/netfilter/iptable_filter.c | 9 ++-------
net/ipv6/netfilter/ip6table_filter.c | 9 ++-------
2 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/net/ipv4/netfilter/iptable_filter.c b/net/ipv4/netfilter/iptable_filter.c
--- a/net/ipv4/netfilter/iptable_filter.c
+++ b/net/ipv4/netfilter/iptable_filter.c
@@ -52,7 +52,7 @@ iptable_filter_hook(unsigned int hook, s
static struct nf_hook_ops *filter_ops __read_mostly;
/* Default to forward because I got too much mail already. */
-static bool forward = NF_ACCEPT;
+static bool forward = true;
module_param(forward, bool, 0000);
static int __net_init iptable_filter_net_init(struct net *net)
@@ -64,7 +64,7 @@ static int __net_init iptable_filter_net
return -ENOMEM;
/* Entry 1 is the FORWARD hook */
((struct ipt_standard *)repl->entries)[1].target.verdict =
- -forward - 1;
+ forward ? -NF_ACCEPT - 1 : -NF_DROP - 1;
net->ipv4.iptable_filter =
ipt_register_table(net, &packet_filter, repl);
@@ -88,11 +88,6 @@ static int __init iptable_filter_init(vo
{
int ret;
- if (forward < 0 || forward > NF_MAX_VERDICT) {
- pr_err("iptables forward must be 0 or 1\n");
- return -EINVAL;
- }
-
ret = register_pernet_subsys(&iptable_filter_net_ops);
if (ret < 0)
return ret;
diff --git a/net/ipv6/netfilter/ip6table_filter.c b/net/ipv6/netfilter/ip6table_filter.c
--- a/net/ipv6/netfilter/ip6table_filter.c
+++ b/net/ipv6/netfilter/ip6table_filter.c
@@ -44,7 +44,7 @@ ip6table_filter_hook(unsigned int hook,
static struct nf_hook_ops *filter_ops __read_mostly;
/* Default to forward because I got too much mail already. */
-static bool forward = NF_ACCEPT;
+static bool forward = true;
module_param(forward, bool, 0000);
static int __net_init ip6table_filter_net_init(struct net *net)
@@ -56,7 +56,7 @@ static int __net_init ip6table_filter_ne
return -ENOMEM;
/* Entry 1 is the FORWARD hook */
((struct ip6t_standard *)repl->entries)[1].target.verdict =
- -forward - 1;
+ forward ? -NF_ACCEPT - 1 : -NF_DROP - 1;
net->ipv6.ip6table_filter =
ip6t_register_table(net, &packet_filter, repl);
@@ -80,11 +80,6 @@ static int __init ip6table_filter_init(v
{
int ret;
- if (forward < 0 || forward > NF_MAX_VERDICT) {
- pr_err("iptables forward must be 0 or 1\n");
- return -EINVAL;
- }
-
ret = register_pernet_subsys(&ip6table_filter_net_ops);
if (ret < 0)
return ret;
--
How could I marry someone with more hair than me? http://baldalex.org
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] netfilter: remove forward module param confusion.
2012-03-22 22:27 ` [PATCH] netfilter: remove forward module param confusion Rusty Russell
@ 2012-03-23 2:36 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2012-03-23 2:36 UTC (permalink / raw)
To: rusty; +Cc: dan.carpenter, netdev, netfilter-devel
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Fri, 23 Mar 2012 08:57:06 +1030
> It used to be an int, and it got changed to a bool parameter at least
> 7 years ago. It happens that NF_ACCEPT and NF_DROP are 0 and 1, so
> this works, but it's unclear, and the check that it's in range is not
> required.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Applied.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-03-23 2:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-22 18:26 module_param: make bool parameters really bool (net & drivers/net) Dan Carpenter
2012-03-22 18:30 ` Dan Carpenter
2012-03-22 22:07 ` Rusty Russell
2012-03-22 22:27 ` [PATCH] netfilter: remove forward module param confusion Rusty Russell
2012-03-23 2:36 ` David Miller
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.