From 1b77796f9af633a5267807c7f4ccb2a9e646ecce Mon Sep 17 00:00:00 2001 From: junyulai Date: Fri, 19 Oct 2018 21:14:30 +0800 Subject: [PATCH 1/4] Fix negative uid stats caused by 464xlat adjust when eBPF is on. When using xt_qtaguid to count per uid stats, NetworkStatsService needs to adjust the 464xlat traffic since iptables module would double count for ipv4 and ipv6 packet. But for eBPF, the per uid stats is collected in a different hook, so the adjustment on root uid would only be needed in tx direction. Bug: 112226716 Test: 1. Make ipv4 traffic in ipv6-only network and check data usage. 2. Make ipv4 traffic in a client which connect to ipv6-only hotspot. 3. runtest frameworks-net 4. cts-tradefed run cts -m CtsNetTestCases -t \ android.net.cts.TrafficStatsTest 5. cts-tradefed run cts -m CtsUsageStatsTestCases Change-Id: Ic9a84f5446eddc943c255d5f3b89dad171f53cac Merged-In: Ic9a84f5446eddc943c255d5f3b89dad171f53cac (cherry picked from commit c33ac0d43b594f6154accf03ae7e3fd34dedc79d) (cherry picked from commit eda8aa1f46d3bccb089fb5e8feea07d7fe6867a0) --- core/java/android/net/NetworkStats.java | 24 ++++-- .../internal/net/NetworkStatsFactory.java | 9 ++- .../server/net/NetworkStatsService.java | 6 +- .../java/android/net/NetworkStatsTest.java | 78 +++++++++++++------ 4 files changed, 82 insertions(+), 35 deletions(-) diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java index edf9bc1c6b390..4bcc2453689d4 100644 --- a/core/java/android/net/NetworkStats.java +++ b/core/java/android/net/NetworkStats.java @@ -776,14 +776,19 @@ public class NetworkStats implements Parcelable { * packet needs to be subtracted from the root UID on the base interface both for tx * and rx traffic (http://b/12249687, http:/b/33681750). * + * As for eBPF, the per uid stats is collected by different hook, the rx packets on base + * interface will not be counted. Thus, the adjustment on root uid is only needed in tx + * direction. + * *

This method will behave fine if {@code stackedIfaces} is an non-synchronized but add-only * {@code ConcurrentHashMap} * @param baseTraffic Traffic on the base interfaces. Will be mutated. * @param stackedTraffic Stats with traffic stacked on top of our ifaces. Will also be mutated. * @param stackedIfaces Mapping ipv6if -> ipv4if interface where traffic is counted on both. + * @param useBpfStats True if eBPF is in use. */ public static void apply464xlatAdjustments(NetworkStats baseTraffic, - NetworkStats stackedTraffic, Map stackedIfaces) { + NetworkStats stackedTraffic, Map stackedIfaces, boolean useBpfStats) { // Total 464xlat traffic to subtract from uid 0 on all base interfaces. // stackedIfaces may grow afterwards, but NetworkStats will just be resized automatically. final NetworkStats adjustments = new NetworkStats(0, stackedIfaces.size()); @@ -802,15 +807,20 @@ public class NetworkStats implements Parcelable { continue; } // Subtract any 464lat traffic seen for the root UID on the current base interface. + // However, for eBPF, the per uid stats is collected by different hook, the rx packets + // on base interface will not be counted. Thus, the adjustment on root uid is only + // needed in tx direction. adjust.iface = baseIface; - adjust.rxBytes = -(entry.rxBytes + entry.rxPackets * IPV4V6_HEADER_DELTA); + if (!useBpfStats) { + adjust.rxBytes = -(entry.rxBytes + entry.rxPackets * IPV4V6_HEADER_DELTA); + adjust.rxPackets = -entry.rxPackets; + } adjust.txBytes = -(entry.txBytes + entry.txPackets * IPV4V6_HEADER_DELTA); - adjust.rxPackets = -entry.rxPackets; adjust.txPackets = -entry.txPackets; adjustments.combineValues(adjust); - // For 464xlat traffic, xt_qtaguid only counts the bytes of the native IPv4 packet sent - // on the stacked interface with prefix "v4-" and drops the IPv6 header size after + // For 464xlat traffic, per uid stats only counts the bytes of the native IPv4 packet + // sent on the stacked interface with prefix "v4-" and drops the IPv6 header size after // unwrapping. To account correctly for on-the-wire traffic, add the 20 additional bytes // difference for all packets (http://b/12249687, http:/b/33681750). entry.rxBytes += entry.rxPackets * IPV4V6_HEADER_DELTA; @@ -829,8 +839,8 @@ public class NetworkStats implements Parcelable { * base and stacked traffic. * @param stackedIfaces Mapping ipv6if -> ipv4if interface where traffic is counted on both. */ - public void apply464xlatAdjustments(Map stackedIfaces) { - apply464xlatAdjustments(this, this, stackedIfaces); + public void apply464xlatAdjustments(Map stackedIfaces, boolean useBpfStats) { + apply464xlatAdjustments(this, this, stackedIfaces, useBpfStats); } /** diff --git a/core/java/com/android/internal/net/NetworkStatsFactory.java b/core/java/com/android/internal/net/NetworkStatsFactory.java index c4d08c7dc5d60..b3cf389c1bb94 100644 --- a/core/java/com/android/internal/net/NetworkStatsFactory.java +++ b/core/java/com/android/internal/net/NetworkStatsFactory.java @@ -113,11 +113,12 @@ public class NetworkStatsFactory { /** * Applies 464xlat adjustments with ifaces noted with {@link #noteStackedIface(String, String)}. - * @see NetworkStats#apply464xlatAdjustments(NetworkStats, NetworkStats, Map) + * @see NetworkStats#apply464xlatAdjustments(NetworkStats, NetworkStats, Map, boolean) */ public static void apply464xlatAdjustments(NetworkStats baseTraffic, - NetworkStats stackedTraffic) { - NetworkStats.apply464xlatAdjustments(baseTraffic, stackedTraffic, sStackedIfaces); + NetworkStats stackedTraffic, boolean useBpfStats) { + NetworkStats.apply464xlatAdjustments(baseTraffic, stackedTraffic, sStackedIfaces, + useBpfStats); } @VisibleForTesting @@ -263,7 +264,7 @@ public class NetworkStatsFactory { // No locking here: apply464xlatAdjustments behaves fine with an add-only ConcurrentHashMap. // TODO: remove this and only apply adjustments in NetworkStatsService. - stats.apply464xlatAdjustments(sStackedIfaces); + stats.apply464xlatAdjustments(sStackedIfaces, mUseBpfStats); return stats; } diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index 9bf3874088d7c..17ab29d747905 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -1622,7 +1622,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { // fold tethering stats and operations into uid snapshot final NetworkStats tetherSnapshot = getNetworkStatsTethering(STATS_PER_UID); tetherSnapshot.filter(UID_ALL, ifaces, TAG_ALL); - NetworkStatsFactory.apply464xlatAdjustments(uidSnapshot, tetherSnapshot); + NetworkStatsFactory.apply464xlatAdjustments(uidSnapshot, tetherSnapshot, + mUseBpfTrafficStats); uidSnapshot.combineAllValues(tetherSnapshot); final TelephonyManager telephonyManager = (TelephonyManager) mContext.getSystemService( @@ -1632,7 +1633,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { final NetworkStats vtStats = telephonyManager.getVtDataUsage(STATS_PER_UID); if (vtStats != null) { vtStats.filter(UID_ALL, ifaces, TAG_ALL); - NetworkStatsFactory.apply464xlatAdjustments(uidSnapshot, vtStats); + NetworkStatsFactory.apply464xlatAdjustments(uidSnapshot, vtStats, + mUseBpfTrafficStats); uidSnapshot.combineAllValues(vtStats); } diff --git a/tests/net/java/android/net/NetworkStatsTest.java b/tests/net/java/android/net/NetworkStatsTest.java index 8f18d072a0bbb..d6dbf5aaa9d88 100644 --- a/tests/net/java/android/net/NetworkStatsTest.java +++ b/tests/net/java/android/net/NetworkStatsTest.java @@ -39,6 +39,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import android.net.NetworkStats.Entry; import android.os.Process; import android.support.test.runner.AndroidJUnit4; import android.support.test.filters.SmallTest; @@ -785,7 +786,38 @@ public class NetworkStatsTest { ArrayMap stackedIface = new ArrayMap<>(); stackedIface.put(v4Iface, baseIface); - NetworkStats.Entry otherEntry = new NetworkStats.Entry( + // Ipv4 traffic sent/received by an app on stacked interface. + final NetworkStats.Entry appEntry = new NetworkStats.Entry( + v4Iface, appUid, SET_DEFAULT, TAG_NONE, + 30501490 /* rxBytes */, + 22401 /* rxPackets */, + 876235 /* txBytes */, + 13805 /* txPackets */, + 0 /* operations */); + + // Traffic measured for the root uid on the base interface if eBPF is in use. + // Incorrectly includes appEntry's bytes and packets, plus IPv4-IPv6 translation + // overhead (20 bytes per packet), only for TX traffic. + final NetworkStats.Entry ebpfRootUidEntry = new NetworkStats.Entry( + baseIface, rootUid, SET_DEFAULT, TAG_NONE, + 163577 /* rxBytes */, + 187 /* rxPackets */, + 1169942 /* txBytes */, + 13902 /* txPackets */, + 0 /* operations */); + + // Traffic measured for the root uid on the base interface if xt_qtaguid is in use. + // Incorrectly includes appEntry's bytes and packets, plus IPv4-IPv6 translation + // overhead (20 bytes per packet), in both directions. + final NetworkStats.Entry xtRootUidEntry = new NetworkStats.Entry( + baseIface, rootUid, SET_DEFAULT, TAG_NONE, + 31113087 /* rxBytes */, + 22588 /* rxPackets */, + 1169942 /* txBytes */, + 13902 /* txPackets */, + 0 /* operations */); + + final NetworkStats.Entry otherEntry = new NetworkStats.Entry( otherIface, appUid, SET_DEFAULT, TAG_NONE, 2600 /* rxBytes */, 2 /* rxPackets */, @@ -793,39 +825,41 @@ public class NetworkStatsTest { 3 /* txPackets */, 0 /* operations */); - NetworkStats stats = new NetworkStats(TEST_START, 3) - .addValues(v4Iface, appUid, SET_DEFAULT, TAG_NONE, - 30501490 /* rxBytes */, - 22401 /* rxPackets */, - 876235 /* txBytes */, - 13805 /* txPackets */, - 0 /* operations */) - .addValues(baseIface, rootUid, SET_DEFAULT, TAG_NONE, - 31113087, - 22588, - 1169942, - 13902, - 0) + final NetworkStats statsXt = new NetworkStats(TEST_START, 3) + .addValues(appEntry) + .addValues(xtRootUidEntry) .addValues(otherEntry); - stats.apply464xlatAdjustments(stackedIface); + final NetworkStats statsEbpf = new NetworkStats(TEST_START, 3) + .addValues(appEntry) + .addValues(ebpfRootUidEntry) + .addValues(otherEntry); - assertEquals(3, stats.size()); - assertValues(stats, 0, v4Iface, appUid, SET_DEFAULT, TAG_NONE, - METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, + statsXt.apply464xlatAdjustments(stackedIface, false); + statsEbpf.apply464xlatAdjustments(stackedIface, true); + + assertEquals(3, statsXt.size()); + assertEquals(3, statsEbpf.size()); + final NetworkStats.Entry expectedAppUid = new NetworkStats.Entry( + v4Iface, appUid, SET_DEFAULT, TAG_NONE, 30949510, 22401, 1152335, 13805, 0); - assertValues(stats, 1, baseIface, 0, SET_DEFAULT, TAG_NONE, - METERED_NO, ROAMING_NO, DEFAULT_NETWORK_NO, + final NetworkStats.Entry expectedRootUid = new NetworkStats.Entry( + baseIface, 0, SET_DEFAULT, TAG_NONE, 163577, 187, 17607, 97, 0); - assertEquals(otherEntry, stats.getValues(2, null)); + assertEquals(expectedAppUid, statsXt.getValues(0, null)); + assertEquals(expectedRootUid, statsXt.getValues(1, null)); + assertEquals(otherEntry, statsXt.getValues(2, null)); + assertEquals(expectedAppUid, statsEbpf.getValues(0, null)); + assertEquals(expectedRootUid, statsEbpf.getValues(1, null)); + assertEquals(otherEntry, statsEbpf.getValues(2, null)); } @Test @@ -850,7 +884,7 @@ public class NetworkStatsTest { .addValues(secondEntry); // Empty map: no adjustment - stats.apply464xlatAdjustments(new ArrayMap<>()); + stats.apply464xlatAdjustments(new ArrayMap<>(), false); assertEquals(2, stats.size()); assertEquals(firstEntry, stats.getValues(0, null)); From 77d3f399256783faee7b55a51ac9a28031ed0d78 Mon Sep 17 00:00:00 2001 From: junyulai Date: Thu, 8 Nov 2018 22:33:09 +0800 Subject: [PATCH 2/4] Change types of fields of network stats reported to framework. Currently, NetworkStats use int to handle uid, set and tag, while native side using unsigned int mixing with signed int with that. This commit make necessary changes in JNI part while libnetdbpf unifying the types of fields. Bug: 112226716 Bug: 119193941 Test: 1. manually reconnect vpn 2. update apps from playstore 3. atest libnetdbpf_test 4. runtest frameworks-net 5. cts-tradefed run cts -m CtsUsageStatsTestCases -t \ android.app.usage.cts.NetworkUsageStatsTest Change-Id: I6c27124db8292e2825fba51b8994f013897cb566 Merged-In: I6c27124db8292e2825fba51b8994f013897cb566 (cherry picked from commit bf7803eed8283d5a1caa2efe90aa449590ddb94f) (cherry picked from commit f5bc6f61b41f62d572bbe5a66797b1f5a8103676) --- core/jni/com_android_internal_net_NetworkStatsFactory.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/jni/com_android_internal_net_NetworkStatsFactory.cpp b/core/jni/com_android_internal_net_NetworkStatsFactory.cpp index c15b7ee4fe04c..de3a78a61a7e8 100644 --- a/core/jni/com_android_internal_net_NetworkStatsFactory.cpp +++ b/core/jni/com_android_internal_net_NetworkStatsFactory.cpp @@ -175,7 +175,7 @@ static int legacyReadNetworkStatsDetail(std::vector* lines, } } s.tag = rawTag >> 32; - if (limitTag != -1 && s.tag != limitTag) { + if (limitTag != -1 && s.tag != static_cast(limitTag)) { //ALOGI("skipping due to tag: %s", buffer); continue; } @@ -188,7 +188,7 @@ static int legacyReadNetworkStatsDetail(std::vector* lines, if (sscanf(pos, "%u %u %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64, &s.uid, &s.set, &s.rxBytes, &s.rxPackets, &s.txBytes, &s.txPackets) == 6) { - if (limitUid != -1 && limitUid != s.uid) { + if (limitUid != -1 && static_cast(limitUid) != s.uid) { //ALOGI("skipping due to uid: %s", buffer); continue; } From 5239804b28b3e49fdea54fc73e431f262c1cf727 Mon Sep 17 00:00:00 2001 From: Varun Shah Date: Fri, 26 Oct 2018 17:03:23 -0700 Subject: [PATCH 3/4] RESTRICT AUTOMERGE: Added an app id security check in isAppForeground. ActivityManagerService#isAppForeground now checks if the caller has the permission to view if an app is in the foreground. Bug: 115384617 Test: cts-tradefed run cts -m CtsSecurityTestCases -t android.security.cts.ActivityManagerTest#testIsAppInForegroundNormal Test: cts-tradefed run cts -m CtsSecurityTestCases -t android.security.cts.ActivityManagerTest#testIsAppInForegroundMalicious Change-Id: I9602c89b2d40036e525c38960a08326dc74c6682 (cherry picked from commit 273eb0918adbc7cd410a583b0d25c518b9b1e35e) --- .../com/android/server/am/ActivityManagerService.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 10707b255c293..a1d42c0913342 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -8768,6 +8768,14 @@ public class ActivityManagerService extends IActivityManager.Stub @Override public boolean isAppForeground(int uid) { + int callerUid = Binder.getCallingUid(); + if (UserHandle.isCore(callerUid) || callerUid == uid) { + return isAppForegroundInternal(uid); + } + return false; + } + + private boolean isAppForegroundInternal(int uid) { synchronized (this) { UidRecord uidRec = mActiveUids.get(uid); if (uidRec == null || uidRec.idle) { From b5dee1bafd5648fd9210b4cba5e23b0665f9add5 Mon Sep 17 00:00:00 2001 From: Jakub Pawlowski Date: Thu, 29 Nov 2018 18:54:21 +0100 Subject: [PATCH 4/4] Bluetooth: Check descriptors size in BluetoothHidDeviceAppSdpSettings Bug: 119819889 Test: compilation Change-Id: If51d0e2af74d99758f79a603d40cc2f5c84e4dde (cherry picked from commit 63519217e8c0682369b5b3dc11417f79b3b48b7e) --- .../bluetooth/BluetoothHidDeviceAppSdpSettings.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/core/java/android/bluetooth/BluetoothHidDeviceAppSdpSettings.java b/core/java/android/bluetooth/BluetoothHidDeviceAppSdpSettings.java index 237082e4cb664..2f0b44f76ffb3 100644 --- a/core/java/android/bluetooth/BluetoothHidDeviceAppSdpSettings.java +++ b/core/java/android/bluetooth/BluetoothHidDeviceAppSdpSettings.java @@ -18,6 +18,7 @@ package android.bluetooth; import android.os.Parcel; import android.os.Parcelable; +import android.util.EventLog; /** @@ -30,6 +31,8 @@ import android.os.Parcelable; */ public final class BluetoothHidDeviceAppSdpSettings implements Parcelable { + private static final int MAX_DESCRIPTOR_SIZE = 2048; + private final String mName; private final String mDescription; private final String mProvider; @@ -55,6 +58,12 @@ public final class BluetoothHidDeviceAppSdpSettings implements Parcelable { mDescription = description; mProvider = provider; mSubclass = subclass; + + if (descriptors == null || descriptors.length > MAX_DESCRIPTOR_SIZE) { + EventLog.writeEvent(0x534e4554, "119819889", -1, ""); + throw new IllegalArgumentException("descriptors must be not null and shorter than " + + MAX_DESCRIPTOR_SIZE); + } mDescriptors = descriptors.clone(); }