From c81837495dab95295f81b8d6492df3231612e369 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Thu, 13 Oct 2016 09:26:01 +0900 Subject: [PATCH 1/2] DO NOT MERGE ApfFilter: systematically use u8, u16, u32 getters This patch adds a getUint8 getter for ByteBuffers and changes ApfFilter to make uses of getUint8/16/32 everywhere. The return types of getUint16 is also changed from long to int, which will expand gracefully to long as an unsigned int as it is guaranteed to be positive after getUint16. Test: ApfTest passes (cherry picked from commit 995dd94673005b43d32456e2de5fda0090b23576) Change-Id: Idde3c9d03d39fbdf6f9b84d398f3fe8ea371483d --- .../net/java/android/net/apf/ApfFilter.java | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/services/net/java/android/net/apf/ApfFilter.java b/services/net/java/android/net/apf/ApfFilter.java index b9eae18240c32..395e73d5cc187 100644 --- a/services/net/java/android/net/apf/ApfFilter.java +++ b/services/net/java/android/net/apf/ApfFilter.java @@ -381,16 +381,16 @@ public class ApfFilter { // TODO: Make this static once RA is its own class. private void prefixOptionToString(StringBuffer sb, int offset) { String prefix = IPv6AddresstoString(offset + 16); - int length = uint8(mPacket.get(offset + 2)); - long valid = mPacket.getInt(offset + 4); - long preferred = mPacket.getInt(offset + 8); + int length = getUint8(mPacket, offset + 2); + long valid = getUint32(mPacket, offset + 4); + long preferred = getUint32(mPacket, offset + 8); sb.append(String.format("%s/%d %ds/%ds ", prefix, length, valid, preferred)); } private void rdnssOptionToString(StringBuffer sb, int offset) { - int optLen = uint8(mPacket.get(offset + 1)) * 8; + int optLen = getUint8(mPacket, offset + 1) * 8; if (optLen < 24) return; // Malformed or empty. - long lifetime = uint32(mPacket.getInt(offset + 4)); + long lifetime = getUint32(mPacket, offset + 4); int numServers = (optLen - 8) / 16; sb.append("DNS ").append(lifetime).append("s"); for (int server = 0; server < numServers; server++) { @@ -404,7 +404,7 @@ public class ApfFilter { sb.append(String.format("RA %s -> %s %ds ", IPv6AddresstoString(IPV6_SRC_ADDR_OFFSET), IPv6AddresstoString(IPV6_DEST_ADDR_OFFSET), - uint16(mPacket.getShort(ICMP6_RA_ROUTER_LIFETIME_OFFSET)))); + getUint16(mPacket, ICMP6_RA_ROUTER_LIFETIME_OFFSET))); for (int i: mPrefixOptionOffsets) { prefixOptionToString(sb, i); } @@ -456,8 +456,8 @@ public class ApfFilter { // Sanity check packet in case a packet arrives before we attach RA filter // to our packet socket. b/29586253 if (getUint16(mPacket, ETH_ETHERTYPE_OFFSET) != ETH_P_IPV6 || - uint8(mPacket.get(IPV6_NEXT_HEADER_OFFSET)) != IPPROTO_ICMPV6 || - uint8(mPacket.get(ICMP6_TYPE_OFFSET)) != ICMP6_ROUTER_ADVERTISEMENT) { + getUint8(mPacket, IPV6_NEXT_HEADER_OFFSET) != IPPROTO_ICMPV6 || + getUint8(mPacket, ICMP6_TYPE_OFFSET) != ICMP6_ROUTER_ADVERTISEMENT) { throw new InvalidRaException("Not an ICMP6 router advertisement"); } @@ -479,8 +479,8 @@ public class ApfFilter { mPacket.position(ICMP6_RA_OPTION_OFFSET); while (mPacket.hasRemaining()) { final int position = mPacket.position(); - final int optionType = uint8(mPacket.get(position)); - final int optionLength = uint8(mPacket.get(position + 1)) * 8; + final int optionType = getUint8(mPacket, position); + final int optionLength = getUint8(mPacket, position + 1) * 8; long lifetime; switch (optionType) { case ICMP6_PREFIX_OPTION_TYPE: @@ -565,10 +565,10 @@ public class ApfFilter { final long optionLifetime; switch (lifetimeLength) { case 2: - optionLifetime = uint16(byteBuffer.getShort(offset)); + optionLifetime = getUint16(byteBuffer, offset); break; case 4: - optionLifetime = uint32(byteBuffer.getInt(offset)); + optionLifetime = getUint32(byteBuffer, offset); break; default: throw new IllegalStateException("bogus lifetime size " + lifetimeLength); @@ -1169,7 +1169,11 @@ public class ApfFilter { return i & 0xffffffffL; } - private static long getUint16(ByteBuffer buffer, int position) { + private static int getUint8(ByteBuffer buffer, int position) { + return uint8(buffer.get(position)); + } + + private static int getUint16(ByteBuffer buffer, int position) { return uint16(buffer.getShort(position)); } From 4c0b7cc77620f139b4d05d2e7568dd79e651a871 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Mon, 17 Oct 2016 14:21:33 +0900 Subject: [PATCH 2/2] DO NOT MERGE ApfFilter: use elapsedRealTime for RA lifetime This patch replaces System.currentTimeMillis() with SystemClock.elapsedRealTime() to make RA lifetime computation more resilient to various external events inducing jumps in currentTimeMillis(). Test: ApfTest passes. (cherry picked from commit 305af8e98a4fce712c1a93daf3b050dac2e8b91a) Change-Id: Idbde700025fecfecefb8162d66b94194a87829d5 --- services/net/java/android/net/apf/ApfFilter.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/services/net/java/android/net/apf/ApfFilter.java b/services/net/java/android/net/apf/ApfFilter.java index 395e73d5cc187..83001df580233 100644 --- a/services/net/java/android/net/apf/ApfFilter.java +++ b/services/net/java/android/net/apf/ApfFilter.java @@ -285,10 +285,9 @@ public class ApfFilter { mReceiveThread.start(); } - // Returns seconds since Unix Epoch. - // TODO: use SystemClock.elapsedRealtime() instead + // Returns seconds since device boot. private static long curTime() { - return System.currentTimeMillis() / DateUtils.SECOND_IN_MILLIS; + return SystemClock.elapsedRealtime() / DateUtils.SECOND_IN_MILLIS; } public static class InvalidRaException extends Exception {