[PATCH net-next 1/2] ipv4: move local_port_range out of CONFIG_SYSCTL

[PATCH net-next 1/2] ipv4: move local_port_range out of CONFIG_SYSCTL

[Cong Wang]

From: Cong Wang <xiyou.wangcong@gmail.com>

When CONFIG_SYSCTL is not set, ip_local_port_range should still work,
just that no one can change it. Therefore we should move it out of sysctl_inet.c.
Also, rename it to ->ip_local_ports instead.

Cc: David S. Miller <davem@davemloft.net>
Cc: Francois Romieu <romieu@fr.zoreil.com>
Reported-by: Stefan de Konink <stefan@konink.de>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/netns/ipv4.h        |  2 +-
 net/ipv4/af_inet.c              | 28 ++++++++++++++++++++++++++++
 net/ipv4/inet_connection_sock.c |  8 ++++----
 net/ipv4/ping.c                 |  4 ++--
 net/ipv4/sysctl_net_ipv4.c      | 29 +++++++++++------------------
 5 files changed, 46 insertions(+), 25 deletions(-)

diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 80f500a..3d95cd4 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -66,7 +66,7 @@ struct netns_ipv4 {
 	int sysctl_icmp_ratemask;
 	int sysctl_icmp_errors_use_inbound_ifaddr;
 
-	struct local_ports sysctl_local_ports;
+	struct local_ports ip_local_ports;
 
 	int sysctl_tcp_ecn;
 	int sysctl_ip_no_pmtu_disc;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 8c54870..cccc8e4 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1650,6 +1650,31 @@ static int __init init_ipv4_mibs(void)
 	return register_pernet_subsys(&ipv4_mib_ops);
 }
 
+static __net_init int inet_init_net(struct net *net)
+{
+	/*
+	 * Set defaults for local port range
+	 */
+	seqlock_init(&net->ipv4.ip_local_ports.lock);
+	net->ipv4.ip_local_ports.range[0] =  32768;
+	net->ipv4.ip_local_ports.range[1] =  61000;
+	return 0;
+}
+
+static __net_exit void inet_exit_net(struct net *net)
+{
+}
+
+static __net_initdata struct pernet_operations af_inet_ops = {
+	.init = inet_init_net,
+	.exit = inet_exit_net,
+};
+
+static int __init init_inet_pernet_ops(void)
+{
+	return register_pernet_subsys(&af_inet_ops);
+}
+
 static int ipv4_proc_init(void);
 
 /*
@@ -1794,6 +1819,9 @@ static int __init inet_init(void)
 	if (ip_mr_init())
 		pr_crit("%s: Cannot init ipv4 mroute\n", __func__);
 #endif
+
+	if (init_inet_pernet_ops())
+		pr_crit("%s: Cannot init ipv4 inet pernet ops\n", __func__);
 	/*
 	 *	Initialise per-cpu ipv4 mibs
 	 */
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 0d1e2cb..a56b8e6 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -37,11 +37,11 @@ void inet_get_local_port_range(struct net *net, int *low, int *high)
 	unsigned int seq;
 
 	do {
-		seq = read_seqbegin(&net->ipv4.sysctl_local_ports.lock);
+		seq = read_seqbegin(&net->ipv4.ip_local_ports.lock);
 
-		*low = net->ipv4.sysctl_local_ports.range[0];
-		*high = net->ipv4.sysctl_local_ports.range[1];
-	} while (read_seqretry(&net->ipv4.sysctl_local_ports.lock, seq));
+		*low = net->ipv4.ip_local_ports.range[0];
+		*high = net->ipv4.ip_local_ports.range[1];
+	} while (read_seqretry(&net->ipv4.ip_local_ports.lock, seq));
 }
 EXPORT_SYMBOL(inet_get_local_port_range);
 
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 8210964..347bdde 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -240,11 +240,11 @@ static void inet_get_ping_group_range_net(struct net *net, kgid_t *low,
 	unsigned int seq;
 
 	do {
-		seq = read_seqbegin(&net->ipv4.sysctl_local_ports.lock);
+		seq = read_seqbegin(&net->ipv4.ip_local_ports.lock);
 
 		*low = data[0];
 		*high = data[1];
-	} while (read_seqretry(&net->ipv4.sysctl_local_ports.lock, seq));
+	} while (read_seqretry(&net->ipv4.ip_local_ports.lock, seq));
 }
 
 
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 44eba05..a116c41 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -45,10 +45,10 @@ static int ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };
 /* Update system visible IP port range */
 static void set_local_port_range(struct net *net, int range[2])
 {
-	write_seqlock(&net->ipv4.sysctl_local_ports.lock);
-	net->ipv4.sysctl_local_ports.range[0] = range[0];
-	net->ipv4.sysctl_local_ports.range[1] = range[1];
-	write_sequnlock(&net->ipv4.sysctl_local_ports.lock);
+	write_seqlock(&net->ipv4.ip_local_ports.lock);
+	net->ipv4.ip_local_ports.range[0] = range[0];
+	net->ipv4.ip_local_ports.range[1] = range[1];
+	write_sequnlock(&net->ipv4.ip_local_ports.lock);
 }
 
 /* Validate changes from /proc interface. */
