Re: [PATCH 1/2 net v4] ipv6: addrconf: fix temp address generation after prefix deprecation

[Fernando Fernandez Mancera <fmancera@suse.de>]

On 5/13/26 4:35 PM, Ido Schimmel wrote:
> On Tue, May 12, 2026 at 08:24:27PM +0200, Fernando Fernandez Mancera wrote:
>> Sashiko feedback [1] is right about the DoS, that is a router that sends
>> multiple 0-lft RA until it exhausts all spawn attempts, leaving temporary
>> addresses disabled on the system.
>>
>> About the leaked address, I do not think the feedback is right. If an ifp
>> does not have any ift, it means something went wrong most likely. Either
>> this address was removed manually (any RA would restore it, even with
>> previous implementation) or for some reason that prefix didn't get an RA but
>> we didn't try to generate one and we MUST do it.
>>
>> I think we can cover it by avoiding to attempt create a new temporary
>> address for a 0-lft RA, it makes sense to me. Something like this:
>>
>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>> index 18a6f2de30ce..6c511e9c1bf5 100644
>> --- a/net/ipv6/addrconf.c
>> +++ b/net/ipv6/addrconf.c
>> @@ -2654,7 +2654,7 @@ static void manage_tempaddrs(struct inet6_dev *idev,
>>   	 * We don't want that to result in creating a new temporary ip address.
>>   	 */
>>   	if ((list_empty(&idev->tempaddr_list) || all_regen) &&
>> -	    (valid_lft || prefered_lft))
>> +	    (valid_lft && prefered_lft))
>>   		create = true;
>>
>>   	if (create && READ_ONCE(idev->cnf.use_tempaddr) > 0) {
>>
>> Any thoughts?
> 
> This still leaves the case of RAs that alternate between prefered_lft >
> 0 and prefered_lft == 0. It will cause the kernel to create an unbounded
> amount of temporary addresses.
> 
> I think that a better fix would be to reset the regeneration counter of
> the newest temporary address whenever the associated public address
> becomes preferred. Something like [1].
> 
> If the newest temporary address has yet to spawn a new address, then its
> regeneration counter is 0 and nothing changes. However, if it was
> incremented and no new address was created, then this patch will reset
> its regeneration counter to reflect that. Before expiring,
> addrconf_verify_rtnl() will notice that this address has yet to spawn a
> new address and call ipv6_create_tempaddr().
> 
> The case of constant RAs with prefered_lft == 0 is not an issue because
> we only reset the regeneration counter when prefered_lft > 0.
> 
> Alternating RAs are also not an issue because we don't call
> ipv6_create_tempaddr() immediately and instead let
> addrconf_verify_rtnl() handle it when the address is about to expire.
> 

Hi Ido, yes this looks good but I am afraid of using time_after(). Sure, 
a temporary address should never be older than 2 ~ 7 days. Yet, we allow 
configuring it up to 2147483647 (68~ years). And that might cause 
problems in 32-bit systems.

I think we either limit this configuration on sysctl directly or we need 
to support this nonsense.. another option would be to move inet6_ifaddr 
out of jiffies which is something that I recently added to my TODO list. 
But that is a bigger change I do not want to bundle with this one.

I noticed that ipv6_add_addr() do:

list_add(&ifa->tmp_list, &idev->tempaddr_list);

Doesn't it mean the first address that matches the base prefix is indeed 
the most recent one? I tried a bit and it seems we can use just the 
first match :)

In addition, I didn't check with Sashiko but it is likely going to 
complain about a race condition for a RA arriving right after we have 
spawned an address but it has not been added to the tempaddr list. 
Therefore we would have two addresses.. anyway, that race is quite 
unlikely. I also have planned work on that are to reduce the multiple 
race windows, currently there are plenty. I suggest to acknowledge such 
race condition for now and improve the code base for the reported scenario.

I propose this small change to your proposed diff, I acknowledge that it 
is a bit fragile tho (maybe a comment at ipv6_add_addr() is worth to 
avoid breaking this accidentally):

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index d550524f4266..af0d3fcbef98 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2607,7 +2607,7 @@ static void manage_tempaddrs(struct inet6_dev *idev,
  		if (ifp != ift->ifpub)
  			continue;

-		if (!newest_ift || time_after(ift->cstamp, newest_ift->cstamp))
+		if (!newest_ift)
  			newest_ift = ift;

  		/* RFC 4941 section 3.3:


Thanks!
Fernando.


> [1]
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 5476b6536eb7..d550524f4266 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2595,8 +2595,9 @@ static void manage_tempaddrs(struct inet6_dev *idev,
>                               __u32 valid_lft, __u32 prefered_lft,
>                               bool create, unsigned long now)
>   {
> +       struct inet6_ifaddr *ift, *newest_ift = NULL;
> +       u32 orig_prefered_lft = prefered_lft;
>          u32 flags;
> -       struct inet6_ifaddr *ift;
>   
>          read_lock_bh(&idev->lock);
>          /* update all temporary addresses in the list */
> @@ -2606,6 +2607,9 @@ static void manage_tempaddrs(struct inet6_dev *idev,
>                  if (ifp != ift->ifpub)
>                          continue;
>   
> +               if (!newest_ift || time_after(ift->cstamp, newest_ift->cstamp))
> +                       newest_ift = ift;
> +
>                  /* RFC 4941 section 3.3:
>                   * If a received option will extend the lifetime of a public
>                   * address, the lifetimes of temporary addresses should
> @@ -2643,6 +2647,12 @@ static void manage_tempaddrs(struct inet6_dev *idev,
>                          ipv6_ifa_notify(0, ift);
>          }
>   
> +       if (newest_ift && orig_prefered_lft > 0) {
> +               spin_lock(&newest_ift->lock);
> +               newest_ift->regen_count = 0;
> +               spin_unlock(&newest_ift->lock);
> +       }
> +
>          /* Also create a temporary address if it's enabled but no temporary
>           * address currently exists.
>           * However, we get called with valid_lft == 0, prefered_lft == 0, create == false