From: Sam Edwards <cfsworks@gmail.com>
To: "David S . Miller" <davem@davemloft.net>,
Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
David Ahern <dsahern@kernel.org>
Cc: Linux Network Development Mailing List <netdev@vger.kernel.org>,
Sam Edwards <CFSworks@gmail.com>
Subject: [PATCH] ipv6/addrconf: fix timing bug in tempaddr regen
Date: Mon, 23 May 2022 14:25:43 -0600 [thread overview]
Message-ID: <20220523202543.9019-1-CFSworks@gmail.com> (raw)
The addrconf_verify_rtnl() function uses a big if/elseif/elseif/... block
to categorize each address by what type of attention it needs. An
about-to-expire (RFC 4941) temporary address is one such category, but the
previous elseif case catches addresses that have already run out their
prefered_lft. This means that if addrconf_verify_rtnl() fails to run in
the necessary time window (i.e. REGEN_ADVANCE time units before the end of
the prefered_lft), the temporary address will never be regenerated, and no
temporary addresses will be available until each one's valid_lft runs out
and manage_tempaddrs() begins anew.
Fix this by moving the entire temporary address regeneration case higher
up so that a temporary address cannot be deprecated until it has had an
opportunity to begin regeneration. Note that this does not fix the
problem of addrconf_verify_rtnl() sometimes not running in time resulting
in the race condition described in RFC 4941 section 3.4 - it only ensures
that the address is regenerated.
Fixing the latter problem may require either using jiffies instead of
seconds for all time arithmetic here, or always rounding up when
regen_advance is converted to seconds.
Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
net/ipv6/addrconf.c | 58 ++++++++++++++++++++++-----------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index b22504176588..5d02e4f0298b 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4518,6 +4518,35 @@ static void addrconf_verify_rtnl(struct net *net)
} else if (ifp->prefered_lft == INFINITY_LIFE_TIME) {
spin_unlock(&ifp->lock);
continue;
+ } else if ((ifp->flags&IFA_F_TEMPORARY) &&
+ !(ifp->flags&IFA_F_TENTATIVE) &&
+ !ifp->regen_count && ifp->ifpub) {
+ unsigned long regen_advance = ifp->idev->cnf.regen_max_retry *
+ ifp->idev->cnf.dad_transmits *
+ max(NEIGH_VAR(ifp->idev->nd_parms, RETRANS_TIME), HZ/100) / HZ;
+
+ if (age >= ifp->prefered_lft - regen_advance) {
+ struct inet6_ifaddr *ifpub = ifp->ifpub;
+ if (time_before(ifp->tstamp + ifp->prefered_lft * HZ, next))
+ next = ifp->tstamp + ifp->prefered_lft * HZ;
+
+ ifp->regen_count++;
+ in6_ifa_hold(ifp);
+ in6_ifa_hold(ifpub);
+ spin_unlock(&ifp->lock);
+
+ spin_lock(&ifpub->lock);
+ ifpub->regen_count = 0;
+ spin_unlock(&ifpub->lock);
+ rcu_read_unlock_bh();
+ ipv6_create_tempaddr(ifpub, true);
+ in6_ifa_put(ifpub);
+ in6_ifa_put(ifp);
+ rcu_read_lock_bh();
+ goto restart;
+ } else if (time_before(ifp->tstamp + ifp->prefered_lft * HZ - regen_advance * HZ, next))
+ next = ifp->tstamp + ifp->prefered_lft * HZ - regen_advance * HZ;
+ spin_unlock(&ifp->lock);
} else if (age >= ifp->prefered_lft) {
/* jiffies - ifp->tstamp > age >= ifp->prefered_lft */
int deprecate = 0;
@@ -4540,35 +4569,6 @@ static void addrconf_verify_rtnl(struct net *net)
in6_ifa_put(ifp);
goto restart;
}
- } else if ((ifp->flags&IFA_F_TEMPORARY) &&
- !(ifp->flags&IFA_F_TENTATIVE)) {
- unsigned long regen_advance = ifp->idev->cnf.regen_max_retry *
- ifp->idev->cnf.dad_transmits *
- max(NEIGH_VAR(ifp->idev->nd_parms, RETRANS_TIME), HZ/100) / HZ;
-
- if (age >= ifp->prefered_lft - regen_advance) {
- struct inet6_ifaddr *ifpub = ifp->ifpub;
- if (time_before(ifp->tstamp + ifp->prefered_lft * HZ, next))
- next = ifp->tstamp + ifp->prefered_lft * HZ;
- if (!ifp->regen_count && ifpub) {
- ifp->regen_count++;
- in6_ifa_hold(ifp);
- in6_ifa_hold(ifpub);
- spin_unlock(&ifp->lock);
-
- spin_lock(&ifpub->lock);
- ifpub->regen_count = 0;
- spin_unlock(&ifpub->lock);
- rcu_read_unlock_bh();
- ipv6_create_tempaddr(ifpub, true);
- in6_ifa_put(ifpub);
- in6_ifa_put(ifp);
- rcu_read_lock_bh();
- goto restart;
- }
- } else if (time_before(ifp->tstamp + ifp->prefered_lft * HZ - regen_advance * HZ, next))
- next = ifp->tstamp + ifp->prefered_lft * HZ - regen_advance * HZ;
- spin_unlock(&ifp->lock);
} else {
/* ifp->prefered_lft <= ifp->valid_lft */
if (time_before(ifp->tstamp + ifp->prefered_lft * HZ, next))
--
2.35.1
next reply other threads:[~2022-05-23 20:26 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-23 20:25 Sam Edwards [this message]
2022-05-24 9:24 ` [PATCH] ipv6/addrconf: fix timing bug in tempaddr regen Paolo Abeni
2022-05-25 20:07 ` Sam Edwards
2022-05-26 7:40 ` Paolo Abeni
2022-05-26 19:11 ` Sam Edwards
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220523202543.9019-1-CFSworks@gmail.com \
--to=cfsworks@gmail.com \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=yoshfuji@linux-ipv6.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.