[PATCH 6.1 0/3] net: add sysctl accept_ra_min_lft

[PATCH 6.1 0/3] net: add sysctl accept_ra_min_lft

[Patrick Rohr]

This series adds a new sysctl accept_ra_min_lft which enforces a minimum
lifetime value for individual RA sections; in particular, router
lifetime, PIO preferred lifetime, and RIO lifetime. If any of those
lifetimes are lower than the configured value, the specific RA section
is ignored.

This fixes a potential denial of service attack vector where rogue WiFi
routers (or devices) can send RAs with low lifetimes to actively drain a
mobile device's battery (by preventing sleep).

In addition to this change, Android uses hardware offloads to drop RAs
for a fraction of the minimum of all lifetimes present in the RA (some
networks have very frequent RAs (5s) with high lifetimes (2h)). Despite
this, we have encountered networks that set the router lifetime to 30s
which results in very frequent CPU wakeups. Instead of disabling IPv6
(and dropping IPv6 ethertype in the WiFi firmware) entirely on such
networks, misconfigured routers must be ignored while still processing
RAs from other IPv6 routers on the same network (i.e. to support IoT
applications).

Patches:
- 1671bcfd76fd ("net: add sysctl accept_ra_min_rtr_lft")
- 5027d54a9c30 ("net: change accept_ra_min_rtr_lft to affect all RA lifetimes")
- 5cb249686e67 ("net: release reference to inet6_dev pointer")

Patrick Rohr (3):
  net: add sysctl accept_ra_min_rtr_lft
  net: change accept_ra_min_rtr_lft to affect all RA lifetimes
  net: release reference to inet6_dev pointer

 Documentation/networking/ip-sysctl.rst |  8 ++++++++
 include/linux/ipv6.h                   |  1 +
 include/uapi/linux/ipv6.h              |  1 +
 net/ipv6/addrconf.c                    | 13 +++++++++++++
 net/ipv6/ndisc.c                       | 13 +++++++++++--
 5 files changed, 34 insertions(+), 2 deletions(-)

-- 
2.42.0.515.g380fc7ccd1-goog

[PATCH 6.1 1/3] net: add sysctl accept_ra_min_rtr_lft

[Patrick Rohr]

This change adds a new sysctl accept_ra_min_rtr_lft to specify the
minimum acceptable router lifetime in an RA. If the received RA router
lifetime is less than the configured value (and not 0), the RA is
ignored.
This is useful for mobile devices, whose battery life can be impacted
by networks that configure RAs with a short lifetime. On such networks,
the device should never gain IPv6 provisioning and should attempt to
drop RAs via hardware offload, if available.

Signed-off-by: Patrick Rohr <prohr@google.com>
Cc: Maciej Żenczykowski <maze@google.com>
Cc: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 Documentation/networking/ip-sysctl.rst |  8 ++++++++
 include/linux/ipv6.h                   |  1 +
 include/uapi/linux/ipv6.h              |  1 +
 net/ipv6/addrconf.c                    | 10 ++++++++++
 net/ipv6/ndisc.c                       | 18 ++++++++++++++++--
 5 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 3301288a7c69..b0e9210f7a28 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -2148,6 +2148,14 @@ accept_ra_min_hop_limit - INTEGER
 
 	Default: 1
 
