[PATCH] ipv6 addrconf: disallow rtr_solicits < -1

[PATCH] ipv6 addrconf: disallow rtr_solicits < -1

[Maciej Żenczykowski]

From: Maciej Żenczykowski <maze@google.com>

This disallows setting /proc/sys/net/ipv6/conf/*/router_solicitations
to values below -1.

-1 continues to mean an unlimited number of retransmits.

Note: this depends on 'ipv6 addrconf: remove addrconf_sysctl_hop_limit()'

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cbd9343751a2..511b76e6a50e 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5729,6 +5729,7 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
 	return ret;
 }
 
+static const int minus_one = -1;
 static const int one = 1;
 static const int two_five_five = 255;
 
@@ -5789,7 +5790,8 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.data		= &ipv6_devconf.rtr_solicits,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= (void *)&minus_one,
 	},
 	{
 		.procname	= "router_solicitation_interval",
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH] ipv6 addrconf: disallow rtr_solicits < -1

[Cong Wang]

On Sun, Oct 2, 2016 at 11:41 PM, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> +static const int minus_one = -1;
>  static const int one = 1;
>  static const int two_five_five = 255;
>
> @@ -5789,7 +5790,8 @@ static const struct ctl_table addrconf_sysctl[] = {
>                 .data           = &ipv6_devconf.rtr_solicits,
>                 .maxlen         = sizeof(int),
>                 .mode           = 0644,
> -               .proc_handler   = proc_dointvec,
> +               .proc_handler   = proc_dointvec_minmax,
> +               .extra1         = (void *)&minus_one,

The cast is not necessary.

Re: [PATCH] ipv6 addrconf: disallow rtr_solicits < -1

[Maciej Żenczykowski]

It is if you don't want build warnings about discarding 'const' qualifier.

Re: [PATCH] ipv6 addrconf: disallow rtr_solicits < -1

[Cong Wang]

On Mon, Oct 3, 2016 at 1:01 PM, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
> It is if you don't want build warnings about discarding 'const' qualifier.

Why do you need to mark it as const? We don't do this in sysctl_net_ipv4.c.

Re: [PATCH] ipv6 addrconf: disallow rtr_solicits < -1

[David Miller]

From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Sun,  2 Oct 2016 23:41:57 -0700

> From: Maciej Żenczykowski <maze@google.com>
> 
> This disallows setting /proc/sys/net/ipv6/conf/*/router_solicitations
> to values below -1.
> 
> -1 continues to mean an unlimited number of retransmits.
> 
> Note: this depends on 'ipv6 addrconf: remove addrconf_sysctl_hop_limit()'
> 
> Signed-off-by: Maciej Żenczykowski <maze@google.com>

Please remove the const qualifier and the casts to be consistent
with how we handle this elsewhere.

Thanks.

Re: [PATCH] ipv6 addrconf: disallow rtr_solicits < -1

[Maciej Żenczykowski]

> Please remove the const qualifier and the casts to be consistent
> with how we handle this elsewhere.
>
> Thanks.

I can of course trivially make that change.

But:

(on net-next/master)
git grep 'extra[12].*=.*\(void *[*]\)'

currently finds 45 matches, and this patch adds a 46th.

In particular there's a ton of such examples in:
  drivers/parport/procfs.c
  kernel/sysctl.c
  net/rxrpc/sysctl.c
  security/keys/sysctl.c

There's also the 2 that were added to:
  net/ipv6/addrconf.c (ie. same file as this patch)
in my previous patch set.

Should I also remove void * from those as well?

Should this be left as is?

Marking it explicitly as const gets the constants into the rodata
section which is cpu page table masked to be read-only so it seems
much safer/better...

- Maciej

Re: [PATCH] ipv6 addrconf: disallow rtr_solicits < -1

[Cong Wang]

On Mon, Oct 3, 2016 at 11:40 PM, Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
>> Please remove the const qualifier and the casts to be consistent
>> with how we handle this elsewhere.
>>
>> Thanks.
>
> I can of course trivially make that change.
>
> But:
>
> (on net-next/master)
> git grep 'extra[12].*=.*\(void *[*]\)'
>
> currently finds 45 matches, and this patch adds a 46th.

Seems the sysctl layer should make them const, but I never look
into it, it doesn't look like it needs to modify these min/max consts.

[PATCH v2] ipv6 addrconf: disallow rtr_solicits < -1

[Maciej Żenczykowski]

From: Maciej Żenczykowski <maze@google.com>

This disallows setting /proc/sys/net/ipv6/conf/*/router_solicitations
to values below -1.

-1 continues to mean an unlimited number of retransmits.

Note: this depends on 'ipv6 addrconf: remove addrconf_sysctl_hop_limit()'

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/ipv6/addrconf.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cbd9343751a2..d8983e15f859 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5729,6 +5729,7 @@ int addrconf_sysctl_ignore_routes_with_linkdown(struct ctl_table *ctl,
 	return ret;
 }
 
+static int minus_one = -1;
 static const int one = 1;
 static const int two_five_five = 255;
 
@@ -5789,7 +5790,8 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.data		= &ipv6_devconf.rtr_solicits,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &minus_one,
 	},
 	{
 		.procname	= "router_solicitation_interval",
-- 
2.8.0.rc3.226.g39d4020

Re: [PATCH v2] ipv6 addrconf: disallow rtr_solicits < -1

[David Miller]

From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Fri,  7 Oct 2016 01:00:49 -0700

> From: Maciej Żenczykowski <maze@google.com>
> 
> This disallows setting /proc/sys/net/ipv6/conf/*/router_solicitations
> to values below -1.
> 
> -1 continues to mean an unlimited number of retransmits.
> 
> Note: this depends on 'ipv6 addrconf: remove addrconf_sysctl_hop_limit()'
> 
> Signed-off-by: Maciej Żenczykowski <maze@google.com>

Applied, thanks.