Merge release-20210301.0-18-g498709250 (automated)

4 files changed, 29 insertions, 19 deletions

diff --git a/pkg/tcpip/network/arp/arp.go b/pkg/tcpip/network/arp/arp.go
index 98ded525c..ae0461a6d 100644
--- a/pkg/tcpip/network/arp/arp.go
+++ b/pkg/tcpip/network/arp/arp.go
@@ -232,7 +232,7 @@ func (e *endpoint) HandlePacket(pkt *stack.PacketBuffer) {
 		linkAddr := tcpip.LinkAddress(h.HardwareAddressSender())
 
 		e.mu.Lock()
-		e.mu.dad.StopLocked(addr, &stack.DADDupAddrDetected{})
+		e.mu.dad.StopLocked(addr, &stack.DADDupAddrDetected{HolderLinkAddress: linkAddr})
 		e.mu.Unlock()
 
 		// The solicited, override, and isRouter flags are not available for ARP;
diff --git a/pkg/tcpip/network/ipv6/icmp.go b/pkg/tcpip/network/ipv6/icmp.go
index 1cebe75ec..6344a3e09 100644
--- a/pkg/tcpip/network/ipv6/icmp.go
+++ b/pkg/tcpip/network/ipv6/icmp.go
@@ -382,6 +382,10 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r
 			// stack know so it can handle such a scenario and do nothing further with
 			// the NS.
 			if srcAddr == header.IPv6Any {
+				// Since this is a DAD message we know the sender does not actually hold
+				// the target address so there is no "holder".
+				var holderLinkAddress tcpip.LinkAddress
+
 				// We would get an error if the address no longer exists or the address
 				// is no longer tentative (DAD resolved between the call to
 				// hasTentativeAddr and this point). Both of these are valid scenarios:
@@ -393,7 +397,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r
 				//
 				// TODO(gvisor.dev/issue/4046): Handle the scenario when a duplicate
 				// address is detected for an assigned address.
-				switch err := e.dupTentativeAddrDetected(targetAddr); err.(type) {
+				switch err := e.dupTentativeAddrDetected(targetAddr, holderLinkAddress); err.(type) {
 				case nil, *tcpip.ErrBadAddress, *tcpip.ErrInvalidEndpointState:
 				default:
 					panic(fmt.Sprintf("unexpected error handling duplicate tentative address: %s", err))
@@ -561,10 +565,24 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r
 		// 5, NDP messages cannot be fragmented. Also note that in the common case
 		// NDP datagrams are very small and AsView() will not incur allocations.
 		na := header.NDPNeighborAdvert(payload.AsView())
+
+		it, err := na.Options().Iter(false /* check */)
+		if err != nil {
+			// If we have a malformed NDP NA option, drop the packet.
+			received.invalid.Increment()
+			return
+		}
+
+		targetLinkAddr, ok := getTargetLinkAddr(it)
+		if !ok {
+			received.invalid.Increment()
+			return
+		}
+
 		targetAddr := na.TargetAddress()
 
 		e.dad.mu.Lock()
-		e.dad.mu.dad.StopLocked(targetAddr, &stack.DADDupAddrDetected{})
+		e.dad.mu.dad.StopLocked(targetAddr, &stack.DADDupAddrDetected{HolderLinkAddress: targetLinkAddr})
 		e.dad.mu.Unlock()
 
 		if e.hasTentativeAddr(targetAddr) {
@@ -584,7 +602,7 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r
 			//
 			// TODO(gvisor.dev/issue/4046): Handle the scenario when a duplicate
 			// address is detected for an assigned address.
-			switch err := e.dupTentativeAddrDetected(targetAddr); err.(type) {
+			switch err := e.dupTentativeAddrDetected(targetAddr, targetLinkAddr); err.(type) {
 			case nil, *tcpip.ErrBadAddress, *tcpip.ErrInvalidEndpointState:
 				return
 			default:
@@ -592,13 +610,6 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r
 			}
 		}
 
-		it, err := na.Options().Iter(false /* check */)
-		if err != nil {
-			// If we have a malformed NDP NA option, drop the packet.
-			received.invalid.Increment()
-			return
-		}
-
 		// At this point we know that the target address is not tentative on the
 		// NIC. However, the target address may still be assigned to the NIC but not
 		// tentative (it could be permanent). Such a scenario is beyond the scope of
@@ -608,11 +619,6 @@ func (e *endpoint) handleICMP(pkt *stack.PacketBuffer, hasFragmentHeader bool, r
 		// TODO(b/143147598): Handle the scenario described above. Also inform the
 		// netstack integration that a duplicate address was detected outside of
 		// DAD.
-		targetLinkAddr, ok := getTargetLinkAddr(it)
-		if !ok {
-			received.invalid.Increment()
-			return
-		}
 
 		// As per RFC 4861 section 7.1.2:
 		//   A node MUST silently discard any received Neighbor Advertisement
diff --git a/pkg/tcpip/network/ipv6/ipv6.go b/pkg/tcpip/network/ipv6/ipv6.go
index d9ab240b6..46b6cc41a 100644
--- a/pkg/tcpip/network/ipv6/ipv6.go
+++ b/pkg/tcpip/network/ipv6/ipv6.go
@@ -348,7 +348,7 @@ func (e *endpoint) hasTentativeAddr(addr tcpip.Address) bool {
 // dupTentativeAddrDetected removes the tentative address if it exists. If the
 // address was generated via SLAAC, an attempt is made to generate a new
 // address.
-func (e *endpoint) dupTentativeAddrDetected(addr tcpip.Address) tcpip.Error {
+func (e *endpoint) dupTentativeAddrDetected(addr tcpip.Address, holderLinkAddr tcpip.LinkAddress) tcpip.Error {
 	e.mu.Lock()
 	defer e.mu.Unlock()
 
@@ -363,7 +363,7 @@ func (e *endpoint) dupTentativeAddrDetected(addr tcpip.Address) tcpip.Error {
 
 	// If the address is a SLAAC address, do not invalidate its SLAAC prefix as an
 	// attempt will be made to generate a new address for it.
-	if err := e.removePermanentEndpointLocked(addressEndpoint, false /* allowSLAACInvalidation */, &stack.DADDupAddrDetected{}); err != nil {
+	if err := e.removePermanentEndpointLocked(addressEndpoint, false /* allowSLAACInvalidation */, &stack.DADDupAddrDetected{HolderLinkAddress: holderLinkAddr}); err != nil {
 		return err
 	}
 
diff --git a/pkg/tcpip/stack/registration.go b/pkg/tcpip/stack/registration.go
index 426342fd2..85f0f471a 100644
--- a/pkg/tcpip/stack/registration.go
+++ b/pkg/tcpip/stack/registration.go
@@ -884,7 +884,11 @@ func (*DADAborted) isDADResult() {}
 var _ DADResult = (*DADDupAddrDetected)(nil)
 
 // DADDupAddrDetected indicates DAD detected a duplicate address.
-type DADDupAddrDetected struct{}
+type DADDupAddrDetected struct {
+	// HolderLinkAddress is the link address of the node that holds the duplicate
+	// address.
+	HolderLinkAddress tcpip.LinkAddress
+}
 
 func (*DADDupAddrDetected) isDADResult() {}