ipv6: Duplicate Address Detection completes for an already removed address

[adamgeorge309]

Summary

When an Internet Protocol version 6 (IPv6) address is removed from an interface while its Duplicate Address Detection (DAD) is running, the DAD entry stays in dadList and its timer keeps running. When the timer fires, processDadTimeout() completes DAD for an address the interface no longer holds.

One path on master that does this: a Router Advertisement (RA) carries two autonomous prefixes that are new to a host whose link-local DAD has completed. In Ipv6NeighbourDiscovery::processRaPrefixInfoForAddrAutoConf(), the first prefix finds only the link-local address, so its address is assigned tentative and its DAD starts (src/inet/networklayer/icmpv6/Ipv6NeighbourDiscovery.cc:2563-2573). The second prefix finds two addresses and takes the path written for a Mobile IPv6 handover. It records the first address as the old care-of address (getGlobalAddress(Ipv6InterfaceData::CoA) at :2559; assignAddress() types every global address as a care-of address when the Home Agent (H) flag of the RA is clear), marks every address tentative, and restarts DAD on the link-local address (:2578-2594).

If the link-local DAD completes first, makeTentativeAddressPermanent() removes the first address without touching its DAD entry:

// src/inet/networklayer/icmpv6/Ipv6NeighbourDiscovery.cc:914-917
        // moved from processRAPrefixInfoForAddrAutoConf()
        // we can remove the old CoA now
        if (!entry.CoA.isUnspecified())
            ie->getProtocolDataForUpdate<Ipv6InterfaceData>()->removeAddress(entry.CoA);

The orphaned timer then reaches makeTentativeAddressPermanent(tentativeAddr, ie) (:881), and permanentlyAssign() indexes the address list with -1:

// src/inet/networklayer/ipv6/Ipv6InterfaceData.cc:390-396
void Ipv6InterfaceData::permanentlyAssign(const Ipv6Address& addr)
{
    int k = findAddress(addr);
    ASSERT(k != -1);
    addresses[k].tentative = false;
    choosePreferredAddress();
}

The other removal sites in the module do not stop a running DAD either: an address whose prefix is advertised with a Valid Lifetime of zero (:1614) and the care-of address removed on returning home (:2547). Only dadHasFailed() (:983-991) deletes the DAD entry of the address it removes.

What the standard says

Request for Comments (RFC) 4862 (IPv6 Stateless Address Autoconfiguration), Section 5.4:

Duplicate Address Detection MUST be performed on all unicast addresses prior to assigning them to an interface

DAD verifies an address that is about to be assigned. An address that has been removed has nothing left to verify.

Why it matters

A release build writes one byte before the address vector and runs on, so the corruption is silent; a debug build stops with ASSERT: Condition 'k != -1' does not hold in function 'permanentlyAssign'.

Reproduced on master 49e1fa0 (4eb3bb4 has the same src/): a Router6 and a StandardHost6 on one 10 Mbps Ethernet link, an Ipv6FlatNetworkConfigurator, sim-time-limit = 10s, default Neighbour Discovery parameters. The router advertises the configurator's prefix aaaa:0:65::/64 and a second prefix aaaa:2::/64 from its routing table configuration:

**.router.routes = xml("<routingTable><local node='router'><interface name='eth0' AdvSendAdvertisements='on'><AdvPrefixList><AdvPrefix AdvValidLifetime='2592000' AdvOnLinkFlag='on' AdvPreferredLifetime='604800' AdvAutonomousFlag='on'>aaaa:2::/64</AdvPrefix></AdvPrefixList></interface></local></routingTable>")

At seed-set = 0 the host's log shows the removed address completing DAD after the link-local DAD removed it:

[4.394825462416] ...host.ipv6.neighbourDiscovery: DAD completed for address fe80::8aa:ff:fe00:2 on eth0, address is unique
[4.534878067138] ...host.ipv6.neighbourDiscovery: DAD completed for address aaaa:0:65:0:8aa:ff:fe00:2 on eth0, address is unique

valgrind, release build:

Invalid write of size 1
   at inet::Ipv6InterfaceData::permanentlyAssign(inet::Ipv6Address const&) (inet/networklayer/ipv6/Ipv6InterfaceData.cc:394)
   by inet::Ipv6NeighbourDiscovery::makeTentativeAddressPermanent(inet::Ipv6Address const&, inet::NetworkInterface*) (inet/networklayer/icmpv6/Ipv6NeighbourDiscovery.cc:888)
   by inet::Ipv6NeighbourDiscovery::processDadTimeout(omnetpp::cMessage*) (inet/networklayer/icmpv6/Ipv6NeighbourDiscovery.cc:881)
 Address 0x1d5b78d0 is 32 bytes before a block of size 192 in arena "client"

Which DAD completes first depends on the random delay each DAD adds to its first timeout: seeds 0, 3, 4, 5, 7 and 9 of 0-9 take this order in the scenario above.