+accept_ra_min_rtr_lft - INTEGER
+	Minimum acceptable router lifetime in Router Advertisement.
+
+	RAs with a router lifetime less than this value shall be
+	ignored. RAs with a router lifetime of 0 are unaffected.
+
+	Default: 0
+
 accept_ra_pinfo - BOOLEAN
 	Learn Prefix Information in Router Advertisement.
 
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 15d7529ac953..a4b35f4c89d7 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -33,6 +33,7 @@ struct ipv6_devconf {
 	__s32		accept_ra_defrtr;
 	__u32		ra_defrtr_metric;
 	__s32		accept_ra_min_hop_limit;
+	__s32		accept_ra_min_rtr_lft;
 	__s32		accept_ra_pinfo;
 	__s32		ignore_routes_with_linkdown;
 #ifdef CONFIG_IPV6_ROUTER_PREF
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 53326dfc59ec..2038eff9b63f 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -198,6 +198,7 @@ enum {
 	DEVCONF_IOAM6_ID_WIDE,
 	DEVCONF_NDISC_EVICT_NOCARRIER,
 	DEVCONF_ACCEPT_UNTRACKED_NA,
+	DEVCONF_ACCEPT_RA_MIN_RTR_LFT,
 	DEVCONF_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 83be84219824..c7e939c619c9 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -202,6 +202,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.ra_defrtr_metric	= IP6_RT_PRIO_USER,
 	.accept_ra_from_local	= 0,
 	.accept_ra_min_hop_limit= 1,
+	.accept_ra_min_rtr_lft	= 0,
 	.accept_ra_pinfo	= 1,
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	.accept_ra_rtr_pref	= 1,
@@ -262,6 +263,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.ra_defrtr_metric	= IP6_RT_PRIO_USER,
 	.accept_ra_from_local	= 0,
 	.accept_ra_min_hop_limit= 1,
+	.accept_ra_min_rtr_lft	= 0,
 	.accept_ra_pinfo	= 1,
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	.accept_ra_rtr_pref	= 1,
@@ -5601,6 +5603,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
 	array[DEVCONF_IOAM6_ID_WIDE] = cnf->ioam6_id_wide;
 	array[DEVCONF_NDISC_EVICT_NOCARRIER] = cnf->ndisc_evict_nocarrier;
 	array[DEVCONF_ACCEPT_UNTRACKED_NA] = cnf->accept_untracked_na;
+	array[DEVCONF_ACCEPT_RA_MIN_RTR_LFT] = cnf->accept_ra_min_rtr_lft;
 }
 
 static inline size_t inet6_ifla6_size(void)
@@ -6794,6 +6797,13 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
+	{
+		.procname	= "accept_ra_min_rtr_lft",
+		.data		= &ipv6_devconf.accept_ra_min_rtr_lft,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+	},
 	{
 		.procname	= "accept_ra_pinfo",
 		.data		= &ipv6_devconf.accept_ra_pinfo,
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index a4d43eb45a9d..c2a6cda4be28 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1284,6 +1284,8 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 		return;
 	}
 
+	lifetime = ntohs(ra_msg->icmph.icmp6_rt_lifetime);
+
 	if (!ipv6_accept_ra(in6_dev)) {
 		ND_PRINTK(2, info,
 			  "RA: %s, did not accept ra for dev: %s\n",
@@ -1291,6 +1293,13 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 		goto skip_linkparms;
 	}
 
+	if (lifetime != 0 && lifetime < in6_dev->cnf.accept_ra_min_rtr_lft) {
+		ND_PRINTK(2, info,
+			  "RA: router lifetime (%ds) is too short: %s\n",
+			  lifetime, skb->dev->name);
+		goto skip_linkparms;
+	}
+
 #ifdef CONFIG_IPV6_NDISC_NODETYPE
 	/* skip link-specific parameters from interior routers */
 	if (skb->ndisc_nodetype == NDISC_NODETYPE_NODEFAULT) {
@@ -1343,8 +1352,6 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 		goto skip_defrtr;
 	}
 
-	lifetime = ntohs(ra_msg->icmph.icmp6_rt_lifetime);
-
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	pref = ra_msg->icmph.icmp6_router_pref;
 	/* 10b is handled as if it were 00b (medium) */
@@ -1495,6 +1502,13 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 		goto out;
 	}
 
+	if (lifetime != 0 && lifetime < in6_dev->cnf.accept_ra_min_rtr_lft) {
+		ND_PRINTK(2, info,
+			  "RA: router lifetime (%ds) is too short: %s\n",
+			  lifetime, skb->dev->name);
+		goto out;
+	}
+
 #ifdef CONFIG_IPV6_ROUTE_INFO
 	if (!in6_dev->cnf.accept_ra_from_local &&
 	    ipv6_chk_addr(dev_net(in6_dev->dev), &ipv6_hdr(skb)->saddr,
-- 
2.42.0.515.g380fc7ccd1-goog

[PATCH 6.1 2/3] net: change accept_ra_min_rtr_lft to affect all RA lifetimes

[Patrick Rohr]

accept_ra_min_rtr_lft only considered the lifetime of the default route
and discarded entire RAs accordingly.

This change renames accept_ra_min_rtr_lft to accept_ra_min_lft, and
applies the value to individual RA sections; in particular, router
lifetime, PIO preferred lifetime, and RIO lifetime. If any of those
lifetimes are lower than the configured value, the specific RA section
is ignored.

In order for the sysctl to be useful to Android, it should really apply
to all lifetimes in the RA, since that is what determines the minimum
frequency at which RAs must be processed by the kernel. Android uses
hardware offloads to drop RAs for a fraction of the minimum of all
lifetimes present in the RA (some networks have very frequent RAs (5s)
with high lifetimes (2h)). Despite this, we have encountered networks
that set the router lifetime to 30s which results in very frequent CPU
wakeups. Instead of disabling IPv6 (and dropping IPv6 ethertype in the
WiFi firmware) entirely on such networks, it seems better to ignore the
misconfigured routers while still processing RAs from other IPv6 routers
on the same network (i.e. to support IoT applications).

The previous implementation dropped the entire RA based on router
lifetime. This turned out to be hard to expand to the other lifetimes
present in the RA in a consistent manner; dropping the entire RA based
on RIO/PIO lifetimes would essentially require parsing the whole thing
twice.

Fixes: 1671bcfd76fd ("net: add sysctl accept_ra_min_rtr_lft")
Cc: Lorenzo Colitti <lorenzo@google.com>
Signed-off-by: Patrick Rohr <prohr@google.com>
Reviewed-by: Maciej Żenczykowski <maze@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Link: https://lore.kernel.org/r/20230726230701.919212-1-prohr@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 Documentation/networking/ip-sysctl.rst |  8 ++++----
 include/linux/ipv6.h                   |  2 +-
 include/uapi/linux/ipv6.h              |  2 +-
 net/ipv6/addrconf.c                    | 13 ++++++++-----
 net/ipv6/ndisc.c                       | 27 +++++++++++---------------
 5 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index b0e9210f7a28..f5f7a464605f 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -2148,11 +2148,11 @@ accept_ra_min_hop_limit - INTEGER
 
 	Default: 1
 
-accept_ra_min_rtr_lft - INTEGER
-	Minimum acceptable router lifetime in Router Advertisement.
+accept_ra_min_lft - INTEGER
+	Minimum acceptable lifetime value in Router Advertisement.
 
-	RAs with a router lifetime less than this value shall be
-	ignored. RAs with a router lifetime of 0 are unaffected.
+	RA sections with a lifetime less than this value shall be
+	ignored. Zero lifetimes stay unaffected.
 
 	Default: 0
 
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index a4b35f4c89d7..9a44de45cc1f 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -33,7 +33,7 @@ struct ipv6_devconf {
 	__s32		accept_ra_defrtr;
 	__u32		ra_defrtr_metric;
 	__s32		accept_ra_min_hop_limit;
-	__s32		accept_ra_min_rtr_lft;
+	__s32		accept_ra_min_lft;
 	__s32		accept_ra_pinfo;
 	__s32		ignore_routes_with_linkdown;
 #ifdef CONFIG_IPV6_ROUTER_PREF
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 2038eff9b63f..4fa8511b1e35 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -198,7 +198,7 @@ enum {
 	DEVCONF_IOAM6_ID_WIDE,
 	DEVCONF_NDISC_EVICT_NOCARRIER,
 	DEVCONF_ACCEPT_UNTRACKED_NA,
-	DEVCONF_ACCEPT_RA_MIN_RTR_LFT,
+	DEVCONF_ACCEPT_RA_MIN_LFT,
 	DEVCONF_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c7e939c619c9..53db8daa7385 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -202,7 +202,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.ra_defrtr_metric	= IP6_RT_PRIO_USER,
 	.accept_ra_from_local	= 0,
 	.accept_ra_min_hop_limit= 1,
-	.accept_ra_min_rtr_lft	= 0,
+	.accept_ra_min_lft	= 0,
 	.accept_ra_pinfo	= 1,
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	.accept_ra_rtr_pref	= 1,
@@ -263,7 +263,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.ra_defrtr_metric	= IP6_RT_PRIO_USER,
 	.accept_ra_from_local	= 0,
 	.accept_ra_min_hop_limit= 1,
-	.accept_ra_min_rtr_lft	= 0,
+	.accept_ra_min_lft	= 0,
 	.accept_ra_pinfo	= 1,
 #ifdef CONFIG_IPV6_ROUTER_PREF
 	.accept_ra_rtr_pref	= 1,
@@ -2733,6 +2733,9 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 		return;
 	}
 
+	if (valid_lft != 0 && valid_lft < in6_dev->cnf.accept_ra_min_lft)
+		return;
+
 	/*
 	 *	Two things going on here:
 	 *	1) Add routes for on-link prefixes
@@ -5603,7 +5606,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
 	array[DEVCONF_IOAM6_ID_WIDE] = cnf->ioam6_id_wide;
 	array[DEVCONF_NDISC_EVICT_NOCARRIER] = cnf->ndisc_evict_nocarrier;
 	array[DEVCONF_ACCEPT_UNTRACKED_NA] = cnf->accept_untracked_na;
-	array[DEVCONF_ACCEPT_RA_MIN_RTR_LFT] = cnf->accept_ra_min_rtr_lft;
+	array[DEVCONF_ACCEPT_RA_MIN_LFT] = cnf->accept_ra_min_lft;
 }
 
 static inline size_t inet6_ifla6_size(void)
@@ -6798,8 +6801,8 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.proc_handler	= proc_dointvec,
 	},
 	{
-		.procname	= "accept_ra_min_rtr_lft",
-		.data		= &ipv6_devconf.accept_ra_min_rtr_lft,
+		.procname	= "accept_ra_min_lft",
+		.data		= &ipv6_devconf.accept_ra_min_lft,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index c2a6cda4be28..6cb2d6a536a8 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1284,8 +1284,6 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 		return;
 	}
 
-	lifetime = ntohs(ra_msg->icmph.icmp6_rt_lifetime);
-
 	if (!ipv6_accept_ra(in6_dev)) {
 		ND_PRINTK(2, info,
 			  "RA: %s, did not accept ra for dev: %s\n",
@@ -1293,13 +1291,6 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 		goto skip_linkparms;
 	}
 
-	if (lifetime != 0 && lifetime < in6_dev->cnf.accept_ra_min_rtr_lft) {
-		ND_PRINTK(2, info,
-			  "RA: router lifetime (%ds) is too short: %s\n",
-			  lifetime, skb->dev->name);
-		goto skip_linkparms;
-	}
-
 #ifdef CONFIG_IPV6_NDISC_NODETYPE
 	/* skip link-specific parameters from interior routers */
 	if (skb->ndisc_nodetype == NDISC_NODETYPE_NODEFAULT) {
@@ -1340,6 +1331,14 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 		goto skip_defrtr;
 	}
 
+	lifetime = ntohs(ra_msg->icmph.icmp6_rt_lifetime);
+	if (lifetime != 0 && lifetime < in6_dev->cnf.accept_ra_min_lft) {
+		ND_PRINTK(2, info,
+			  "RA: router lifetime (%ds) is too short: %s\n",
+			  lifetime, skb->dev->name);
+		goto skip_defrtr;
+	}
+
 	/* Do not accept RA with source-addr found on local machine unless
 	 * accept_ra_from_local is set to true.
 	 */
@@ -1502,13 +1501,6 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 		goto out;
 	}
 
-	if (lifetime != 0 && lifetime < in6_dev->cnf.accept_ra_min_rtr_lft) {
-		ND_PRINTK(2, info,
-			  "RA: router lifetime (%ds) is too short: %s\n",
-			  lifetime, skb->dev->name);
-		goto out;
-	}
-
 #ifdef CONFIG_IPV6_ROUTE_INFO
 	if (!in6_dev->cnf.accept_ra_from_local &&
 	    ipv6_chk_addr(dev_net(in6_dev->dev), &ipv6_hdr(skb)->saddr,
@@ -1533,6 +1525,9 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 			if (ri->prefix_len == 0 &&
 			    !in6_dev->cnf.accept_ra_defrtr)
 				continue;
+			if (ri->lifetime != 0 &&
+			    ntohl(ri->lifetime) < in6_dev->cnf.accept_ra_min_lft)
+				continue;
 			if (ri->prefix_len < in6_dev->cnf.accept_ra_rt_info_min_plen)
 				continue;
 			if (ri->prefix_len > in6_dev->cnf.accept_ra_rt_info_max_plen)
-- 
2.42.0.515.g380fc7ccd1-goog

[PATCH 6.1 3/3] net: release reference to inet6_dev pointer

[Patrick Rohr]

addrconf_prefix_rcv returned early without releasing the inet6_dev
pointer when the PIO lifetime is less than accept_ra_min_lft.

Fixes: 5027d54a9c30 ("net: change accept_ra_min_rtr_lft to affect all RA lifetimes")
Cc: Maciej Żenczykowski <maze@google.com>
Cc: Lorenzo Colitti <lorenzo@google.com>
Cc: David Ahern <dsahern@kernel.org>
Cc: Simon Horman <horms@kernel.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Maciej Żenczykowski <maze@google.com>
Signed-off-by: Patrick Rohr <prohr@google.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/ipv6/addrconf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 53db8daa7385..96f4351b55a6 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2734,7 +2734,7 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 	}
 
 	if (valid_lft != 0 && valid_lft < in6_dev->cnf.accept_ra_min_lft)
-		return;
+		goto put;
 
 	/*
 	 *	Two things going on here:
-- 
2.42.0.515.g380fc7ccd1-goog

Re: [PATCH 6.1 0/3] net: add sysctl accept_ra_min_lft

[Patrick Rohr]

On Mon, Sep 25, 2023 at 2:10 PM Patrick Rohr <prohr@google.com> wrote:
>
> This series adds a new sysctl accept_ra_min_lft which enforces a minimum
> lifetime value for individual RA sections; in particular, router
> lifetime, PIO preferred lifetime, and RIO lifetime. If any of those
> lifetimes are lower than the configured value, the specific RA section
> is ignored.
>
> This fixes a potential denial of service attack vector where rogue WiFi
> routers (or devices) can send RAs with low lifetimes to actively drain a
> mobile device's battery (by preventing sleep).
>
> In addition to this change, Android uses hardware offloads to drop RAs
> for a fraction of the minimum of all lifetimes present in the RA (some
> networks have very frequent RAs (5s) with high lifetimes (2h)). Despite
> this, we have encountered networks that set the router lifetime to 30s
> which results in very frequent CPU wakeups. Instead of disabling IPv6
> (and dropping IPv6 ethertype in the WiFi firmware) entirely on such
> networks, misconfigured routers must be ignored while still processing
> RAs from other IPv6 routers on the same network (i.e. to support IoT
> applications).
>
> Patches:
> - 1671bcfd76fd ("net: add sysctl accept_ra_min_rtr_lft")
> - 5027d54a9c30 ("net: change accept_ra_min_rtr_lft to affect all RA lifetimes")
> - 5cb249686e67 ("net: release reference to inet6_dev pointer")
>
> Patrick Rohr (3):
>   net: add sysctl accept_ra_min_rtr_lft
>   net: change accept_ra_min_rtr_lft to affect all RA lifetimes
>   net: release reference to inet6_dev pointer
>
>  Documentation/networking/ip-sysctl.rst |  8 ++++++++
>  include/linux/ipv6.h                   |  1 +
>  include/uapi/linux/ipv6.h              |  1 +
>  net/ipv6/addrconf.c                    | 13 +++++++++++++
>  net/ipv6/ndisc.c                       | 13 +++++++++++--
>  5 files changed, 34 insertions(+), 2 deletions(-)
>
> --
> 2.42.0.515.g380fc7ccd1-goog
>

Was this rejected?
Any resolution on this (ACK or NAK) would be useful. Thanks!

Re: [PATCH 6.1 0/3] net: add sysctl accept_ra_min_lft

[Greg KH]

On Thu, Oct 05, 2023 at 02:37:59PM -0700, Patrick Rohr wrote:
> On Mon, Sep 25, 2023 at 2:10 PM Patrick Rohr <prohr@google.com> wrote:
> >
> > This series adds a new sysctl accept_ra_min_lft which enforces a minimum
> > lifetime value for individual RA sections; in particular, router
> > lifetime, PIO preferred lifetime, and RIO lifetime. If any of those
> > lifetimes are lower than the configured value, the specific RA section
> > is ignored.
> >
> > This fixes a potential denial of service attack vector where rogue WiFi
> > routers (or devices) can send RAs with low lifetimes to actively drain a
> > mobile device's battery (by preventing sleep).
> >
> > In addition to this change, Android uses hardware offloads to drop RAs
> > for a fraction of the minimum of all lifetimes present in the RA (some
> > networks have very frequent RAs (5s) with high lifetimes (2h)). Despite
> > this, we have encountered networks that set the router lifetime to 30s
> > which results in very frequent CPU wakeups. Instead of disabling IPv6
> > (and dropping IPv6 ethertype in the WiFi firmware) entirely on such
> > networks, misconfigured routers must be ignored while still processing
> > RAs from other IPv6 routers on the same network (i.e. to support IoT
> > applications).
> >
> > Patches:
> > - 1671bcfd76fd ("net: add sysctl accept_ra_min_rtr_lft")
> > - 5027d54a9c30 ("net: change accept_ra_min_rtr_lft to affect all RA lifetimes")
> > - 5cb249686e67 ("net: release reference to inet6_dev pointer")
> >
> > Patrick Rohr (3):
> >   net: add sysctl accept_ra_min_rtr_lft
> >   net: change accept_ra_min_rtr_lft to affect all RA lifetimes
> >   net: release reference to inet6_dev pointer
> >
> >  Documentation/networking/ip-sysctl.rst |  8 ++++++++
> >  include/linux/ipv6.h                   |  1 +
> >  include/uapi/linux/ipv6.h              |  1 +
> >  net/ipv6/addrconf.c                    | 13 +++++++++++++
> >  net/ipv6/ndisc.c                       | 13 +++++++++++--
> >  5 files changed, 34 insertions(+), 2 deletions(-)
> >
> > --
> > 2.42.0.515.g380fc7ccd1-goog
> >
> 
> Was this rejected?
> Any resolution on this (ACK or NAK) would be useful. Thanks!

They are in our "to get to" queue, which is very long still due to
multiple conferences and travel.

But I will note, you didn't put the git id of the patches in the patches
themselves, so it will take me extra work to add them there when
applying.

Also, why just 6.1?  What about newer stable kernels?  You can't update
and have a regression, right?

thanks,

greg k-h

Re: [PATCH 6.1 0/3] net: add sysctl accept_ra_min_lft

[Greg KH]

On Fri, Oct 06, 2023 at 07:52:19AM +0200, Greg KH wrote:
> On Thu, Oct 05, 2023 at 02:37:59PM -0700, Patrick Rohr wrote:
> > On Mon, Sep 25, 2023 at 2:10 PM Patrick Rohr <prohr@google.com> wrote:
> > >
> > > This series adds a new sysctl accept_ra_min_lft which enforces a minimum
> > > lifetime value for individual RA sections; in particular, router
> > > lifetime, PIO preferred lifetime, and RIO lifetime. If any of those
> > > lifetimes are lower than the configured value, the specific RA section
> > > is ignored.
> > >
> > > This fixes a potential denial of service attack vector where rogue WiFi
> > > routers (or devices) can send RAs with low lifetimes to actively drain a
> > > mobile device's battery (by preventing sleep).
> > >
> > > In addition to this change, Android uses hardware offloads to drop RAs
> > > for a fraction of the minimum of all lifetimes present in the RA (some
> > > networks have very frequent RAs (5s) with high lifetimes (2h)). Despite
> > > this, we have encountered networks that set the router lifetime to 30s
> > > which results in very frequent CPU wakeups. Instead of disabling IPv6
> > > (and dropping IPv6 ethertype in the WiFi firmware) entirely on such
> > > networks, misconfigured routers must be ignored while still processing
> > > RAs from other IPv6 routers on the same network (i.e. to support IoT
> > > applications).
> > >
> > > Patches:
> > > - 1671bcfd76fd ("net: add sysctl accept_ra_min_rtr_lft")
> > > - 5027d54a9c30 ("net: change accept_ra_min_rtr_lft to affect all RA lifetimes")
> > > - 5cb249686e67 ("net: release reference to inet6_dev pointer")
> > >
> > > Patrick Rohr (3):
> > >   net: add sysctl accept_ra_min_rtr_lft
> > >   net: change accept_ra_min_rtr_lft to affect all RA lifetimes
> > >   net: release reference to inet6_dev pointer
> > >
> > >  Documentation/networking/ip-sysctl.rst |  8 ++++++++
> > >  include/linux/ipv6.h                   |  1 +
> > >  include/uapi/linux/ipv6.h              |  1 +
> > >  net/ipv6/addrconf.c                    | 13 +++++++++++++
> > >  net/ipv6/ndisc.c                       | 13 +++++++++++--
> > >  5 files changed, 34 insertions(+), 2 deletions(-)
> > >
> > > --
> > > 2.42.0.515.g380fc7ccd1-goog
> > >
> > 
> > Was this rejected?
> > Any resolution on this (ACK or NAK) would be useful. Thanks!
> 
> They are in our "to get to" queue, which is very long still due to
> multiple conferences and travel.
> 
> But I will note, you didn't put the git id of the patches in the patches
> themselves, so it will take me extra work to add them there when
> applying.
> 
> Also, why just 6.1?  What about newer stable kernels?  You can't update
> and have a regression, right?

Note, because of this, we can not take these patches now at all anyway :(

thanks,

greg k-h

Re: [PATCH 6.1 0/3] net: add sysctl accept_ra_min_lft

[Maciej Żenczykowski]

On Thu, Oct 5, 2023 at 11:21 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Oct 06, 2023 at 07:52:19AM +0200, Greg KH wrote:
> > On Thu, Oct 05, 2023 at 02:37:59PM -0700, Patrick Rohr wrote:
> > > On Mon, Sep 25, 2023 at 2:10 PM Patrick Rohr <prohr@google.com> wrote:
> > > >
> > > > This series adds a new sysctl accept_ra_min_lft which enforces a minimum
> > > > lifetime value for individual RA sections; in particular, router
> > > > lifetime, PIO preferred lifetime, and RIO lifetime. If any of those
> > > > lifetimes are lower than the configured value, the specific RA section
> > > > is ignored.
> > > >
> > > > This fixes a potential denial of service attack vector where rogue WiFi
> > > > routers (or devices) can send RAs with low lifetimes to actively drain a
> > > > mobile device's battery (by preventing sleep).
> > > >
> > > > In addition to this change, Android uses hardware offloads to drop RAs
> > > > for a fraction of the minimum of all lifetimes present in the RA (some
> > > > networks have very frequent RAs (5s) with high lifetimes (2h)). Despite
> > > > this, we have encountered networks that set the router lifetime to 30s
> > > > which results in very frequent CPU wakeups. Instead of disabling IPv6
> > > > (and dropping IPv6 ethertype in the WiFi firmware) entirely on such
> > > > networks, misconfigured routers must be ignored while still processing
> > > > RAs from other IPv6 routers on the same network (i.e. to support IoT
> > > > applications).
> > > >
> > > > Patches:
> > > > - 1671bcfd76fd ("net: add sysctl accept_ra_min_rtr_lft")
> > > > - 5027d54a9c30 ("net: change accept_ra_min_rtr_lft to affect all RA lifetimes")
> > > > - 5cb249686e67 ("net: release reference to inet6_dev pointer")
> > > >
> > > > Patrick Rohr (3):
> > > >   net: add sysctl accept_ra_min_rtr_lft
> > > >   net: change accept_ra_min_rtr_lft to affect all RA lifetimes
> > > >   net: release reference to inet6_dev pointer
> > > >
> > > >  Documentation/networking/ip-sysctl.rst |  8 ++++++++
> > > >  include/linux/ipv6.h                   |  1 +
> > > >  include/uapi/linux/ipv6.h              |  1 +
> > > >  net/ipv6/addrconf.c                    | 13 +++++++++++++
> > > >  net/ipv6/ndisc.c                       | 13 +++++++++++--
> > > >  5 files changed, 34 insertions(+), 2 deletions(-)
> > > >
> > > > --
> > > > 2.42.0.515.g380fc7ccd1-goog
> > > >
> > >
> > > Was this rejected?
> > > Any resolution on this (ACK or NAK) would be useful. Thanks!
> >
> > They are in our "to get to" queue, which is very long still due to
> > multiple conferences and travel.
> >
> > But I will note, you didn't put the git id of the patches in the patches
> > themselves, so it will take me extra work to add them there when
> > applying.
> >
> > Also, why just 6.1?  What about newer stable kernels?  You can't update
> > and have a regression, right?
>
> Note, because of this, we can not take these patches now at all anyway :(
>
> thanks,
>
> greg k-h

Because without any knowledge of whether these patches would even be
accepted into stable, or whether they would need to go in via ACK,
preparing them for more trees seemed like pointless busywork...

If you're willing to take them for all stable (perhaps 5.10+?) trees
I'm sure Patrick can prepare them and resend them.

Would that be OK?

Re: [PATCH 6.1 0/3] net: add sysctl accept_ra_min_lft

[Greg KH]

On Fri, Oct 06, 2023 at 12:06:13AM -0700, Maciej Żenczykowski wrote:
> On Thu, Oct 5, 2023 at 11:21 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Oct 06, 2023 at 07:52:19AM +0200, Greg KH wrote:
> > > On Thu, Oct 05, 2023 at 02:37:59PM -0700, Patrick Rohr wrote:
> > > > On Mon, Sep 25, 2023 at 2:10 PM Patrick Rohr <prohr@google.com> wrote:
> > > > >
> > > > > This series adds a new sysctl accept_ra_min_lft which enforces a minimum
> > > > > lifetime value for individual RA sections; in particular, router
> > > > > lifetime, PIO preferred lifetime, and RIO lifetime. If any of those
> > > > > lifetimes are lower than the configured value, the specific RA section
> > > > > is ignored.
> > > > >
> > > > > This fixes a potential denial of service attack vector where rogue WiFi
> > > > > routers (or devices) can send RAs with low lifetimes to actively drain a
> > > > > mobile device's battery (by preventing sleep).
> > > > >
> > > > > In addition to this change, Android uses hardware offloads to drop RAs
> > > > > for a fraction of the minimum of all lifetimes present in the RA (some
> > > > > networks have very frequent RAs (5s) with high lifetimes (2h)). Despite
> > > > > this, we have encountered networks that set the router lifetime to 30s
> > > > > which results in very frequent CPU wakeups. Instead of disabling IPv6
> > > > > (and dropping IPv6 ethertype in the WiFi firmware) entirely on such
> > > > > networks, misconfigured routers must be ignored while still processing
> > > > > RAs from other IPv6 routers on the same network (i.e. to support IoT
> > > > > applications).
> > > > >
> > > > > Patches:
> > > > > - 1671bcfd76fd ("net: add sysctl accept_ra_min_rtr_lft")
> > > > > - 5027d54a9c30 ("net: change accept_ra_min_rtr_lft to affect all RA lifetimes")
> > > > > - 5cb249686e67 ("net: release reference to inet6_dev pointer")
> > > > >
> > > > > Patrick Rohr (3):
> > > > >   net: add sysctl accept_ra_min_rtr_lft
> > > > >   net: change accept_ra_min_rtr_lft to affect all RA lifetimes
> > > > >   net: release reference to inet6_dev pointer
> > > > >
> > > > >  Documentation/networking/ip-sysctl.rst |  8 ++++++++
> > > > >  include/linux/ipv6.h                   |  1 +
> > > > >  include/uapi/linux/ipv6.h              |  1 +
> > > > >  net/ipv6/addrconf.c                    | 13 +++++++++++++
> > > > >  net/ipv6/ndisc.c                       | 13 +++++++++++--
> > > > >  5 files changed, 34 insertions(+), 2 deletions(-)
> > > > >
> > > > > --
> > > > > 2.42.0.515.g380fc7ccd1-goog
> > > > >
> > > >
> > > > Was this rejected?
> > > > Any resolution on this (ACK or NAK) would be useful. Thanks!
> > >
> > > They are in our "to get to" queue, which is very long still due to
> > > multiple conferences and travel.
> > >
> > > But I will note, you didn't put the git id of the patches in the patches
> > > themselves, so it will take me extra work to add them there when
> > > applying.
> > >
> > > Also, why just 6.1?  What about newer stable kernels?  You can't update
> > > and have a regression, right?
> >
> > Note, because of this, we can not take these patches now at all anyway :(
> >
> > thanks,
> >
> > greg k-h
> 
> Because without any knowledge of whether these patches would even be
> accepted into stable, or whether they would need to go in via ACK,
> preparing them for more trees seemed like pointless busywork...

At the least, it's needed for 6.5-stable as again, we can't take
something for an older stable tree and not a newer one as that would be
a regression.  Without that backport present, we don't even waste our
time in reviewing stuff like this :)

thanks,

greg k-h

Re: [PATCH 6.1 0/3] net: add sysctl accept_ra_min_lft

[Sasha Levin]

On Fri, Oct 06, 2023 at 12:06:13AM -0700, Maciej Żenczykowski wrote:
>On Thu, Oct 5, 2023 at 11:21 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>>
>> On Fri, Oct 06, 2023 at 07:52:19AM +0200, Greg KH wrote:
>> > On Thu, Oct 05, 2023 at 02:37:59PM -0700, Patrick Rohr wrote:
>> > > On Mon, Sep 25, 2023 at 2:10 PM Patrick Rohr <prohr@google.com> wrote:
>> > > >
>> > > > This series adds a new sysctl accept_ra_min_lft which enforces a minimum
>> > > > lifetime value for individual RA sections; in particular, router
>> > > > lifetime, PIO preferred lifetime, and RIO lifetime. If any of those
>> > > > lifetimes are lower than the configured value, the specific RA section
>> > > > is ignored.
>> > > >
>> > > > This fixes a potential denial of service attack vector where rogue WiFi
>> > > > routers (or devices) can send RAs with low lifetimes to actively drain a
>> > > > mobile device's battery (by preventing sleep).
>> > > >
>> > > > In addition to this change, Android uses hardware offloads to drop RAs
>> > > > for a fraction of the minimum of all lifetimes present in the RA (some
>> > > > networks have very frequent RAs (5s) with high lifetimes (2h)). Despite
>> > > > this, we have encountered networks that set the router lifetime to 30s
>> > > > which results in very frequent CPU wakeups. Instead of disabling IPv6
>> > > > (and dropping IPv6 ethertype in the WiFi firmware) entirely on such
>> > > > networks, misconfigured routers must be ignored while still processing
>> > > > RAs from other IPv6 routers on the same network (i.e. to support IoT
>> > > > applications).
>> > > >
>> > > > Patches:
>> > > > - 1671bcfd76fd ("net: add sysctl accept_ra_min_rtr_lft")
>> > > > - 5027d54a9c30 ("net: change accept_ra_min_rtr_lft to affect all RA lifetimes")
>> > > > - 5cb249686e67 ("net: release reference to inet6_dev pointer")
>> > > >
>> > > > Patrick Rohr (3):
>> > > >   net: add sysctl accept_ra_min_rtr_lft
>> > > >   net: change accept_ra_min_rtr_lft to affect all RA lifetimes
>> > > >   net: release reference to inet6_dev pointer
>> > > >
>> > > >  Documentation/networking/ip-sysctl.rst |  8 ++++++++
>> > > >  include/linux/ipv6.h                   |  1 +
>> > > >  include/uapi/linux/ipv6.h              |  1 +
>> > > >  net/ipv6/addrconf.c                    | 13 +++++++++++++
>> > > >  net/ipv6/ndisc.c                       | 13 +++++++++++--
>> > > >  5 files changed, 34 insertions(+), 2 deletions(-)
>> > > >
>> > > > --
>> > > > 2.42.0.515.g380fc7ccd1-goog
>> > > >
>> > >
>> > > Was this rejected?
>> > > Any resolution on this (ACK or NAK) would be useful. Thanks!
>> >
>> > They are in our "to get to" queue, which is very long still due to
>> > multiple conferences and travel.
>> >
>> > But I will note, you didn't put the git id of the patches in the patches
>> > themselves, so it will take me extra work to add them there when
>> > applying.
>> >
>> > Also, why just 6.1?  What about newer stable kernels?  You can't update
>> > and have a regression, right?
>>
>> Note, because of this, we can not take these patches now at all anyway :(
>>
>> thanks,
>>
>> greg k-h
>
>Because without any knowledge of whether these patches would even be
>accepted into stable, or whether they would need to go in via ACK,
>preparing them for more trees seemed like pointless busywork...

FWIW, the above just means that we get to do the busywork rather than
the submitter...

-- 
Thanks,
Sasha

Re: [PATCH 6.1 0/3] net: add sysctl accept_ra_min_lft

[Maciej Żenczykowski]

> >> > > Was this rejected?
> >> > > Any resolution on this (ACK or NAK) would be useful. Thanks!
> >> >
> >> > They are in our "to get to" queue, which is very long still due to
> >> > multiple conferences and travel.
> >> >
> >> > But I will note, you didn't put the git id of the patches in the patches
> >> > themselves, so it will take me extra work to add them there when
> >> > applying.
> >> >
> >> > Also, why just 6.1?  What about newer stable kernels?  You can't update
> >> > and have a regression, right?
> >>
> >> Note, because of this, we can not take these patches now at all anyway :(
> >
> >Because without any knowledge of whether these patches would even be
> >accepted into stable, or whether they would need to go in via ACK,
> >preparing them for more trees seemed like pointless busywork...
>
> FWIW, the above just means that we get to do the busywork rather than
> the submitter...

We're certainly willing to do the work, but we're not entirely sure
what you want,
and whether you will indeed even accept these patches...
We're just trying to be mindful of everyone's time...

For example as a reviewer myself I know that in many cases it is
simply easier to do the
clean (!) cherrypick yourself (you presumably have scripts that
automate the entire thing),
rather than try to verify that someone else's cherrypick is actually
indeed clean.

These patches cleanly cherry pick, build, and pass our tests on
current 6.5 and 6.1 LTS.

git checkout remotes/linux-stable/v6.5.6 && git cherry-pick
1671bcfd76fdc0b9e65153cf759153083755fe4c && git cherry-pick
5027d54a9c30bc7ec808360378e2b4753f053f25 && git cherry-pick
5cb249686e67dbef3ffe53887fa725eefc5a7144  && run_uml_ack_net_test

(and same thing with 6.1.56)

Do you simply want the upstream sha1s?
Or do you want us to follow up with patches?

The situation gets more complex with 5.15 and older:

In this particular case, there are 3 patches, they could (and maybe
even should?)
be squashed into 1 patch for <=5.15 stable.  Greg didn't like that - I
get why - I don't have
an opinion here.  But it does result in complexities... for example:
one of the patches
adds to UAPI (and cannot be trivially cherrypicked to older kernels
because the enum
needs previous enums to be added first), and then the next patch renames it...

There's no clear obviously correct thing to do here, there's at least
a few possibilities:
(a) 4 patches: the first has no upstream equivalent and simply fast
forwards the enum
to the point where the next apply, 3 as clean as possible backports follow up
(b) 3 patches, the first one squashes in all the uapi enum fast forwarding
- including the patch that renames the UAPI constant
(c) just a single squashed patch
(d) we could also cherrypick without the UAPI portions... they're not
terribly important...
(e...) maybe other ways I've missed

Thanks,
- Maciej

Re: [PATCH 6.1 0/3] net: add sysctl accept_ra_min_lft

[Greg KH]

On Fri, Oct 06, 2023 at 12:40:36PM -0700, Maciej Żenczykowski wrote:
> > >> > > Was this rejected?
> > >> > > Any resolution on this (ACK or NAK) would be useful. Thanks!
> > >> >
> > >> > They are in our "to get to" queue, which is very long still due to
> > >> > multiple conferences and travel.
> > >> >
> > >> > But I will note, you didn't put the git id of the patches in the patches
> > >> > themselves, so it will take me extra work to add them there when
> > >> > applying.
> > >> >
> > >> > Also, why just 6.1?  What about newer stable kernels?  You can't update
> > >> > and have a regression, right?
> > >>
> > >> Note, because of this, we can not take these patches now at all anyway :(
> > >
> > >Because without any knowledge of whether these patches would even be
> > >accepted into stable, or whether they would need to go in via ACK,
> > >preparing them for more trees seemed like pointless busywork...
> >
> > FWIW, the above just means that we get to do the busywork rather than
> > the submitter...
> 
> We're certainly willing to do the work, but we're not entirely sure
> what you want,
> and whether you will indeed even accept these patches...
> We're just trying to be mindful of everyone's time...
> 
> For example as a reviewer myself I know that in many cases it is
> simply easier to do the
> clean (!) cherrypick yourself (you presumably have scripts that
> automate the entire thing),
> rather than try to verify that someone else's cherrypick is actually
> indeed clean.
> 
> These patches cleanly cherry pick, build, and pass our tests on
> current 6.5 and 6.1 LTS.
> 
> git checkout remotes/linux-stable/v6.5.6 && git cherry-pick
> 1671bcfd76fdc0b9e65153cf759153083755fe4c && git cherry-pick
> 5027d54a9c30bc7ec808360378e2b4753f053f25 && git cherry-pick
> 5cb249686e67dbef3ffe53887fa725eefc5a7144  && run_uml_ack_net_test
> 
> (and same thing with 6.1.56)
> 
> Do you simply want the upstream sha1s?

For things that cherry-pick cleanly, yes, that's easiest.

> Or do you want us to follow up with patches?

For kernels that cherry-picking does not apply cleanly, yes.

thanks,

greg k-h

Re: [PATCH 6.1 0/3] net: add sysctl accept_ra_min_lft

[Greg KH]

On Sat, Oct 07, 2023 at 11:30:41AM +0200, Greg KH wrote:
> On Fri, Oct 06, 2023 at 12:40:36PM -0700, Maciej Żenczykowski wrote:
> > > >> > > Was this rejected?
> > > >> > > Any resolution on this (ACK or NAK) would be useful. Thanks!
> > > >> >
> > > >> > They are in our "to get to" queue, which is very long still due to
> > > >> > multiple conferences and travel.
> > > >> >
> > > >> > But I will note, you didn't put the git id of the patches in the patches
> > > >> > themselves, so it will take me extra work to add them there when
> > > >> > applying.
> > > >> >
> > > >> > Also, why just 6.1?  What about newer stable kernels?  You can't update
> > > >> > and have a regression, right?
> > > >>
> > > >> Note, because of this, we can not take these patches now at all anyway :(
> > > >
> > > >Because without any knowledge of whether these patches would even be
> > > >accepted into stable, or whether they would need to go in via ACK,
> > > >preparing them for more trees seemed like pointless busywork...
> > >
> > > FWIW, the above just means that we get to do the busywork rather than
> > > the submitter...
> > 
> > We're certainly willing to do the work, but we're not entirely sure
> > what you want,
> > and whether you will indeed even accept these patches...
> > We're just trying to be mindful of everyone's time...
> > 
> > For example as a reviewer myself I know that in many cases it is
> > simply easier to do the
> > clean (!) cherrypick yourself (you presumably have scripts that
> > automate the entire thing),
> > rather than try to verify that someone else's cherrypick is actually
> > indeed clean.
> > 
> > These patches cleanly cherry pick, build, and pass our tests on
> > current 6.5 and 6.1 LTS.
> > 
> > git checkout remotes/linux-stable/v6.5.6 && git cherry-pick
> > 1671bcfd76fdc0b9e65153cf759153083755fe4c && git cherry-pick
> > 5027d54a9c30bc7ec808360378e2b4753f053f25 && git cherry-pick
> > 5cb249686e67dbef3ffe53887fa725eefc5a7144  && run_uml_ack_net_test
> > 
> > (and same thing with 6.1.56)
> > 
> > Do you simply want the upstream sha1s?
> 
> For things that cherry-pick cleanly, yes, that's easiest.

And I've cherry-picked these for 6.1.y and 6.5.y now.  If you all want
them in older kernels, please send working backports.

thanks,

greg k-h