@@ -57,7 +57,7 @@ static int ipv4_local_port_range(struct ctl_table *table, int write,
 				 size_t *lenp, loff_t *ppos)
 {
 	struct net *net =
-		container_of(table->data, struct net, ipv4.sysctl_local_ports.range);
+		container_of(table->data, struct net, ipv4.ip_local_ports.range);
 	int ret;
 	int range[2];
 	struct ctl_table tmp = {
@@ -90,11 +90,11 @@ static void inet_get_ping_group_range_table(struct ctl_table *table, kgid_t *low
 		container_of(table->data, struct net, ipv4.sysctl_ping_group_range);
 	unsigned int seq;
 	do {
-		seq = read_seqbegin(&net->ipv4.sysctl_local_ports.lock);
+		seq = read_seqbegin(&net->ipv4.ip_local_ports.lock);
 
 		*low = data[0];
 		*high = data[1];
-	} while (read_seqretry(&net->ipv4.sysctl_local_ports.lock, seq));
+	} while (read_seqretry(&net->ipv4.ip_local_ports.lock, seq));
 }
 
 /* Update system visible IP port range */
@@ -103,10 +103,10 @@ static void set_ping_group_range(struct ctl_table *table, kgid_t low, kgid_t hig
 	kgid_t *data = table->data;
 	struct net *net =
 		container_of(table->data, struct net, ipv4.sysctl_ping_group_range);
-	write_seqlock(&net->ipv4.sysctl_local_ports.lock);
+	write_seqlock(&net->ipv4.ip_local_ports.lock);
 	data[0] = low;
 	data[1] = high;
-	write_sequnlock(&net->ipv4.sysctl_local_ports.lock);
+	write_sequnlock(&net->ipv4.ip_local_ports.lock);
 }
 
 /* Validate changes from /proc interface. */
@@ -819,8 +819,8 @@ static struct ctl_table ipv4_net_table[] = {
 	},
 	{
 		.procname	= "ip_local_port_range",
-		.maxlen		= sizeof(init_net.ipv4.sysctl_local_ports.range),
-		.data		= &init_net.ipv4.sysctl_local_ports.range,
+		.maxlen		= sizeof(init_net.ipv4.ip_local_ports.range),
+		.data		= &init_net.ipv4.ip_local_ports.range,
 		.mode		= 0644,
 		.proc_handler	= ipv4_local_port_range,
 	},
@@ -865,13 +865,6 @@ static __net_init int ipv4_sysctl_init_net(struct net *net)
 	net->ipv4.sysctl_ping_group_range[0] = make_kgid(&init_user_ns, 1);
 	net->ipv4.sysctl_ping_group_range[1] = make_kgid(&init_user_ns, 0);
 
-	/*
-	 * Set defaults for local port range
-	 */
-	seqlock_init(&net->ipv4.sysctl_local_ports.lock);
-	net->ipv4.sysctl_local_ports.range[0] =  32768;
-	net->ipv4.sysctl_local_ports.range[1] =  61000;
-
 	net->ipv4.ipv4_hdr = register_net_sysctl(net, "net/ipv4", table);
 	if (net->ipv4.ipv4_hdr == NULL)
 		goto err_reg;
-- 
1.8.3.1

[PATCH net-next 2/2] ping: move ping_group_range out of CONFIG_SYSCTL

[Cong Wang]

From: Cong Wang <xiyou.wangcong@gmail.com>

Similarly, when CONFIG_SYSCTL is not set, ping_group_range should still
work, just that no one can change it. Therefore we should move it out of
sysctl_net_ipv4.c. And, it should not share the same seqlock with
ip_local_port_range.

BTW, rename it to ->ping_group_range instead.

Cc: David S. Miller <davem@davemloft.net>
Cc: Francois Romieu <romieu@fr.zoreil.com>
Reported-by: Stefan de Konink <stefan@konink.de>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/netns/ipv4.h   |  7 ++++++-
 net/ipv4/af_inet.c         |  8 ++++++++
 net/ipv4/ping.c            |  6 +++---
 net/ipv4/sysctl_net_ipv4.c | 13 +++----------
 4 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 3d95cd4..b2704fd0 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -20,6 +20,11 @@ struct local_ports {
 	int		range[2];
 };
 
+struct ping_group_range {
+	seqlock_t	lock;
+	kgid_t		range[2];
+};
+
 struct netns_ipv4 {
 #ifdef CONFIG_SYSCTL
 	struct ctl_table_header	*forw_hdr;
@@ -72,7 +77,7 @@ struct netns_ipv4 {
 	int sysctl_ip_no_pmtu_disc;
 	int sysctl_ip_fwd_use_pmtu;
 
-	kgid_t sysctl_ping_group_range[2];
+	struct ping_group_range ping_group_range;
 
 	atomic_t dev_addr_genid;
 
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index cccc8e4..6d6dd34 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1658,6 +1658,14 @@ static __net_init int inet_init_net(struct net *net)
 	seqlock_init(&net->ipv4.ip_local_ports.lock);
 	net->ipv4.ip_local_ports.range[0] =  32768;
 	net->ipv4.ip_local_ports.range[1] =  61000;
+
+	seqlock_init(&net->ipv4.ping_group_range.lock);
+	/*
+	 * Sane defaults - nobody may create ping sockets.
+	 * Boot scripts should set this to distro-specific group.
+	 */
+	net->ipv4.ping_group_range.range[0] = make_kgid(&init_user_ns, 1);
+	net->ipv4.ping_group_range.range[1] = make_kgid(&init_user_ns, 0);
 	return 0;
 }
 
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 347bdde..044a0dd 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -236,15 +236,15 @@ exit:
 static void inet_get_ping_group_range_net(struct net *net, kgid_t *low,
 					  kgid_t *high)
 {
-	kgid_t *data = net->ipv4.sysctl_ping_group_range;
+	kgid_t *data = net->ipv4.ping_group_range.range;
 	unsigned int seq;
 
 	do {
-		seq = read_seqbegin(&net->ipv4.ip_local_ports.lock);
+		seq = read_seqbegin(&net->ipv4.ping_group_range.lock);
 
 		*low = data[0];
 		*high = data[1];
-	} while (read_seqretry(&net->ipv4.ip_local_ports.lock, seq));
+	} while (read_seqretry(&net->ipv4.ping_group_range.lock, seq));
 }
 
 
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index a116c41..5cde8f2 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -87,7 +87,7 @@ static void inet_get_ping_group_range_table(struct ctl_table *table, kgid_t *low
 {
 	kgid_t *data = table->data;
 	struct net *net =
-		container_of(table->data, struct net, ipv4.sysctl_ping_group_range);
+		container_of(table->data, struct net, ipv4.ping_group_range.range);
 	unsigned int seq;
 	do {
 		seq = read_seqbegin(&net->ipv4.ip_local_ports.lock);
@@ -102,7 +102,7 @@ static void set_ping_group_range(struct ctl_table *table, kgid_t low, kgid_t hig
 {
 	kgid_t *data = table->data;
 	struct net *net =
-		container_of(table->data, struct net, ipv4.sysctl_ping_group_range);
+		container_of(table->data, struct net, ipv4.ping_group_range.range);
 	write_seqlock(&net->ipv4.ip_local_ports.lock);
 	data[0] = low;
 	data[1] = high;
@@ -805,7 +805,7 @@ static struct ctl_table ipv4_net_table[] = {
 	},
 	{
 		.procname	= "ping_group_range",
-		.data		= &init_net.ipv4.sysctl_ping_group_range,
+		.data		= &init_net.ipv4.ping_group_range.range,
 		.maxlen		= sizeof(gid_t)*2,
 		.mode		= 0644,
 		.proc_handler	= ipv4_ping_group_range,
@@ -858,13 +858,6 @@ static __net_init int ipv4_sysctl_init_net(struct net *net)
 			table[i].data += (void *)net - (void *)&init_net;
 	}
 
-	/*
-	 * Sane defaults - nobody may create ping sockets.
-	 * Boot scripts should set this to distro-specific group.
-	 */
-	net->ipv4.sysctl_ping_group_range[0] = make_kgid(&init_user_ns, 1);
-	net->ipv4.sysctl_ping_group_range[1] = make_kgid(&init_user_ns, 0);
-
 	net->ipv4.ipv4_hdr = register_net_sysctl(net, "net/ipv4", table);
 	if (net->ipv4.ipv4_hdr == NULL)
 		goto err_reg;
-- 
1.8.3.1

Re: [PATCH net-next 1/2] ipv4: move local_port_range out of CONFIG_SYSCTL

[David Miller]

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Tue,  6 May 2014 11:02:49 -0700

> From: Cong Wang <xiyou.wangcong@gmail.com>
> 
> When CONFIG_SYSCTL is not set, ip_local_port_range should still work,
> just that no one can change it. Therefore we should move it out of sysctl_inet.c.
> Also, rename it to ->ip_local_ports instead.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Francois Romieu <romieu@fr.zoreil.com>
> Reported-by: Stefan de Konink <stefan@konink.de>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

I think these two changes fix fairly serious issues that have existed for some
time, so it should go into 'net', do you agree?

Re: [PATCH net-next 1/2] ipv4: move local_port_range out of CONFIG_SYSCTL

[Francois Romieu]

David Miller <davem@davemloft.net> :
[...]
> I think these two changes fix fairly serious issues that have existed for some
> time, so it should go into 'net', do you agree?

Patch #1, yes.

Without patch #2, the root group id is always granted ping init when
CONFIG_SYSCTL is not set. It's more cautious than serious.

-- 
Ueimor

Re: [PATCH net-next 1/2] ipv4: move local_port_range out of CONFIG_SYSCTL

[David Miller]

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Thu, 8 May 2014 00:48:48 +0200

> David Miller <davem@davemloft.net> :
> [...]
>> I think these two changes fix fairly serious issues that have existed for some
>> time, so it should go into 'net', do you agree?
> 
> Patch #1, yes.
> 
> Without patch #2, the root group id is always granted ping init when
> CONFIG_SYSCTL is not set. It's more cautious than serious.

Still, initializing such fundamental values to different defaults
based upon whether SYSCTL is enabled or not is pretty bad, so I've
decided to apply both patches to 'net'.

Thanks.

Re: [PATCH net-next 1/2] ipv4: move local_port_range out of CONFIG_SYSCTL

[David Miller]

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Tue,  6 May 2014 11:02:49 -0700

> From: Cong Wang <xiyou.wangcong@gmail.com>
> 
> When CONFIG_SYSCTL is not set, ip_local_port_range should still work,
> just that no one can change it. Therefore we should move it out of sysctl_inet.c.
> Also, rename it to ->ip_local_ports instead.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Francois Romieu <romieu@fr.zoreil.com>
> Reported-by: Stefan de Konink <stefan@konink.de>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied to 'net'.

Re: [PATCH net-next 2/2] ping: move ping_group_range out of CONFIG_SYSCTL

[David Miller]

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Tue,  6 May 2014 11:02:50 -0700

> From: Cong Wang <xiyou.wangcong@gmail.com>
> 
> Similarly, when CONFIG_SYSCTL is not set, ping_group_range should still
> work, just that no one can change it. Therefore we should move it out of
> sysctl_net_ipv4.c. And, it should not share the same seqlock with
> ip_local_port_range.
> 
> BTW, rename it to ->ping_group_range instead.
> 
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Francois Romieu <romieu@fr.zoreil.com>
> Reported-by: Stefan de Konink <stefan@konink.de>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Also applied to 'net', thanks.

Re: [PATCH net-next 1/2] ipv4: move local_port_range out of CONFIG_SYSCTL

[Cong Wang]

On Thu, May 8, 2014 at 7:52 PM, David Miller <davem@davemloft.net> wrote:
> From: Francois Romieu <romieu@fr.zoreil.com>
> Date: Thu, 8 May 2014 00:48:48 +0200
>
>> David Miller <davem@davemloft.net> :
>> [...]
>>> I think these two changes fix fairly serious issues that have existed for some
>>> time, so it should go into 'net', do you agree?
>>
>> Patch #1, yes.
>>
>> Without patch #2, the root group id is always granted ping init when
>> CONFIG_SYSCTL is not set. It's more cautious than serious.
>
> Still, initializing such fundamental values to different defaults
> based upon whether SYSCTL is enabled or not is pretty bad, so I've
> decided to apply both patches to 'net'.
>

Makes sense for me. Thanks, everyone!