Skip to content

Commit 76506a9

Browse files
jiribohacdavem330
authored andcommitted
IPv6: fix DESYNC_FACTOR
The IPv6 temporary address generation uses a variable called DESYNC_FACTOR to prevent hosts updating the addresses at the same time. Quoting RFC 4941: ... The value DESYNC_FACTOR is a random value (different for each client) that ensures that clients don't synchronize with each other and generate new addresses at exactly the same time ... DESYNC_FACTOR is defined as: DESYNC_FACTOR -- A random value within the range 0 - MAX_DESYNC_FACTOR. It is computed once at system start (rather than each time it is used) and must never be greater than (TEMP_VALID_LIFETIME - REGEN_ADVANCE). First, I believe the RFC has a typo in it and meant to say: "and must never be greater than (TEMP_PREFERRED_LIFETIME - REGEN_ADVANCE)" The reason is that at various places in the RFC, DESYNC_FACTOR is used in a calculation like (TEMP_PREFERRED_LIFETIME - DESYNC_FACTOR) or (TEMP_PREFERRED_LIFETIME - REGEN_ADVANCE - DESYNC_FACTOR). It needs to be smaller than (TEMP_PREFERRED_LIFETIME - REGEN_ADVANCE) for the result of these calculations to be larger than zero. It's never used in a calculation together with TEMP_VALID_LIFETIME. I already submitted an errata to the rfc-editor: https://www.rfc-editor.org/errata_search.php?rfc=4941 The Linux implementation of DESYNC_FACTOR is very wrong: max_desync_factor is used in places DESYNC_FACTOR should be used. max_desync_factor is initialized to the RFC-recommended value for MAX_DESYNC_FACTOR (600) but the whole point is to get a _random_ value. And nothing ensures that the value used is not greater than (TEMP_PREFERRED_LIFETIME - REGEN_ADVANCE), which leads to underflows. The effect can easily be observed when setting the temp_prefered_lft sysctl e.g. to 60. The preferred lifetime of the temporary addresses will be bogus. TEMP_PREFERRED_LIFETIME and REGEN_ADVANCE are not constants and can be influenced by these three sysctls: regen_max_retry, dad_transmits and temp_prefered_lft. Thus, the upper bound for desync_factor needs to be re-calculated each time a new address is generated and if desync_factor is larger than the new upper bound, a new random value needs to be re-generated. And since we already have max_desync_factor configurable per interface, we also need to calculate and store desync_factor per interface. Signed-off-by: Jiri Bohac <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 9d6280d commit 76506a9

File tree

2 files changed

+32
-8
lines changed

2 files changed

+32
-8
lines changed

include/net/if_inet6.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ struct inet6_dev {
190190
__u32 if_flags;
191191
int dead;
192192

193+
u32 desync_factor;
193194
u8 rndid[8];
194195
struct list_head tempaddr_list;
195196

net/ipv6/addrconf.c

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,7 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
422422
#endif
423423

424424
INIT_LIST_HEAD(&ndev->tempaddr_list);
425+
ndev->desync_factor = U32_MAX;
425426
if ((dev->flags&IFF_LOOPBACK) ||
426427
dev->type == ARPHRD_TUNNEL ||
427428
dev->type == ARPHRD_TUNNEL6 ||
@@ -1183,6 +1184,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
11831184
int ret = 0;
11841185
u32 addr_flags;
11851186
unsigned long now = jiffies;
1187+
long max_desync_factor;
11861188

11871189
write_lock_bh(&idev->lock);
11881190
if (ift) {
@@ -1218,20 +1220,41 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
12181220
ipv6_try_regen_rndid(idev, tmpaddr);
12191221
memcpy(&addr.s6_addr[8], idev->rndid, 8);
12201222
age = (now - ifp->tstamp) / HZ;
1223+
1224+
regen_advance = idev->cnf.regen_max_retry *
1225+
idev->cnf.dad_transmits *
1226+
NEIGH_VAR(idev->nd_parms, RETRANS_TIME) / HZ;
1227+
1228+
/* recalculate max_desync_factor each time and update
1229+
* idev->desync_factor if it's larger
1230+
*/
1231+
max_desync_factor = min_t(__u32,
1232+
idev->cnf.max_desync_factor,
1233+
idev->cnf.temp_prefered_lft - regen_advance);
1234+
1235+
if (unlikely(idev->desync_factor > max_desync_factor)) {
1236+
if (max_desync_factor > 0) {
1237+
get_random_bytes(&idev->desync_factor,
1238+
sizeof(idev->desync_factor));
1239+
idev->desync_factor %= max_desync_factor;
1240+
} else {
1241+
idev->desync_factor = 0;
1242+
}
1243+
}
1244+
12211245
tmp_valid_lft = min_t(__u32,
12221246
ifp->valid_lft,
12231247
idev->cnf.temp_valid_lft + age);
1224-
tmp_prefered_lft = min_t(__u32,
1225-
ifp->prefered_lft,
1226-
idev->cnf.temp_prefered_lft + age -
1227-
idev->cnf.max_desync_factor);
1248+
tmp_prefered_lft = idev->cnf.temp_prefered_lft + age -
1249+
idev->desync_factor;
1250+
/* guard against underflow in case of concurrent updates to cnf */
1251+
if (unlikely(tmp_prefered_lft < 0))
1252+
tmp_prefered_lft = 0;
1253+
tmp_prefered_lft = min_t(__u32, ifp->prefered_lft, tmp_prefered_lft);
12281254
tmp_plen = ifp->prefix_len;
12291255
tmp_tstamp = ifp->tstamp;
12301256
spin_unlock_bh(&ifp->lock);
12311257

1232-
regen_advance = idev->cnf.regen_max_retry *
1233-
idev->cnf.dad_transmits *
1234-
NEIGH_VAR(idev->nd_parms, RETRANS_TIME) / HZ;
12351258
write_unlock_bh(&idev->lock);
12361259

12371260
/* A temporary address is created only if this calculated Preferred
@@ -2316,7 +2339,7 @@ static void manage_tempaddrs(struct inet6_dev *idev,
23162339
max_valid = 0;
23172340

23182341
max_prefered = idev->cnf.temp_prefered_lft -
2319-
idev->cnf.max_desync_factor - age;
2342+
idev->desync_factor - age;
23202343
if (max_prefered < 0)
23212344
max_prefered = 0;
23222345

0 commit comments

Comments
 (0)