diff mbox

[3.16.y-ckt,stable] Patch "net/ipv6: add sysctl option accept_ra_min_hop_limit" has been added to the 3.16.y-ckt tree

Message ID 1458642286-32021-1-git-send-email-luis.henriques@canonical.com
State New
Headers show

Commit Message

Luis Henriques March 22, 2016, 10:24 a.m. UTC
This is a note to let you know that I have just added a patch titled

    net/ipv6: add sysctl option accept_ra_min_hop_limit

to the linux-3.16.y-queue branch of the 3.16.y-ckt extended stable tree 
which can be found at:

    http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-3.16.y-queue

This patch is scheduled to be released in version 3.16.7-ckt26.

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.16.y-ckt tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Luis

---8<------------------------------------------------------------

From dafdd3bb2296d4b70703c0bdb725f3265064ba4a Mon Sep 17 00:00:00 2001
From: Hangbin Liu <liuhangbin@gmail.com>
Date: Thu, 30 Jul 2015 14:28:42 +0800
Subject: net/ipv6: add sysctl option accept_ra_min_hop_limit

commit 8013d1d7eafb0589ca766db6b74026f76b7f5cb4 upstream.

Commit 6fd99094de2b ("ipv6: Don't reduce hop limit for an interface")
disabled accept hop limit from RA if it is smaller than the current hop
limit for security stuff. But this behavior kind of break the RFC definition.

RFC 4861, 6.3.4.  Processing Received Router Advertisements
   A Router Advertisement field (e.g., Cur Hop Limit, Reachable Time,
   and Retrans Timer) may contain a value denoting that it is
   unspecified.  In such cases, the parameter should be ignored and the
   host should continue using whatever value it is already using.

   If the received Cur Hop Limit value is non-zero, the host SHOULD set
   its CurHopLimit variable to the received value.

So add sysctl option accept_ra_min_hop_limit to let user choose the minimum
hop limit value they can accept from RA. And set default to 1 to meet RFC
standards.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Acked-by: YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
[ luis: backported to 3.16: adjusted context ]
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
 Documentation/networking/ip-sysctl.txt |  8 ++++++++
 include/linux/ipv6.h                   |  1 +
 include/uapi/linux/ipv6.h              |  1 +
 net/ipv6/addrconf.c                    | 10 ++++++++++
 net/ipv6/ndisc.c                       | 16 +++++++---------
 5 files changed, 27 insertions(+), 9 deletions(-)

Comments

Hideaki Yoshifuji March 22, 2016, 11:02 a.m. UTC | #1
Hi,

