[PATCH net] ipv6: bound temporary address regeneration advance

[PATCH net] ipv6: bound temporary address regeneration advance

[Yingjie Wang]

ipv6_get_regen_advance() combines regen_min_advance, regen_max_retry
and dad_transmits. All three are signed sysctls, and the latter two
are multiplied as int. Negative values or overflow of large positive
inputs can make the unsigned long result much larger than intended.

When ipv6_create_tempaddr() clamps the preferred lifetime, it adds age
and one to this advance and stores it in a u32. The sum can wrap or be
truncated to a short preferred lifetime. addrconf_verify_rtnl() then
finds the new address due for regeneration and restarts its scan after
creating another one. The temporary-address path has had no
max_addresses check since commit 4b08a8f1bd8c ("ipv6: remove
max_addresses check from ipv6_create_tempaddr"). In a baseline namespace
test, setting dad_transmits=-1 and adding a temporary-address template
with mngtmpaddr produced 2859 loopback addresses in two seconds.

Non-negative inputs can also overflow the int product, so rejecting
negative writes alone is not enough.

Reject negative values for all three sysctls with -EINVAL. Writes that
used to succeed now fail validation; negative values have no useful meaning
(negative regen_max_retry already disables temporary addresses on the
first attempt). Keep zero valid. Use checked u64 arithmetic and
div_u64() to calculate the advance, and cap it at U32_MAX. Compare the
required preferred lifetime in u64 before narrowing it to u32. Calculate
the desync margin only when the configured lifetime exceeds the advance,
avoiding an unsigned subtraction wrap on 32-bit builds.

The issue was found and this fix was prepared with assistance from
OpenAI Codex.

Tested on x86-64 KASAN:
- negative writes to the three sysctls were rejected without changing
  their values, and zero remained valid;
- normal temporary-address setup produced three IPv6 addresses;
- with HZ=1000 and a one-second retransmission time,
  regen_min_advance=0, regen_max_retry=2147483647 and dad_transmits=1
  caused the baseline kernel to reach 2847 loopback addresses after
  two seconds; the fixed kernel left two.

Kernel selftests were not run.

An i386 defconfig with CONFIG_IPV6=y and HZ=1000 built addrconf.o with
W=1; nm -u showed no __udivdi3 reference.

Fixes: f4bcbf360ac8 ("net: ipv6/addrconf: clamp preferred_lft to the minimum required")
Signed-off-by: Yingjie Wang <1075151112@qq.com>
---
 Documentation/networking/ip-sysctl.rst |  3 ++
 net/ipv6/addrconf.c                    | 50 ++++++++++++++++++--------
 2 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index 4b19c8d3efda7..e78a59ebc9df2 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -2872,6 +2872,7 @@ dad_transmits - INTEGER
 	The amount of Duplicate Address Detection probes to send.
 
 	Default: 1
+	Minimum: 0
 
 forwarding - INTEGER
 	Configure interface-specific Host/Router behaviour.
@@ -3023,12 +3024,14 @@ regen_min_advance - INTEGER
 	does not conform to RFC 8981.
 
 	Default: 2
+	Minimum: 0
 
 regen_max_retry - INTEGER
 	Number of attempts before give up attempting to generate
 	valid temporary addresses.
 
 	Default: 5
+	Minimum: 0
 
 max_addresses - INTEGER
 	Maximum number of autoconfigured addresses per interface.  Setting
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c8c0dd04982d9..39ecb2c25055b 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -60,7 +60,9 @@
 #endif
 #include <linux/capability.h>
 #include <linux/delay.h>
+#include <linux/math64.h>
 #include <linux/notifier.h>
+#include <linux/overflow.h>
 #include <linux/string.h>
 #include <linux/hash.h>
 
@@ -1376,10 +1378,19 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 
 static unsigned long ipv6_get_regen_advance(const struct inet6_dev *idev)
 {
-	return READ_ONCE(idev->cnf.regen_min_advance) +
-		READ_ONCE(idev->cnf.regen_max_retry) *
-		READ_ONCE(idev->cnf.dad_transmits) *
-		max(NEIGH_VAR(idev->nd_parms, RETRANS_TIME), HZ/100) / HZ;
+	unsigned int retrans_time = max(NEIGH_VAR(idev->nd_parms,
+						 RETRANS_TIME), HZ / 100);
+	u32 min_advance = READ_ONCE(idev->cnf.regen_min_advance);
+	u32 retries = READ_ONCE(idev->cnf.regen_max_retry);
+	u32 dad = READ_ONCE(idev->cnf.dad_transmits);
+	u64 duration;
+
+	/* Preferred lifetimes are u32, so a larger advance cannot be used. */
+	if (check_mul_overflow((u64)retries * dad, (u64)retrans_time,
+			       &duration))
+		return U32_MAX;
+
+	return min_t(u64, (u64)min_advance + div_u64(duration, HZ), U32_MAX);
 }
 
 static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
@@ -1394,6 +1405,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
 	struct ifa6_config cfg;
 	long max_desync_factor;
 	struct in6_addr addr;
+	u64 min_preferred_lft;
 	int ret = 0;
 
 	write_lock_bh(&idev->lock);
@@ -1430,9 +1442,13 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
 	 * idev->desync_factor if it's larger
 	 */
 	cnf_temp_preferred_lft = READ_ONCE(idev->cnf.temp_prefered_lft);
-	max_desync_factor = min_t(long,
-				  READ_ONCE(idev->cnf.max_desync_factor),
-				  cnf_temp_preferred_lft - regen_advance);
+	max_desync_factor = 0;
+	if (cnf_temp_preferred_lft > 0 &&
+	    regen_advance < cnf_temp_preferred_lft)
+		max_desync_factor =
+			min_t(long,
+			      READ_ONCE(idev->cnf.max_desync_factor),
+			      cnf_temp_preferred_lft - regen_advance);
 
 	if (unlikely(idev->desync_factor > max_desync_factor)) {
 		if (max_desync_factor > 0) {
@@ -1485,15 +1501,16 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
 	 * temporary addresses being generated.
 	 */
 	age = (now - tmp_tstamp + ADDRCONF_TIMER_FUZZ_MINUS) / HZ;
-	if (cfg.preferred_lft <= regen_advance + age) {
-		cfg.preferred_lft = regen_advance + age + 1;
-		if (cfg.preferred_lft > cfg.valid_lft ||
-		    cfg.preferred_lft > if_public_preferred_lft) {
+	min_preferred_lft = (u64)regen_advance + age + 1;
+	if (cfg.preferred_lft < min_preferred_lft) {
+		if (min_preferred_lft > cfg.valid_lft ||
+		    min_preferred_lft > if_public_preferred_lft) {
 			in6_ifa_put(ifp);
 			in6_dev_put(idev);
 			ret = -1;
 			goto out;
 		}
+		cfg.preferred_lft = min_preferred_lft;
 	}
 
 	cfg.ifa_flags = IFA_F_TEMPORARY;
@@ -4705,7 +4722,7 @@ static void addrconf_verify_rtnl(struct net *net)
 
 				unsigned long regen_advance = ipv6_get_regen_advance(ifp->idev);
 
-				if (age + regen_advance >= ifp->prefered_lft) {
+				if ((u64)age + regen_advance >= ifp->prefered_lft) {
 					struct inet6_ifaddr *ifpub = ifp->ifpub;
 					if (time_before(ifp->tstamp + ifp->prefered_lft * HZ, next))
 						next = ifp->tstamp + ifp->prefered_lft * HZ;
@@ -6921,7 +6938,8 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.data		= &ipv6_devconf.dad_transmits,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
 	},
 	{
 		.procname	= "router_solicitations",
@@ -7001,14 +7019,16 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.data           = &ipv6_devconf.regen_min_advance,
 		.maxlen         = sizeof(int),
 		.mode           = 0644,
-		.proc_handler   = proc_dointvec,
+		.proc_handler   = proc_dointvec_minmax,
+		.extra1         = SYSCTL_ZERO,
 	},
 	{
 		.procname	= "regen_max_retry",
 		.data		= &ipv6_devconf.regen_max_retry,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
 	},
 	{
 		.procname	= "max_desync_factor",

base-commit: 7076f684d5132e6a6145e1da2fa74771ecd052e5

Re: [PATCH net] ipv6: bound temporary address regeneration advance

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message names only regen_min_advance,
  regen_max_retry and dad_transmits as inputs to the overflowing int…

Pre-existing issues:
- [Low] In addrconf_verify_rtnl(), the deadline expressions `ifp->tstamp
  + ifp->prefered_lft * HZ` (lines 4727, 4744, 4783) multiply a __u32 by…

--

ipv6: bound temporary address regeneration advance

The commit makes the regen_min_advance, regen_max_retry and dad_transmits
sysctls reject negative values. It also reworks ipv6_get_regen_advance()
to use checked u64 arithmetic capped at U32_MAX. In ipv6_create_tempaddr()
the minimum preferred lifetime is now computed in u64 before narrowing,
and max_desync_factor is computed without an unsigned wrap.

> ipv6_get_regen_advance() combines regen_min_advance, regen_max_retry
> and dad_transmits. All three are signed sysctls, and the latter two
> are multiplied as int. Negative values or overflow of large positive
> inputs can make the unsigned long result much larger than intended.

[Severity: Medium]
Is this description complete? The old expression also multiplied by
NEIGH_VAR(idev->nd_parms, RETRANS_TIME). That value can come from an
on-link Router Advertisement, not only from a sysctl.

ndisc_router_discovery() in net/ipv6/ndisc.c stores the RA Retrans Timer
in the int RETRANS_TIME slot:

	if (in6_dev->nd_parms) {
		unsigned long rtime = ntohl(ra_msg->retrans_timer);

		if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT/HZ) {
			rtime = (rtime*HZ)/1000;
			if (rtime < HZ/100)
				rtime = HZ/100;
			NEIGH_VAR_SET(in6_dev->nd_parms, RETRANS_TIME, rtime);

On 64-bit, the rtime/1000 < MAX_SCHEDULE_TIMEOUT/HZ check always passes
for a u32. The only gate is ipv6_accept_ra(), which is on by default for
hosts.

Take the default sysctls (2/5/1), HZ=1000 and an RA Retrans Timer of
858992859 ms. The old int expression becomes:

    2 + (5 * 1 * 858992859) / 1000
  = 2 + (-3001) / 1000
  = 2 + (-3)
  = -1

So the old ipv6_get_regen_advance() returned ULONG_MAX. That is the same
state as the dad_transmits=-1 reproducer.

With age 0, the old ipv6_create_tempaddr() then set cfg.preferred_lft to
(u32)(ULONG_MAX + 1) = 0. Temporary addresses were therefore created
already deprecated.

Sometimes the new address does not stay tentative (accept_dad=0, or
IFF_NOARP devices). Then addrconf_verify_rtnl() sees
age + ULONG_MAX >= 0 and restarts its scan. This gives the same
address-creation loop under RTNL, and any RA that refreshes the public
address tstamp can start it again.

The new check_mul_overflow() in ipv6_get_regen_advance() does close this
path, because retrans_time now goes through the checked u64 multiply.

Could the commit message mention the RA-supplied Retrans Timer as an
unprivileged on-link trigger, next to the privileged sysctl reproducers?
That would matter for stable and security triage.

> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index c8c0dd04982d9..39ecb2c25055b 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c

[ ... ]

> @@ -4705,7 +4722,7 @@ static void addrconf_verify_rtnl(struct net *net)
>  
>  				unsigned long regen_advance = ipv6_get_regen_advance(ifp->idev);
>  
> -				if (age + regen_advance >= ifp->prefered_lft) {
> +				if ((u64)age + regen_advance >= ifp->prefered_lft) {
>  					struct inet6_ifaddr *ifpub = ifp->ifpub;
>  					if (time_before(ifp->tstamp + ifp->prefered_lft * HZ, next))
>  						next = ifp->tstamp + ifp->prefered_lft * HZ;

[Severity: Low]
This isn't a bug introduced by this patch, but can these deadline
calculations in addrconf_verify_rtnl() wrap by themselves?

ifp->prefered_lft is __u32 and HZ is an int constant, so
ifp->prefered_lft * HZ is computed in 32-bit unsigned arithmetic. At
HZ=1000 it wraps for preferred lifetimes above about 4294967 seconds.

The same pattern shows up further down in this function:

				} else if (time_before(ifp->tstamp + ifp->prefered_lft * HZ - regen_advance * HZ, next))
					next = ifp->tstamp + ifp->prefered_lft * HZ - regen_advance * HZ;
    ...
				/* ifp->prefered_lft <= ifp->valid_lft */
				if (time_before(ifp->tstamp + ifp->prefered_lft * HZ, next))
					next = ifp->tstamp + ifp->prefered_lft * HZ;

If next ends up in the past, the clamp at the end of the function
applies:

	if (time_before(next_sched, jiffies + ADDRCONF_TIMER_FUZZ_MAX))
		next_sched = jiffies + ADDRCONF_TIMER_FUZZ_MAX;

Would addr_chk_work then be rescheduled every ADDRCONF_TIMER_FUZZ_MAX
until the real deadline? Each run would do a needless full scan of the
address table under RTNL.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/tencent_F6E8C7CAC6D390C8E912EEE4FD9FE4333505%40qq.com

Re: [PATCH net] ipv6: bound temporary address regeneration advance

[Paolo Abeni]

On 10/26/26 09:22, Yingjie Wang wrote:
> @@ -1394,6 +1405,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
>   	struct ifa6_config cfg;
>   	long max_desync_factor;
>   	struct in6_addr addr;
> +	u64 min_preferred_lft;

Minor nit: please fix the variable declaration order above.

>   	int ret = 0;
>   
>   	write_lock_bh(&idev->lock);
> @@ -1430,9 +1442,13 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
>   	 * idev->desync_factor if it's larger
>   	 */
>   	cnf_temp_preferred_lft = READ_ONCE(idev->cnf.temp_prefered_lft);
> -	max_desync_factor = min_t(long,
> -				  READ_ONCE(idev->cnf.max_desync_factor),
> -				  cnf_temp_preferred_lft - regen_advance);
> +	max_desync_factor = 0;
> +	if (cnf_temp_preferred_lft > 0 &&
> +	    regen_advance < cnf_temp_preferred_lft)
> +		max_desync_factor =
> +			min_t(long,
> +			      READ_ONCE(idev->cnf.max_desync_factor),
> +			      cnf_temp_preferred_lft - regen_advance);

The above is quite ugly, you can probably make the statement more
palatable using some additional variable to store the intermediate
values.

/P

[PATCH net v2] ipv6: bound temporary address regeneration advance

[Yingjie Wang]

ipv6_get_regen_advance() combines regen_min_advance, regen_max_retry,
dad_transmits and the neighbor retransmission time. The first three are
signed sysctls, and the product is evaluated as int. Negative values or
signed overflow can make the unsigned long result much larger than
intended.

The retransmission time can also be supplied by an on-link Router
Advertisement. On an x86-64 HZ=1000 host with regen_min_advance=2,
regen_max_retry=3 and dad_transmits=1, an RA Retrans Timer of
1431654765 ms makes the old int product wrap to -3001. Division by HZ
and addition of regen_min_advance then produce -1, which is returned as
ULONG_MAX. This gives an on-link trigger on hosts that accept RAs and
use temporary addresses without requiring a sysctl write.

When ipv6_create_tempaddr() clamps the preferred lifetime, it adds age
and one to this advance and stores it in a u32. The sum can wrap or be
truncated to a short preferred lifetime. addrconf_verify_rtnl() then
finds the new address due for regeneration and restarts its scan after
creating another one. The temporary-address path has had no
max_addresses check since commit 4b08a8f1bd8c ("ipv6: remove
max_addresses check from ipv6_create_tempaddr"). In a baseline namespace
test, setting dad_transmits=-1 and adding a temporary-address template
with mngtmpaddr produced 2859 loopback addresses in two seconds.

Reject negative values for all three sysctls with -EINVAL. Writes that
used to succeed now fail validation; negative values have no useful
meaning (negative regen_max_retry already disables temporary addresses
on the first attempt). Keep zero valid. Use checked u64 arithmetic and
div_u64() to calculate the advance, including values supplied by an RA,
and cap it at U32_MAX. Compare the required preferred lifetime in u64
before narrowing it to u32. Calculate the desync margin without an
unsigned subtraction wrap on 32-bit builds. Clamp the effective maximum
to that non-negative margin so a negative max_desync_factor cannot make
the u32 comparison architecture-dependent.

Tested on x86-64 KASAN:
- negative writes to the three sysctls were rejected without changing
  their values, and zero remained valid;
- normal temporary-address setup and a case with writable
  max_desync_factor=-1 each produced three IPv6 addresses;
- with HZ=1000 and a one-second retransmission time,
  regen_min_advance=0, regen_max_retry=2147483647 and dad_transmits=1
  caused the baseline kernel to reach 2847 loopback addresses after
  two seconds; the fixed kernel left two.

Kernel selftests were not run.

An i386 defconfig with CONFIG_IPV6=y and HZ=1000 built addrconf.o with
W=1; nm -u showed no __udivdi3 reference.

Fixes: f4bcbf360ac8 ("net: ipv6/addrconf: clamp preferred_lft to the minimum required")
Signed-off-by: Yingjie Wang <1075151112@qq.com>
---
Changes in v2:
- Reorder the declarations for the netdev reverse-Christmas-tree convention.
- Use an intermediate variable to make the max_desync_factor calculation
  easier to read, as requested by Paolo Abeni.
- Clamp the effective max_desync_factor to a non-negative range so the u32
  comparison has the same result on 32-bit and 64-bit builds.
- Document the RA-supplied Retrans Timer trigger raised in review.

Link: https://patchwork.kernel.org/project/netdevbpf/patch/tencent_F6E8C7CAC6D390C8E912EEE4FD9FE4333505@qq.com/

 Documentation/networking/ip-sysctl.rst |  3 ++
 net/ipv6/addrconf.c                    | 53 ++++++++++++++++++--------
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index d3523c9064a22..817cba10e1787 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -2874,6 +2874,7 @@ dad_transmits - INTEGER
 	The amount of Duplicate Address Detection probes to send.
 
 	Default: 1
+	Minimum: 0
 
 forwarding - INTEGER
 	Configure interface-specific Host/Router behaviour.
@@ -3025,12 +3026,14 @@ regen_min_advance - INTEGER
 	does not conform to RFC 8981.
 
 	Default: 2
+	Minimum: 0
 
 regen_max_retry - INTEGER
 	Number of attempts before give up attempting to generate
 	valid temporary addresses.
 
 	Default: 5
+	Minimum: 0
 
 max_addresses - INTEGER
 	Maximum number of autoconfigured addresses per interface.  Setting
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c8c0dd04982d9..ff2a29c94bdfe 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -60,7 +60,9 @@
 #endif
 #include <linux/capability.h>
 #include <linux/delay.h>
+#include <linux/math64.h>
 #include <linux/notifier.h>
+#include <linux/overflow.h>
 #include <linux/string.h>
 #include <linux/hash.h>
 
@@ -1376,10 +1378,19 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 
 static unsigned long ipv6_get_regen_advance(const struct inet6_dev *idev)
 {
-	return READ_ONCE(idev->cnf.regen_min_advance) +
-		READ_ONCE(idev->cnf.regen_max_retry) *
-		READ_ONCE(idev->cnf.dad_transmits) *
-		max(NEIGH_VAR(idev->nd_parms, RETRANS_TIME), HZ/100) / HZ;
+	unsigned int retrans_time = max(NEIGH_VAR(idev->nd_parms,
+						 RETRANS_TIME), HZ / 100);
+	u32 min_advance = READ_ONCE(idev->cnf.regen_min_advance);
+	u32 retries = READ_ONCE(idev->cnf.regen_max_retry);
+	u32 dad = READ_ONCE(idev->cnf.dad_transmits);
+	u64 duration;
+
+	/* Preferred lifetimes are u32, so a larger advance cannot be used. */
+	if (check_mul_overflow((u64)retries * dad, (u64)retrans_time,
+			       &duration))
+		return U32_MAX;
+
+	return min_t(u64, (u64)min_advance + div_u64(duration, HZ), U32_MAX);
 }
 
 static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
@@ -1391,8 +1402,10 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
 	u32 if_public_preferred_lft;
 	s32 cnf_temp_preferred_lft;
 	struct inet6_ifaddr *ift;
+	u32 desync_factor_limit;
 	struct ifa6_config cfg;
 	long max_desync_factor;
+	u64 min_preferred_lft;
 	struct in6_addr addr;
 	int ret = 0;
 
@@ -1430,9 +1443,15 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
 	 * idev->desync_factor if it's larger
 	 */
 	cnf_temp_preferred_lft = READ_ONCE(idev->cnf.temp_prefered_lft);
-	max_desync_factor = min_t(long,
-				  READ_ONCE(idev->cnf.max_desync_factor),
-				  cnf_temp_preferred_lft - regen_advance);
+	desync_factor_limit = max_t(s32, cnf_temp_preferred_lft, 0);
+	if (regen_advance >= desync_factor_limit)
+		desync_factor_limit = 0;
+	else
+		desync_factor_limit -= regen_advance;
+
+	max_desync_factor = clamp_t(long,
+				    READ_ONCE(idev->cnf.max_desync_factor),
+				    0, desync_factor_limit);
 
 	if (unlikely(idev->desync_factor > max_desync_factor)) {
 		if (max_desync_factor > 0) {
@@ -1485,15 +1504,16 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
 	 * temporary addresses being generated.
 	 */
 	age = (now - tmp_tstamp + ADDRCONF_TIMER_FUZZ_MINUS) / HZ;
-	if (cfg.preferred_lft <= regen_advance + age) {
-		cfg.preferred_lft = regen_advance + age + 1;
-		if (cfg.preferred_lft > cfg.valid_lft ||
-		    cfg.preferred_lft > if_public_preferred_lft) {
+	min_preferred_lft = (u64)regen_advance + age + 1;
+	if (cfg.preferred_lft < min_preferred_lft) {
+		if (min_preferred_lft > cfg.valid_lft ||
+		    min_preferred_lft > if_public_preferred_lft) {
 			in6_ifa_put(ifp);
 			in6_dev_put(idev);
 			ret = -1;
 			goto out;
 		}
+		cfg.preferred_lft = min_preferred_lft;
 	}
 
 	cfg.ifa_flags = IFA_F_TEMPORARY;
@@ -4705,7 +4725,7 @@ static void addrconf_verify_rtnl(struct net *net)
 
 				unsigned long regen_advance = ipv6_get_regen_advance(ifp->idev);
 
-				if (age + regen_advance >= ifp->prefered_lft) {
+				if ((u64)age + regen_advance >= ifp->prefered_lft) {
 					struct inet6_ifaddr *ifpub = ifp->ifpub;
 					if (time_before(ifp->tstamp + ifp->prefered_lft * HZ, next))
 						next = ifp->tstamp + ifp->prefered_lft * HZ;
@@ -6921,7 +6941,8 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.data		= &ipv6_devconf.dad_transmits,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
 	},
 	{
 		.procname	= "router_solicitations",
@@ -7001,14 +7022,16 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.data           = &ipv6_devconf.regen_min_advance,
 		.maxlen         = sizeof(int),
 		.mode           = 0644,
-		.proc_handler   = proc_dointvec,
+		.proc_handler   = proc_dointvec_minmax,
+		.extra1         = SYSCTL_ZERO,
 	},
 	{
 		.procname	= "regen_max_retry",
 		.data		= &ipv6_devconf.regen_max_retry,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
 	},
 	{
 		.procname	= "max_desync_factor",

base-commit: d7314b51341c037973ce022f12c6009fd07b021e
-- 
2.43.0

Re: [PATCH net v2] ipv6: bound temporary address regeneration advance

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v2] ipv6: bound temporary address regeneration advance

[Ido Schimmel]

On Thu, Oct 29, 2026 at 11:26:27PM +0800, Yingjie Wang wrote:
> ipv6_get_regen_advance() combines regen_min_advance, regen_max_retry,
> dad_transmits and the neighbor retransmission time. The first three are
> signed sysctls, and the product is evaluated as int. Negative values or
> signed overflow can make the unsigned long result much larger than
> intended.
> 
> The retransmission time can also be supplied by an on-link Router
> Advertisement. On an x86-64 HZ=1000 host with regen_min_advance=2,
> regen_max_retry=3 and dad_transmits=1, an RA Retrans Timer of
> 1431654765 ms makes the old int product wrap to -3001. Division by HZ
> and addition of regen_min_advance then produce -1, which is returned as
> ULONG_MAX. This gives an on-link trigger on hosts that accept RAs and
> use temporary addresses without requiring a sysctl write.
> 
> When ipv6_create_tempaddr() clamps the preferred lifetime, it adds age
> and one to this advance and stores it in a u32. The sum can wrap or be
> truncated to a short preferred lifetime. addrconf_verify_rtnl() then
> finds the new address due for regeneration and restarts its scan after
> creating another one. The temporary-address path has had no
> max_addresses check since commit 4b08a8f1bd8c ("ipv6: remove
> max_addresses check from ipv6_create_tempaddr"). In a baseline namespace
> test, setting dad_transmits=-1 and adding a temporary-address template
> with mngtmpaddr produced 2859 loopback addresses in two seconds.
> 
> Reject negative values for all three sysctls with -EINVAL. Writes that
> used to succeed now fail validation; negative values have no useful
> meaning (negative regen_max_retry already disables temporary addresses
> on the first attempt). Keep zero valid. Use checked u64 arithmetic and
> div_u64() to calculate the advance, including values supplied by an RA,
> and cap it at U32_MAX. Compare the required preferred lifetime in u64
> before narrowing it to u32. Calculate the desync margin without an
> unsigned subtraction wrap on 32-bit builds. Clamp the effective maximum
> to that non-negative margin so a negative max_desync_factor cannot make
> the u32 comparison architecture-dependent.
> 
> Tested on x86-64 KASAN:
> - negative writes to the three sysctls were rejected without changing
>   their values, and zero remained valid;
> - normal temporary-address setup and a case with writable
>   max_desync_factor=-1 each produced three IPv6 addresses;
> - with HZ=1000 and a one-second retransmission time,
>   regen_min_advance=0, regen_max_retry=2147483647 and dad_transmits=1
>   caused the baseline kernel to reach 2847 loopback addresses after
>   two seconds; the fixed kernel left two.
> 
> Kernel selftests were not run.
> 
> An i386 defconfig with CONFIG_IPV6=y and HZ=1000 built addrconf.o with
> W=1; nm -u showed no __udivdi3 reference.
> 
> Fixes: f4bcbf360ac8 ("net: ipv6/addrconf: clamp preferred_lft to the minimum required")
> Signed-off-by: Yingjie Wang <1075151112@qq.com>
> ---
> Changes in v2:
> - Reorder the declarations for the netdev reverse-Christmas-tree convention.
> - Use an intermediate variable to make the max_desync_factor calculation
>   easier to read, as requested by Paolo Abeni.
> - Clamp the effective max_desync_factor to a non-negative range so the u32
>   comparison has the same result on 32-bit and 64-bit builds.
> - Document the RA-supplied Retrans Timer trigger raised in review.
> 
> Link: https://patchwork.kernel.org/project/netdevbpf/patch/tencent_F6E8C7CAC6D390C8E912EEE4FD9FE4333505@qq.com/
> 
>  Documentation/networking/ip-sysctl.rst |  3 ++
>  net/ipv6/addrconf.c                    | 53 ++++++++++++++++++--------
>  2 files changed, 41 insertions(+), 15 deletions(-)

This patch is very hard to understand.

My understanding is that ipv6_create_tempaddr() makes sure that it's not
creating a temporary address that requires immediate regeneration. It's
doing that by setting the preferred lifetime of such an address to
'regen_advance + age + 1', so that at least one second will pass before
a new temporary address needs to be generated.

The problem is that this expression can overflow to 0 or be truncated when
assigned to 'preferred_lft' if it's larger than U32_MAX, resulting in a
preferred lifetime that requires immediate regeneration.

Can't we solve it with the following diff?

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c8c0dd04982d..d9d13e78bf13 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1487,7 +1487,8 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, bool block)
 	age = (now - tmp_tstamp + ADDRCONF_TIMER_FUZZ_MINUS) / HZ;
 	if (cfg.preferred_lft <= regen_advance + age) {
 		cfg.preferred_lft = regen_advance + age + 1;
-		if (cfg.preferred_lft > cfg.valid_lft ||
+		if (cfg.preferred_lft <= regen_advance + age ||
+		    cfg.preferred_lft > cfg.valid_lft ||
 		    cfg.preferred_lft > if_public_preferred_lft) {
 			in6_ifa_put(ifp);
 			in6_dev_put(idev);