Don't crash when receiving an RTM_DELNEIGH or NUD_FAILED.
These events don't have MAC addresses, so the code attempts to
create an Ipv6ForwardingRule with a null MAC address. This
crashes when attempting to get the raw MAC address bytes to send
to netd in the TetherOffloadRuleParcel.
This was not caught by unit tests because the test exercise this
code path in a way that is not correct (by sending RTM_DELNEIGH
and NUD_FAILED events with MAC addresses). Fix the unit tests to
properly pass in null MAC addresses for these events.
Bug: 153697068
Test: fixed existing tests to be more realistic
Change-Id: I26d89a81f1c448d9b4809652b079a5f5eace3924
diff --git a/Tethering/src/android/net/ip/IpServer.java b/Tethering/src/android/net/ip/IpServer.java
index 1dac5b7..83727bc 100644
--- a/Tethering/src/android/net/ip/IpServer.java
+++ b/Tethering/src/android/net/ip/IpServer.java
@@ -122,6 +122,8 @@
// TODO: have this configurable
private static final int DHCP_LEASE_TIME_SECS = 3600;
+ private static final MacAddress NULL_MAC_ADDRESS = MacAddress.fromString("00:00:00:00:00:00");
+
private static final String TAG = "IpServer";
private static final boolean DBG = false;
private static final boolean VDBG = false;
@@ -902,9 +904,12 @@
return;
}
+ // When deleting rules, we still need to pass a non-null MAC, even though it's ignored.
+ // Do this here instead of in the Ipv6ForwardingRule constructor to ensure that we never
+ // add rules with a null MAC, only delete them.
+ MacAddress dstMac = e.isValid() ? e.macAddr : NULL_MAC_ADDRESS;
Ipv6ForwardingRule rule = new Ipv6ForwardingRule(upstreamIfindex,
- mInterfaceParams.index, (Inet6Address) e.ip, mInterfaceParams.macAddr,
- e.macAddr);
+ mInterfaceParams.index, (Inet6Address) e.ip, mInterfaceParams.macAddr, dstMac);
if (e.isValid()) {
addIpv6ForwardingRule(rule);
} else {
diff --git a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
index fdfdae8..f9be7b9 100644
--- a/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
+++ b/Tethering/tests/unit/src/android/net/ip/IpServerTest.java
@@ -587,6 +587,7 @@
final InetAddress neighB = InetAddresses.parseNumericAddress("2001:db8::2");
final InetAddress neighLL = InetAddresses.parseNumericAddress("fe80::1");
final InetAddress neighMC = InetAddresses.parseNumericAddress("ff02::1234");
+ final MacAddress macNull = MacAddress.fromString("00:00:00:00:00:00");
final MacAddress macA = MacAddress.fromString("00:00:00:00:00:0a");
final MacAddress macB = MacAddress.fromString("11:22:33:00:00:0b");
@@ -612,13 +613,14 @@
verifyNoMoreInteractions(mNetd);
// A neighbor that is no longer valid causes the rule to be removed.
- recvNewNeigh(myIfindex, neighA, NUD_FAILED, macA);
- verify(mNetd).tetherOffloadRuleRemove(matches(UPSTREAM_IFINDEX, neighA, macA));
+ // NUD_FAILED events do not have a MAC address.
+ recvNewNeigh(myIfindex, neighA, NUD_FAILED, null);
+ verify(mNetd).tetherOffloadRuleRemove(matches(UPSTREAM_IFINDEX, neighA, macNull));
reset(mNetd);
// A neighbor that is deleted causes the rule to be removed.
recvDelNeigh(myIfindex, neighB, NUD_STALE, macB);
- verify(mNetd).tetherOffloadRuleRemove(matches(UPSTREAM_IFINDEX, neighB, macB));
+ verify(mNetd).tetherOffloadRuleRemove(matches(UPSTREAM_IFINDEX, neighB, macNull));
reset(mNetd);
// Upstream changes result in deleting and re-adding the rules.