Luis Henriques wrote:
> This is a note to let you know that I have just added a patch titled
> 
>     net/ipv6: add sysctl option accept_ra_min_hop_limit
> 
> to the linux-3.16.y-queue branch of the 3.16.y-ckt extended stable tree 
> which can be found at:
> 
>     http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-3.16.y-queue
> 
> This patch is scheduled to be released in version 3.16.7-ckt26.
> 
> If you, or anyone else, feels it should not be added to this tree, please 
> reply to this email.
> 
> For more information about the 3.16.y-ckt tree, see
> https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable
> 
> Thanks.
> -Luis
> 
> ---8<------------------------------------------------------------
> 
> From dafdd3bb2296d4b70703c0bdb725f3265064ba4a Mon Sep 17 00:00:00 2001
> From: Hangbin Liu <liuhangbin@gmail.com>
> Date: Thu, 30 Jul 2015 14:28:42 +0800
> Subject: net/ipv6: add sysctl option accept_ra_min_hop_limit
> 
> commit 8013d1d7eafb0589ca766db6b74026f76b7f5cb4 upstream.
> 
> Commit 6fd99094de2b ("ipv6: Don't reduce hop limit for an interface")
> disabled accept hop limit from RA if it is smaller than the current hop
> limit for security stuff. But this behavior kind of break the RFC definition.
> 
> RFC 4861, 6.3.4.  Processing Received Router Advertisements
>    A Router Advertisement field (e.g., Cur Hop Limit, Reachable Time,
>    and Retrans Timer) may contain a value denoting that it is
>    unspecified.  In such cases, the parameter should be ignored and the
>    host should continue using whatever value it is already using.
> 
>    If the received Cur Hop Limit value is non-zero, the host SHOULD set
>    its CurHopLimit variable to the received value.
> 
> So add sysctl option accept_ra_min_hop_limit to let user choose the minimum
> hop limit value they can accept from RA. And set default to 1 to meet RFC
> standards.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> Acked-by: YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> [ luis: backported to 3.16: adjusted context ]
> Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
> ---
>  Documentation/networking/ip-sysctl.txt |  8 ++++++++
>  include/linux/ipv6.h                   |  1 +
>  include/uapi/linux/ipv6.h              |  1 +
>  net/ipv6/addrconf.c                    | 10 ++++++++++
>  net/ipv6/ndisc.c                       | 16 +++++++---------
>  5 files changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> index ab42c95f9985..36aa39eee48f 100644
> --- a/Documentation/networking/ip-sysctl.txt
> +++ b/Documentation/networking/ip-sysctl.txt
> @@ -1210,6 +1210,14 @@ accept_ra_defrtr - BOOLEAN
>  	Functional default: enabled if accept_ra is enabled.
>  			    disabled if accept_ra is disabled.
> 
> +accept_ra_min_hop_limit - INTEGER
> +	Minimum hop limit Information in Router Advertisement.
> +
> +	Hop limit Information in Router Advertisement less than this
> +	variable shall be ignored.
> +
> +	Default: 1
> +
>  accept_ra_pinfo - BOOLEAN
>  	Learn Prefix Information in Router Advertisement.
> 
> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> index f3d5d11b8871..9cc2240d7e52 100644
> --- a/include/linux/ipv6.h
> +++ b/include/linux/ipv6.h
> @@ -29,6 +29,7 @@ struct ipv6_devconf {
>  	__s32		max_desync_factor;
>  	__s32		max_addresses;
>  	__s32		accept_ra_defrtr;
> +	__s32		accept_ra_min_hop_limit;
>  	__s32		accept_ra_pinfo;
>  #ifdef CONFIG_IPV6_ROUTER_PREF
>  	__s32		accept_ra_rtr_pref;
> diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
> index 593b0e32d956..f1d321427905 100644
> --- a/include/uapi/linux/ipv6.h
> +++ b/include/uapi/linux/ipv6.h
> @@ -163,6 +163,7 @@ enum {
>  	DEVCONF_MLDV1_UNSOLICITED_REPORT_INTERVAL,
>  	DEVCONF_MLDV2_UNSOLICITED_REPORT_INTERVAL,
>  	DEVCONF_SUPPRESS_FRAG_NDISC,

Please add followings:
        __DEVCONF_ACCEPT_RA_FROM_LOCAL,
        __DEVCONF_USE_OPTIMISTIC,
        __DEVCONF_ACCEPT_RA_MTU,
        __DEVCONF_STABLE_SECRET,
        __DEVCONF_USE_OIF_ADDRS_ONLY,

Otherwise 

> +	DEVCONF_ACCEPT_RA_MIN_HOP_LIMIT,

has different value than the value we have in mainline.

--yoshfuji

>  	DEVCONF_MAX
>  };
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 75630e173adf..405575f9ae77 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -186,6 +186,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
>  	.max_desync_factor	= MAX_DESYNC_FACTOR,
>  	.max_addresses		= IPV6_MAX_ADDRESSES,
>  	.accept_ra_defrtr	= 1,
> +	.accept_ra_min_hop_limit= 1,
>  	.accept_ra_pinfo	= 1,
>  #ifdef CONFIG_IPV6_ROUTER_PREF
>  	.accept_ra_rtr_pref	= 1,
> @@ -222,6 +223,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
>  	.max_desync_factor	= MAX_DESYNC_FACTOR,
>  	.max_addresses		= IPV6_MAX_ADDRESSES,
>  	.accept_ra_defrtr	= 1,
> +	.accept_ra_min_hop_limit= 1,
>  	.accept_ra_pinfo	= 1,
>  #ifdef CONFIG_IPV6_ROUTER_PREF
>  	.accept_ra_rtr_pref	= 1,
> @@ -4294,6 +4296,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
>  	array[DEVCONF_MAX_DESYNC_FACTOR] = cnf->max_desync_factor;
>  	array[DEVCONF_MAX_ADDRESSES] = cnf->max_addresses;
>  	array[DEVCONF_ACCEPT_RA_DEFRTR] = cnf->accept_ra_defrtr;
> +	array[DEVCONF_ACCEPT_RA_MIN_HOP_LIMIT] = cnf->accept_ra_min_hop_limit;
>  	array[DEVCONF_ACCEPT_RA_PINFO] = cnf->accept_ra_pinfo;
>  #ifdef CONFIG_IPV6_ROUTER_PREF
>  	array[DEVCONF_ACCEPT_RA_RTR_PREF] = cnf->accept_ra_rtr_pref;
> @@ -5079,6 +5082,13 @@ static struct addrconf_sysctl_table
>  			.proc_handler	= proc_dointvec,
>  		},
>  		{
> +			.procname	= "accept_ra_min_hop_limit",
> +			.data		= &ipv6_devconf.accept_ra_min_hop_limit,
> +			.maxlen		= sizeof(int),
> +			.mode		= 0644,
> +			.proc_handler	= proc_dointvec,
> +		},
> +		{
>  			.procname	= "accept_ra_pinfo",
>  			.data		= &ipv6_devconf.accept_ra_pinfo,
>  			.maxlen		= sizeof(int),
> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index e2eb53c719dc..577338204863 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -1189,18 +1189,16 @@ static void ndisc_router_discovery(struct sk_buff *skb)
> 
>  	if (rt)
>  		rt6_set_expires(rt, jiffies + (HZ * lifetime));
> -	if (ra_msg->icmph.icmp6_hop_limit) {
> -		/* Only set hop_limit on the interface if it is higher than
> -		 * the current hop_limit.
> -		 */
> -		if (in6_dev->cnf.hop_limit < ra_msg->icmph.icmp6_hop_limit) {
> +	if (in6_dev->cnf.accept_ra_min_hop_limit < 256 &&
> +	    ra_msg->icmph.icmp6_hop_limit) {
> +		if (in6_dev->cnf.accept_ra_min_hop_limit <= ra_msg->icmph.icmp6_hop_limit) {
>  			in6_dev->cnf.hop_limit = ra_msg->icmph.icmp6_hop_limit;
> +			if (rt)
> +				dst_metric_set(&rt->dst, RTAX_HOPLIMIT,
> +					       ra_msg->icmph.icmp6_hop_limit);
>  		} else {
> -			ND_PRINTK(2, warn, "RA: Got route advertisement with lower hop_limit than current\n");
> +			ND_PRINTK(2, warn, "RA: Got route advertisement with lower hop_limit than minimum\n");
>  		}
> -		if (rt)
> -			dst_metric_set(&rt->dst, RTAX_HOPLIMIT,
> -				       ra_msg->icmph.icmp6_hop_limit);
>  	}
> 
>  skip_defrtr:
>
Luis Henriques March 24, 2016, 10:04 a.m. UTC | #2
On Tue, Mar 22, 2016 at 08:02:28PM +0900, YOSHIFUJI Hideaki/吉藤英明 wrote:
> > diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
> > index 593b0e32d956..f1d321427905 100644
> > --- a/include/uapi/linux/ipv6.h
> > +++ b/include/uapi/linux/ipv6.h
> > @@ -163,6 +163,7 @@ enum {
> >  	DEVCONF_MLDV1_UNSOLICITED_REPORT_INTERVAL,
> >  	DEVCONF_MLDV2_UNSOLICITED_REPORT_INTERVAL,
> >  	DEVCONF_SUPPRESS_FRAG_NDISC,
> 
> Please add followings:
>         __DEVCONF_ACCEPT_RA_FROM_LOCAL,
>         __DEVCONF_USE_OPTIMISTIC,
>         __DEVCONF_ACCEPT_RA_MTU,
>         __DEVCONF_STABLE_SECRET,
>         __DEVCONF_USE_OIF_ADDRS_ONLY,
> 
> Otherwise 
> 
> > +	DEVCONF_ACCEPT_RA_MIN_HOP_LIMIT,
> 
> has different value than the value we have in mainline.

Thank you for your review, Yoshifuji.  I guess that makes sense, as this
is an uapi include file.  I'll modify this patch to include these
additional definitions in the enum.

Cheers,
--
Luís
diff mbox

Patch

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index ab42c95f9985..36aa39eee48f 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1210,6 +1210,14 @@  accept_ra_defrtr - BOOLEAN
 	Functional default: enabled if accept_ra is enabled.
 			    disabled if accept_ra is disabled.

+accept_ra_min_hop_limit - INTEGER
+	Minimum hop limit Information in Router Advertisement.
+
+	Hop limit Information in Router Advertisement less than this
+	variable shall be ignored.
+
+	Default: 1
+
 accept_ra_pinfo - BOOLEAN
 	Learn Prefix Information in Router Advertisement.

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index f3d5d11b8871..9cc2240d7e52 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -29,6 +29,7 @@  struct ipv6_devconf {
 	__s32		max_desync_factor;
 	__s32		max_addresses;
 	__s32		accept_ra_defrtr;
+	__s32		accept_ra_min_hop_limit;
 	__s32		accept_ra_pinfo;
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	__s32		accept_ra_rtr_pref;
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 593b0e32d956..f1d321427905 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -163,6 +163,7 @@  enum {
 	DEVCONF_MLDV1_UNSOLICITED_REPORT_INTERVAL,
 	DEVCONF_MLDV2_UNSOLICITED_REPORT_INTERVAL,
 	DEVCONF_SUPPRESS_FRAG_NDISC,
+	DEVCONF_ACCEPT_RA_MIN_HOP_LIMIT,
 	DEVCONF_MAX
 };

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 75630e173adf..405575f9ae77 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -186,6 +186,7 @@  static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.max_desync_factor	= MAX_DESYNC_FACTOR,
 	.max_addresses		= IPV6_MAX_ADDRESSES,
 	.accept_ra_defrtr	= 1,
+	.accept_ra_min_hop_limit= 1,
 	.accept_ra_pinfo	= 1,
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	.accept_ra_rtr_pref	= 1,
@@ -222,6 +223,7 @@  static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.max_desync_factor	= MAX_DESYNC_FACTOR,
 	.max_addresses		= IPV6_MAX_ADDRESSES,
 	.accept_ra_defrtr	= 1,
+	.accept_ra_min_hop_limit= 1,
 	.accept_ra_pinfo	= 1,
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	.accept_ra_rtr_pref	= 1,
@@ -4294,6 +4296,7 @@  static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
 	array[DEVCONF_MAX_DESYNC_FACTOR] = cnf->max_desync_factor;
 	array[DEVCONF_MAX_ADDRESSES] = cnf->max_addresses;
 	array[DEVCONF_ACCEPT_RA_DEFRTR] = cnf->accept_ra_defrtr;
+	array[DEVCONF_ACCEPT_RA_MIN_HOP_LIMIT] = cnf->accept_ra_min_hop_limit;
 	array[DEVCONF_ACCEPT_RA_PINFO] = cnf->accept_ra_pinfo;
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	array[DEVCONF_ACCEPT_RA_RTR_PREF] = cnf->accept_ra_rtr_pref;
@@ -5079,6 +5082,13 @@  static struct addrconf_sysctl_table
 			.proc_handler	= proc_dointvec,
 		},
 		{
+			.procname	= "accept_ra_min_hop_limit",
+			.data		= &ipv6_devconf.accept_ra_min_hop_limit,
+			.maxlen		= sizeof(int),
+			.mode		= 0644,
+			.proc_handler	= proc_dointvec,
+		},
+		{
 			.procname	= "accept_ra_pinfo",
 			.data		= &ipv6_devconf.accept_ra_pinfo,
 			.maxlen		= sizeof(int),
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index e2eb53c719dc..577338204863 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1189,18 +1189,16 @@  static void ndisc_router_discovery(struct sk_buff *skb)

 	if (rt)
 		rt6_set_expires(rt, jiffies + (HZ * lifetime));
-	if (ra_msg->icmph.icmp6_hop_limit) {
-		/* Only set hop_limit on the interface if it is higher than
-		 * the current hop_limit.
-		 */
-		if (in6_dev->cnf.hop_limit < ra_msg->icmph.icmp6_hop_limit) {
+	if (in6_dev->cnf.accept_ra_min_hop_limit < 256 &&
+	    ra_msg->icmph.icmp6_hop_limit) {
+		if (in6_dev->cnf.accept_ra_min_hop_limit <= ra_msg->icmph.icmp6_hop_limit) {
 			in6_dev->cnf.hop_limit = ra_msg->icmph.icmp6_hop_limit;
+			if (rt)
+				dst_metric_set(&rt->dst, RTAX_HOPLIMIT,
+					       ra_msg->icmph.icmp6_hop_limit);
 		} else {
-			ND_PRINTK(2, warn, "RA: Got route advertisement with lower hop_limit than current\n");
+			ND_PRINTK(2, warn, "RA: Got route advertisement with lower hop_limit than minimum\n");
 		}
-		if (rt)
-			dst_metric_set(&rt->dst, RTAX_HOPLIMIT,
-				       ra_msg->icmph.icmp6_hop_limit);
 	}

 skip_defrtr: