From 138e10e3ba38d5def03e6c314b2f714f7a1f490c Mon Sep 17 00:00:00 2001 From: Kangping Dong Date: Fri, 27 Jun 2025 14:19:08 +0800 Subject: [PATCH 01/21] [Thread] add REDACT_FOR_THREAD_NETWORK_PRIVILEGED This is used for redacting sensitive Thread info in ThreadNetworkSpecifier (to be added in ag/34224158) Bug: 374037595 Flag: EXEMPT new code is not used Change-Id: I606d59fc5259c79bddaa51ca0c270684dd3b674a --- .../src/android/net/NetworkCapabilities.java | 10 ++++++ .../android/server/ConnectivityService.java | 31 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java index e8b4642429..9b099e42b8 100644 --- a/framework/src/android/net/NetworkCapabilities.java +++ b/framework/src/android/net/NetworkCapabilities.java @@ -169,6 +169,15 @@ public final class NetworkCapabilities implements Parcelable { @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) public static final long REDACT_FOR_NETWORK_SETTINGS = 1 << 2; + /** + * Redact any fields that need {@link android.Manifest.permission#THREAD_NETWORK_PRIVILEGED} + * permission since the receiving app does not hold this permission. + * + * @see android.Manifest.permission#THREAD_NETWORK_PRIVILEGED + * @hide for only {@code ThreadNetworkSpecifier} which is in the same module + */ + public static final long REDACT_FOR_THREAD_NETWORK_PRIVILEGED = 1 << 3; + /** * Redact all fields in this object that require any relevant permission. * @hide @@ -182,6 +191,7 @@ public final class NetworkCapabilities implements Parcelable { REDACT_FOR_ACCESS_FINE_LOCATION, REDACT_FOR_LOCAL_MAC_ADDRESS, REDACT_FOR_NETWORK_SETTINGS, + REDACT_FOR_THREAD_NETWORK_PRIVILEGED, REDACT_ALL }) @Retention(RetentionPolicy.SOURCE) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index cfceca3c65..20026ccafb 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -109,6 +109,7 @@ import static android.net.NetworkCapabilities.NET_ENTERPRISE_ID_5; import static android.net.NetworkCapabilities.REDACT_FOR_ACCESS_FINE_LOCATION; import static android.net.NetworkCapabilities.REDACT_FOR_LOCAL_MAC_ADDRESS; import static android.net.NetworkCapabilities.REDACT_FOR_NETWORK_SETTINGS; +import static android.net.NetworkCapabilities.REDACT_FOR_THREAD_NETWORK_PRIVILEGED; import static android.net.NetworkCapabilities.RES_ID_MATCH_ALL_RESERVATIONS; import static android.net.NetworkCapabilities.RES_ID_UNSET; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; @@ -3208,6 +3209,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private Boolean mHasLocationPermission = null; private Boolean mHasLocalMacAddressPermission = null; private Boolean mHasSettingsPermission = null; + private Boolean mHasThreadNetworkPrivilegedPermission = null; RedactionPermissionChecker(int callingPid, int callingUid, @NonNull String callingPackageName, @Nullable String callingAttributionTag) { @@ -3267,6 +3269,20 @@ public class ConnectivityService extends IConnectivityManager.Stub } return mHasSettingsPermission; } + + /** + * Returns whether the app holds THREAD_NETWORK_PRIVILEGED permission or not (might return + * cached result if the permission was already checked before). + */ + @CheckResult + private boolean hasThreadNetworkPrivilegedPermission() { + if (mHasThreadNetworkPrivilegedPermission == null) { + // If there is no cached result, perform the check now. + mHasThreadNetworkPrivilegedPermission = ConnectivityService.this + .hasThreadNetworkPrivilegedPermission(mCallingPid, mCallingUid); + } + return mHasThreadNetworkPrivilegedPermission; + } } private static boolean shouldRedact(@NetworkCapabilities.RedactionType long redactions, @@ -3301,6 +3317,11 @@ public class ConnectivityService extends IConnectivityManager.Stub redactions &= ~REDACT_FOR_NETWORK_SETTINGS; } } + if (shouldRedact(redactions, REDACT_FOR_THREAD_NETWORK_PRIVILEGED)) { + if (redactionPermissionChecker.hasThreadNetworkPrivilegedPermission()) { + redactions &= ~REDACT_FOR_THREAD_NETWORK_PRIVILEGED; + } + } return redactions; } @@ -4269,6 +4290,16 @@ public class ConnectivityService extends IConnectivityManager.Stub Manifest.permission.LOCAL_MAC_ADDRESS, pid, uid); } + @CheckResult + private boolean hasThreadNetworkPrivilegedPermission(int pid, int uid) { + // The THREAD_NETWORK_PRIVILEGED permission was added in Android V + if (!SdkLevel.isAtLeastV()) { + return false; + } + return PERMISSION_GRANTED == mContext.checkPermission( + Manifest.permission.THREAD_NETWORK_PRIVILEGED, pid, uid); + } + private void sendConnectedBroadcast(NetworkInfo info) { sendGeneralBroadcast(info, CONNECTIVITY_ACTION); } From 89c94195a00dabee8e33533aeea27f095f27ce04 Mon Sep 17 00:00:00 2001 From: Kangping Dong Date: Mon, 30 Jun 2025 14:31:14 +0800 Subject: [PATCH 02/21] [connectivity] add NetworkCapabilities.RedactionHelper Add a new hidden inner class NetworkCapabilities.RedactionHelper for accessing conditionally available NetworkSpecifier methods via reflection. Reflection is necessary because NetworkSpecifier is platform code. So it's possible for a mainline update to define a getApplicableRedactions() or redact(long) method in a subclass of NetworkSpecifier (e.g., EthernetNetworkSpecifier), and then to run that mainline module on an Android release where the NetworkSpecifier base class does not have those methods. On such a device, ConnectivityService must still call those methods if they exist, otherwise it would fail to redact information that needs to be redacted. Another way to do this would be for ConnectivityService to just call the method and catch NoSuchMethodError, but this code runs fairly often and constructing exceptions is expensive. This class is not used in this CL but will be used for NetworkSpecifier redaction in ag/34242202 and ag/34242203 Bug 374037595 Flag: NONE new code nerver used Change-Id: I8f88c6bb105ebf8d485e18cd214bebf2abe67e63 --- .../src/android/net/NetworkCapabilities.java | 151 ++++++++++++++++++ .../android/net/NetworkCapabilitiesTest.java | 78 +++++++++ 2 files changed, 229 insertions(+) diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java index 9b099e42b8..5974b58c9e 100644 --- a/framework/src/android/net/NetworkCapabilities.java +++ b/framework/src/android/net/NetworkCapabilities.java @@ -19,6 +19,8 @@ package android.net; import static com.android.internal.annotations.VisibleForTesting.Visibility.PRIVATE; import static com.android.net.module.util.BitUtils.appendStringRepresentationOfBitMaskToStringBuilder; import static com.android.net.module.util.BitUtils.describeDifferences; +import static com.android.net.module.util.FrameworkConnectivityStatsLog.CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED; +import static com.android.net.module.util.FrameworkConnectivityStatsLog.CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_SPECIFIER_REDACTION_REFLECTION_ERROR; import android.annotation.FlaggedApi; import android.annotation.IntDef; @@ -37,6 +39,7 @@ import android.os.Process; import android.text.TextUtils; import android.util.ArraySet; import android.util.Log; +import android.util.LruCache; import android.util.Range; import com.android.internal.annotations.VisibleForTesting; @@ -44,15 +47,19 @@ import com.android.modules.utils.build.SdkLevel; import com.android.net.flags.Flags; import com.android.net.module.util.BitUtils; import com.android.net.module.util.CollectionUtils; +import com.android.net.module.util.FrameworkConnectivityStatsLog; import com.android.net.module.util.NetworkCapabilitiesUtils; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.StringJoiner; @@ -3519,4 +3526,148 @@ public final class NetworkCapabilities implements Parcelable { return new NetworkCapabilities(mCaps); } } + + /** + * Helper class for providing access to {@link NetworkSpecifier} redaction methods via + * reflection. + * + * Reflection is necessary because NetworkSpecifier is platform code. So it's possible for a + * mainline update to define a getApplicableRedactions() or redact(long) method in a subclass + * of NetworkSpecifier (e.g., EthernetNetworkSpecifier), and then to run that mainline module + * on an Android release where the NetworkSpecifier base class does not have those methods. + * On such a device, ConnectivityService must still call those methods if they exist, otherwise + * it would fail to redact information that needs to be redacted. Another way to do this would + * be for ConnectivityService to just call the method and catch NoSuchMethodError, but this + * code runs fairly often and constructing exceptions is expensive. + * + * @hide + */ + public static final class RedactionHelper { + // LruCache is thread-safe so that no locks are used to project it this class + private static final LruCache, Optional> + sGetApplicableRedactionsMethodCache = new LruCache<>(100); + private static final LruCache, Optional> + sRedactLongMethodCache = new LruCache<>(100); + + private RedactionHelper() {} + + /** + * Returns the {@link Method} of the {@link NetworkSpecifier} class for the given method + * name, or {@code null} if the method doesn't exist. + *

+ * This is typically used for {@link NetworkSpecifier#getApplicableRedactions()} and {@link + * NetworkSpecifier#redact(long)} to work with new NetworkSpecifier subclasses added in new + * mainline version but deployed to older platforms which don't have those two + * NetworkSpecifier methods. + *

+ * See {@link #createWithLocationInfoSanitizedIfNecessaryWhenParceled} and {@link + * #ensureSufficientPermissionsForRequest} for how this method should be used. + */ + @Nullable + private static Method getNetworkSpecifierMethod( + Class clazz, String name, Class returnType, + Class... parameterTypes) { + // Use {@code getMethods()} rather than {@code getDeclaredMethods()} so that it can + // return the inherited NetworkSpecifier methods for those existing subclasses which + // don't override {@link NetworkSpecifier#getApplicableRedactions()} and {@link + // NetworkSpecifier#redact(long)}. + // Note also {@code getMethods()} returns only public methods, which is necessary for + // code to call the method later. + for (Method method : clazz.getMethods()) { + if (name.equals(method.getName()) + && returnType.equals(method.getReturnType()) + && Arrays.equals(parameterTypes, method.getParameterTypes())) { + return method; + } + } + return null; + } + + /** + * Returns the {@link NetworkSpecifier#getApplicableRedactions()} method or {@code null} if + * it doesn't exist for the given {@link NetworkSpecifier} subclass. + */ + @Nullable + private static Method getApplicableRedactionsMethod( + Class clazz) { + // If this method is called concurrently from two threads, both may see cache missed + // and the reflection branch in method will be executed for twice. But this is okay + // because getApplicableRedactions() itself has no side-effect (and this is why we can + // create caches for it). + Optional cache = sGetApplicableRedactionsMethodCache.get(clazz); + if (cache != null) { + return cache.orElse(null); + } + Method method = getNetworkSpecifierMethod( + clazz, "getApplicableRedactions", long.class); + sGetApplicableRedactionsMethodCache.put(clazz, Optional.ofNullable(method)); + return method; + } + + /** + * Returns the value of {@link NetworkSpecifier#getApplicableRedactions()} or + * {@link REDACT_NONE} if the platform doesn't support + * {@link NetworkSpecifier#getApplicableRedactions()}. + */ + public static long getSpecifierApplicableRedactions(NetworkSpecifier specifier) { + final Method getApplicableRedactions = + getApplicableRedactionsMethod(specifier.getClass()); + if (getApplicableRedactions == null) { + return REDACT_NONE; + } + + try { + return (long) getApplicableRedactions.invoke(specifier); + } catch (IllegalAccessException | InvocationTargetException + | IllegalArgumentException e) { + FrameworkConnectivityStatsLog.write(CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED, + CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_SPECIFIER_REDACTION_REFLECTION_ERROR); + Log.wtf(TAG, "Un expected reflection error on specifier " + specifier, e); + return REDACT_ALL; + } + } + + /** + * Returns the {@link NetworkSpecifier#redact(long)} method or {@code null} if it doesn't + * exist for the given {@link NetworkSpecifier} subclass. + */ + @Nullable + private static Method getRedactLongMethod(Class clazz) { + // If this method is called concurrently from two threads, both may see cache missed + // and the reflection branch in method will be executed for twice. But this is okay + // because redact(long) itself has no side-effect (and this is why we can create caches + // for it). + Optional cache = sRedactLongMethodCache.get(clazz); + if (cache != null) { + return cache.orElse(null); + } + Method method = getNetworkSpecifierMethod( + clazz, "redact", NetworkSpecifier.class, long.class); + sRedactLongMethodCache.put(clazz, Optional.ofNullable(method)); + return method; + } + + /** + * Returns the result of {@link NetworkSpecifier#redact(long)} or {@code specifier} itself + * if the platform doesn't support {@link NetworkSpecifier#redact(long)}. + */ + @Nullable + public static NetworkSpecifier getSpecifierRedactResult( + NetworkSpecifier specifier, long redactions) { + final Method redactLong = getRedactLongMethod(specifier.getClass()); + if (redactLong == null) { + return specifier; + } + + try { + return (NetworkSpecifier) redactLong.invoke(specifier, redactions); + } catch (IllegalAccessException | InvocationTargetException + | IllegalArgumentException e) { + FrameworkConnectivityStatsLog.write(CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED, + CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_SPECIFIER_REDACTION_REFLECTION_ERROR); + Log.wtf(TAG, "Un expected reflection error on specifier " + specifier, e); + return null; + } + } + } } diff --git a/tests/common/java/android/net/NetworkCapabilitiesTest.java b/tests/common/java/android/net/NetworkCapabilitiesTest.java index 499b52d3be..4f6dbb0f1d 100644 --- a/tests/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/common/java/android/net/NetworkCapabilitiesTest.java @@ -51,6 +51,8 @@ import static android.net.NetworkCapabilities.NET_ENTERPRISE_ID_5; import static android.net.NetworkCapabilities.REDACT_FOR_ACCESS_FINE_LOCATION; import static android.net.NetworkCapabilities.REDACT_FOR_LOCAL_MAC_ADDRESS; import static android.net.NetworkCapabilities.REDACT_FOR_NETWORK_SETTINGS; +import static android.net.NetworkCapabilities.REDACT_FOR_THREAD_NETWORK_PRIVILEGED; +import static android.net.NetworkCapabilities.REDACT_NONE; import static android.net.NetworkCapabilities.SIGNAL_STRENGTH_UNSPECIFIED; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET; @@ -63,9 +65,11 @@ import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static android.net.NetworkCapabilities.TRANSPORT_WIFI_AWARE; import static android.os.Process.INVALID_UID; +import static com.android.modules.utils.build.SdkLevel.isAtLeastB; import static com.android.modules.utils.build.SdkLevel.isAtLeastS; import static com.android.modules.utils.build.SdkLevel.isAtLeastT; import static com.android.modules.utils.build.SdkLevel.isAtLeastV; +import static com.android.net.thread.platform.flags.Flags.FLAG_THREAD_MOBILE_ENABLED; import static com.android.testutils.MiscAsserts.assertEmpty; import static com.android.testutils.MiscAsserts.assertThrows; import static com.android.testutils.ParcelUtils.assertParcelingIsLossless; @@ -74,11 +78,15 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; +import android.annotation.Nullable; +import android.net.NetworkCapabilities.RedactionHelper; import android.net.wifi.aware.DiscoverySession; import android.net.wifi.aware.PeerHandle; import android.net.wifi.aware.WifiAwareNetworkSpecifier; @@ -99,6 +107,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -1235,6 +1244,44 @@ public class NetworkCapabilitiesTest { } } + /** + * Redactable NetworkSpecifier subclass for verifying redaction mechanism. + */ + private static final class RedactableNetworkSpecifier extends NetworkSpecifier { + final boolean locationRedacted; + final boolean threadNetworkRedacted; + + RedactableNetworkSpecifier() { + locationRedacted = false; + threadNetworkRedacted = false; + } + + RedactableNetworkSpecifier(boolean locationRedacted, boolean threadNetworkRedacted) { + this.locationRedacted = locationRedacted; + this.threadNetworkRedacted = threadNetworkRedacted; + } + + @Override + public long getApplicableRedactions() { + return REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_THREAD_NETWORK_PRIVILEGED; + } + + @Override + @Nullable + public NetworkSpecifier redact(long redactions) { + return new RedactableNetworkSpecifier( + (redactions & REDACT_FOR_ACCESS_FINE_LOCATION) != 0, + (redactions & REDACT_FOR_THREAD_NETWORK_PRIVILEGED) != 0); + } + } + + /** + * Unredactable NetworkSpecifier subclass for verifying redaction mechanism. + */ + private static final class UnredactableNetworkSpecifier extends NetworkSpecifier { + // not overriding the getApplicableRedactions() and redact(long) methods + } + @Test public void testBuilder() { final int ownerUid = 1001; @@ -1621,4 +1668,35 @@ public class NetworkCapabilitiesTest { assertFalse(wifiRequestNc.satisfiedByNetworkCapabilities(localWifiNc)); } } + + @Test + public void testRedactionHelper() throws Exception { + final UnredactableNetworkSpecifier unredactableSpecifier = + new UnredactableNetworkSpecifier(); + final RedactableNetworkSpecifier redactableSpecifier = new RedactableNetworkSpecifier(); + + assertEquals(REDACT_NONE, + RedactionHelper.getSpecifierApplicableRedactions(unredactableSpecifier)); + assertEquals(unredactableSpecifier, + RedactionHelper.getSpecifierRedactResult( + unredactableSpecifier, REDACT_FOR_ACCESS_FINE_LOCATION)); + assertEquals(REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_THREAD_NETWORK_PRIVILEGED, + RedactionHelper.getSpecifierApplicableRedactions(redactableSpecifier)); + // Call getSpecifierApplicableRedactions() twice to make sure the cache works + assertEquals(REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_THREAD_NETWORK_PRIVILEGED, + RedactionHelper.getSpecifierApplicableRedactions(redactableSpecifier)); + final RedactableNetworkSpecifier redactedSpecifier = + (RedactableNetworkSpecifier) RedactionHelper.getSpecifierRedactResult( + redactableSpecifier, + REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_THREAD_NETWORK_PRIVILEGED); + assertTrue(redactedSpecifier.locationRedacted); + assertTrue(redactedSpecifier.threadNetworkRedacted); + // Call getSpecifierRedactResult twice to make sure the cache works + final RedactableNetworkSpecifier redactedSpecifier2 = + (RedactableNetworkSpecifier) RedactionHelper.getSpecifierRedactResult( + redactableSpecifier, + REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_THREAD_NETWORK_PRIVILEGED); + assertTrue(redactedSpecifier2.locationRedacted); + assertTrue(redactedSpecifier2.threadNetworkRedacted); + } } From 5fd72324ce7f25e07173d3c202faf4022bf0e69b Mon Sep 17 00:00:00 2001 From: Kangping Dong Date: Mon, 30 Jun 2025 18:23:36 +0800 Subject: [PATCH 03/21] [connectivity] redact NeworkSpecifier in NetworkCapabilities The existing NetworkCapabilities class redacts only TransportInfo and leaves NetworkSpecifier as it is. This is counter-intuitive and the specifier should be redacted along with TransportInfo as well. Bug 374037595 Flag: NONE this has no behavior change for all existing NetworkSpecifier subclasses because they are not overriding the new NetworkSpecifier redaction methods Change-Id: I73c880add1604afc8b56190647305a54e89e8e53 --- .../src/android/net/NetworkCapabilities.java | 18 +++++++---- framework/src/android/net/TransportInfo.java | 3 ++ .../android/net/NetworkCapabilitiesTest.java | 32 +++++++++++++++++-- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java index 5974b58c9e..ca2db5123c 100644 --- a/framework/src/android/net/NetworkCapabilities.java +++ b/framework/src/android/net/NetworkCapabilities.java @@ -352,6 +352,11 @@ public final class NetworkCapabilities implements Parcelable { if (mTransportInfo != null) { mTransportInfo = nc.mTransportInfo.makeCopy(redactions); } + if (mNetworkSpecifier != null) { + mNetworkSpecifier = + (NetworkSpecifier) RedactionHelper.getSpecifierRedactResult( + mNetworkSpecifier, redactions); + } } /** @@ -3022,13 +3027,14 @@ public final class NetworkCapabilities implements Parcelable { * @hide */ public @RedactionType long getApplicableRedactions() { - // Currently, there are no fields redacted in NetworkCapabilities itself, so we just - // passthrough the redactions required by the embedded TransportInfo. If this changes - // in the future, modify this method. - if (mTransportInfo == null) { - return NetworkCapabilities.REDACT_NONE; + long redactions = REDACT_NONE; + if (mTransportInfo != null) { + redactions |= mTransportInfo.getApplicableRedactions(); } - return mTransportInfo.getApplicableRedactions(); + if (mNetworkSpecifier != null) { + redactions |= RedactionHelper.getSpecifierApplicableRedactions(mNetworkSpecifier); + } + return redactions; } private NetworkCapabilities removeDefaultCapabilites() { diff --git a/framework/src/android/net/TransportInfo.java b/framework/src/android/net/TransportInfo.java index fa889eabb8..9a49ecef15 100644 --- a/framework/src/android/net/TransportInfo.java +++ b/framework/src/android/net/TransportInfo.java @@ -50,6 +50,9 @@ public interface TransportInfo { * *

* + *

Subclasses should silently ignore redactions which are not included in {@link + * #getApplicableRedactions()}. + * * @param redactions bitmask of redactions that needs to be performed on this instance. * @return Copy of this instance with the necessary redactions. * @hide diff --git a/tests/common/java/android/net/NetworkCapabilitiesTest.java b/tests/common/java/android/net/NetworkCapabilitiesTest.java index 4f6dbb0f1d..373b1f36da 100644 --- a/tests/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/common/java/android/net/NetworkCapabilitiesTest.java @@ -418,13 +418,15 @@ public class NetworkCapabilitiesTest { assertParcelingIsLossless(cap); } - private static NetworkCapabilities createNetworkCapabilitiesWithTransportInfo() { + private static NetworkCapabilities createNetworkCapabilitiesWithTransportInfoAndSpecifier() { return new NetworkCapabilities() .addCapability(NET_CAPABILITY_INTERNET) .addCapability(NET_CAPABILITY_EIMS) .addCapability(NET_CAPABILITY_NOT_METERED) + .addTransportType(TRANSPORT_THREAD) .setSSID(TEST_SSID) .setTransportInfo(new TestTransportInfo()) + .setNetworkSpecifier(new RedactableNetworkSpecifier()) .setRequestorPackageName("com.android.test") .setRequestorUid(9304); } @@ -433,7 +435,8 @@ public class NetworkCapabilitiesTest { public void testNetworkCapabilitiesCopyWithNoRedactions() { assumeTrue(isAtLeastS()); - final NetworkCapabilities netCap = createNetworkCapabilitiesWithTransportInfo(); + final NetworkCapabilities netCap = + createNetworkCapabilitiesWithTransportInfoAndSpecifier(); final NetworkCapabilities netCapWithNoRedactions = new NetworkCapabilities(netCap, NetworkCapabilities.REDACT_NONE); TestTransportInfo testTransportInfo = @@ -441,13 +444,18 @@ public class NetworkCapabilitiesTest { assertFalse(testTransportInfo.locationRedacted); assertFalse(testTransportInfo.localMacAddressRedacted); assertFalse(testTransportInfo.settingsRedacted); + RedactableNetworkSpecifier testNetworkSpecifier = + (RedactableNetworkSpecifier) netCapWithNoRedactions.getNetworkSpecifier(); + assertFalse(testNetworkSpecifier.locationRedacted); + assertFalse(testNetworkSpecifier.threadNetworkRedacted); } @Test public void testNetworkCapabilitiesCopyWithoutLocationSensitiveFields() { assumeTrue(isAtLeastS()); - final NetworkCapabilities netCap = createNetworkCapabilitiesWithTransportInfo(); + final NetworkCapabilities netCap = + createNetworkCapabilitiesWithTransportInfoAndSpecifier(); final NetworkCapabilities netCapWithNoRedactions = new NetworkCapabilities(netCap, REDACT_FOR_ACCESS_FINE_LOCATION); TestTransportInfo testTransportInfo = @@ -455,6 +463,24 @@ public class NetworkCapabilitiesTest { assertTrue(testTransportInfo.locationRedacted); assertFalse(testTransportInfo.localMacAddressRedacted); assertFalse(testTransportInfo.settingsRedacted); + RedactableNetworkSpecifier testNetworkSpecifier = + (RedactableNetworkSpecifier) netCapWithNoRedactions.getNetworkSpecifier(); + assertTrue(testNetworkSpecifier.locationRedacted); + assertFalse(testNetworkSpecifier.threadNetworkRedacted); + } + + @Test + public void testGetApplicableRedactions() { + assumeTrue(isAtLeastS()); + + final NetworkCapabilities netCap = + createNetworkCapabilitiesWithTransportInfoAndSpecifier(); + final long redactions = netCap.getApplicableRedactions(); + + assertEquals( + REDACT_FOR_ACCESS_FINE_LOCATION | REDACT_FOR_LOCAL_MAC_ADDRESS + | REDACT_FOR_NETWORK_SETTINGS | REDACT_FOR_THREAD_NETWORK_PRIVILEGED, + redactions); } @Test From d2c9202a9b534722a6b33762fa610f6096c1769d Mon Sep 17 00:00:00 2001 From: Kangping Dong Date: Fri, 27 Jun 2025 14:53:04 +0800 Subject: [PATCH 04/21] [connectivity] add permission check on requested NetworkSpecifier This commit adds permission check for new NetworkSpecifier subclasses which override the new redact(long) method. It throws SecurityException if the caller doesn't hold the permissions to use the sensitive info in the NetworkSpecifier class. Bug: 374037595 Flag: NONE can't use simply flag guarding because the code needs to maintain compatibility across different mainline and platform versions Change-Id: Ife8efab3e99adf41d7522fedbde3b4ec80045c77 --- .../android/server/ConnectivityService.java | 37 +++++ .../CSNetworkSpecifierTest.kt | 154 ++++++++++++++++++ 2 files changed, 191 insertions(+) create mode 100644 tests/unit/java/com/android/server/connectivityservice/CSNetworkSpecifierTest.kt diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 20026ccafb..72981f4f50 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -110,6 +110,7 @@ import static android.net.NetworkCapabilities.REDACT_FOR_ACCESS_FINE_LOCATION; import static android.net.NetworkCapabilities.REDACT_FOR_LOCAL_MAC_ADDRESS; import static android.net.NetworkCapabilities.REDACT_FOR_NETWORK_SETTINGS; import static android.net.NetworkCapabilities.REDACT_FOR_THREAD_NETWORK_PRIVILEGED; +import static android.net.NetworkCapabilities.REDACT_NONE; import static android.net.NetworkCapabilities.RES_ID_MATCH_ALL_RESERVATIONS; import static android.net.NetworkCapabilities.RES_ID_UNSET; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; @@ -209,6 +210,7 @@ import android.net.ConnectivityManager; import android.net.ConnectivityManager.BlockedReason; import android.net.ConnectivityManager.NetworkCallback; import android.net.ConnectivityManager.RestrictBackgroundStatus; +import android.net.NetworkCapabilities.RedactionHelper; import android.net.ConnectivitySettingsManager; import android.net.DataStallReportParcelable; import android.net.DnsResolverServiceManager; @@ -428,6 +430,8 @@ import java.io.IOException; import java.io.InterruptedIOException; import java.io.PrintWriter; import java.io.Writer; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.net.Inet4Address; import java.net.Inet6Address; import java.net.InetAddress; @@ -8880,11 +8884,44 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + @Nullable + private NetworkSpecifier maybeRedactRequestedNetworkSpecifier( + @Nullable NetworkSpecifier specifier, int callerPid, int callerUid, + String callerPackageName, @Nullable String callerAttributionTag, + boolean includeLocationSensitiveInfo) { + if (specifier == null) { + return null; + } + + final long applicableRedactions = + RedactionHelper.getSpecifierApplicableRedactions(specifier); + if (applicableRedactions == REDACT_NONE) { + return specifier; + } + + final RedactionPermissionChecker redactionPermissionChecker = + new RedactionPermissionChecker( + callerPid, callerUid, callerPackageName, callerAttributionTag); + final long redactions = retrieveRequiredRedactions( + applicableRedactions, redactionPermissionChecker, + includeLocationSensitiveInfo); + return RedactionHelper.getSpecifierRedactResult(specifier, redactions); + } + // This checks that the passed capabilities either do not request a // specific SSID/SignalStrength, or the calling app has permission to do so. private void ensureSufficientPermissionsForRequest(NetworkCapabilities nc, int callerPid, int callerUid, String callerPackageName, @Nullable String callerAttributionTag, boolean includeLocationSensitiveInfo) { + final NetworkSpecifier specifier = nc.getNetworkSpecifier(); + final NetworkSpecifier redactedSpecifier = maybeRedactRequestedNetworkSpecifier( + specifier, callerPid, callerUid, callerPackageName, callerAttributionTag, + includeLocationSensitiveInfo); + if (!Objects.equals(redactedSpecifier, specifier)) { + throw new SecurityException( + "Insufficient permissions to request a specific NetworkSpecifier"); + } + if (null != nc.getSsid() && !hasSettingsPermission(callerPid, callerUid)) { throw new SecurityException("Insufficient permissions to request a specific SSID"); } diff --git a/tests/unit/java/com/android/server/connectivityservice/CSNetworkSpecifierTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSNetworkSpecifierTest.kt new file mode 100644 index 0000000000..ba77fcc995 --- /dev/null +++ b/tests/unit/java/com/android/server/connectivityservice/CSNetworkSpecifierTest.kt @@ -0,0 +1,154 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server + +import android.Manifest.permission.LOCAL_MAC_ADDRESS +import android.Manifest.permission.THREAD_NETWORK_PRIVILEGED +import android.content.pm.PackageManager.PERMISSION_DENIED +import android.content.pm.PackageManager.PERMISSION_GRANTED +import android.net.MacAddress +import android.net.NetworkCapabilities +import android.net.NetworkCapabilities.REDACT_FOR_LOCAL_MAC_ADDRESS +import android.net.NetworkCapabilities.REDACT_FOR_THREAD_NETWORK_PRIVILEGED +import android.net.NetworkCapabilities.TRANSPORT_WIFI +import android.net.NetworkRequest +import android.net.NetworkSpecifier +import android.os.Build.VERSION_CODES +import android.os.Parcel +import android.os.Parcelable +import com.android.testutils.DevSdkIgnoreRule +import com.android.testutils.DevSdkIgnoreRule.IgnoreAfter +import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo +import com.android.testutils.DevSdkIgnoreRunner +import com.android.testutils.TestableNetworkCallback +import com.android.testutils.assertThrows +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +/** Unit tests for {@link NetworkSpecifier} functionalities in the ConnectivityService class. */ +@RunWith(DevSdkIgnoreRunner::class) +@IgnoreUpTo(VERSION_CODES.R) +@DevSdkIgnoreRunner.MonitorThreadLeak +class CSNetworkSpecifierTest : CSTest() { + @get:Rule(order = 0) val mIgnoreRule = DevSdkIgnoreRule() + + /** + * A NetworkSpecifier subclass which overrides {@link + * NetworkSpecifier#getApplicableRedactions()} and {@link NetworkSpecifier#redact(long)}. + */ + data class RedactableNetworkSpecifier( + val macAddress: MacAddress?, val threadPrivilegedKey: String?) : + NetworkSpecifier(), Parcelable { + constructor( + p: Parcel + ) : this(p.readParcelable(MacAddress::class.java.classLoader, MacAddress::class.java), + p.readString()) + + override fun canBeSatisfiedBy(other: NetworkSpecifier?) = equals(other) + + override fun redact(redactions: Long): NetworkSpecifier? = + RedactableNetworkSpecifier( + if ((redactions and REDACT_FOR_LOCAL_MAC_ADDRESS) != 0L) null + else macAddress, + if ((redactions and REDACT_FOR_THREAD_NETWORK_PRIVILEGED) != 0L) null + else threadPrivilegedKey, + ) + + override fun getApplicableRedactions() = + REDACT_FOR_LOCAL_MAC_ADDRESS or REDACT_FOR_THREAD_NETWORK_PRIVILEGED + + override fun writeToParcel(dest: Parcel, flags: Int) { + dest.writeParcelable(macAddress, 0 /* parcelableFlags */) + dest.writeString(threadPrivilegedKey) + } + + override fun describeContents() = 0 + companion object CREATOR : Parcelable.Creator { + override fun createFromParcel(source: Parcel) = RedactableNetworkSpecifier(source) + override fun newArray(size: Int) = arrayOfNulls(size) + } + } + + fun newWifiRequestBuilder(): NetworkRequest.Builder { + return NetworkRequest.Builder().addTransportType(TRANSPORT_WIFI) + } + + @Test + @IgnoreUpTo(VERSION_CODES.UPSIDE_DOWN_CAKE) + fun testRequestNetworkPermission_permissionDenied_throwsSecurityException() { + context.setPermission(LOCAL_MAC_ADDRESS, PERMISSION_DENIED) + context.setPermission(THREAD_NETWORK_PRIVILEGED, PERMISSION_DENIED) + val specifier = RedactableNetworkSpecifier(DEFAULT_MAC_ADDR, DEFAULT_THREAD_KEY) + val request = newWifiRequestBuilder().setNetworkSpecifier(specifier).build() + + assertThrows(SecurityException::class.java) { + cm.requestNetwork(request, TestableNetworkCallback()) + } + assertThrows(SecurityException::class.java) { + cm.registerNetworkCallback(request, TestableNetworkCallback()) + } + } + + @Test + @IgnoreUpTo(VERSION_CODES.UPSIDE_DOWN_CAKE) + fun testRequestNetworkPermission_noNetworkSpecifier_noThrow() { + context.setPermission(LOCAL_MAC_ADDRESS, PERMISSION_DENIED) + context.setPermission(THREAD_NETWORK_PRIVILEGED, PERMISSION_DENIED) + val request = newWifiRequestBuilder().build() + + cm.requestNetwork(request, TestableNetworkCallback()) + val callback = TestableNetworkCallback() + cm.registerNetworkCallback(request, callback) + cm.unregisterNetworkCallback(callback) + } + + @Test + @IgnoreUpTo(VERSION_CODES.UPSIDE_DOWN_CAKE) + fun testRequestNetworkPermission_permissionGranted_noThrow() { + context.setPermission(LOCAL_MAC_ADDRESS, PERMISSION_GRANTED) + context.setPermission(THREAD_NETWORK_PRIVILEGED, PERMISSION_GRANTED) + val specifier = RedactableNetworkSpecifier(DEFAULT_MAC_ADDR, DEFAULT_THREAD_KEY) + val request = newWifiRequestBuilder().setNetworkSpecifier(specifier).build() + + cm.requestNetwork(request, TestableNetworkCallback()) + val callback = TestableNetworkCallback() + cm.registerNetworkCallback(request, callback) + cm.unregisterNetworkCallback(callback) + } + + @Test + @IgnoreAfter(VERSION_CODES.UPSIDE_DOWN_CAKE) + fun testThreadPrivilegedPermission_platformOlderThanV_throwsSecurityException() { + context.setPermission(LOCAL_MAC_ADDRESS, PERMISSION_GRANTED) + context.setPermission(THREAD_NETWORK_PRIVILEGED, PERMISSION_GRANTED) + val specifier = RedactableNetworkSpecifier(DEFAULT_MAC_ADDR, DEFAULT_THREAD_KEY) + val request = newWifiRequestBuilder().setNetworkSpecifier(specifier).build() + + assertThrows(SecurityException::class.java) { + cm.requestNetwork(request, TestableNetworkCallback()) + } + assertThrows(SecurityException::class.java) { + cm.registerNetworkCallback(request, TestableNetworkCallback()) + } + } + + companion object { + private val DEFAULT_MAC_ADDR = MacAddress.fromString("0:1:2:3:4:5") + private val DEFAULT_THREAD_KEY = "thread-key" + } +} From b9c6df36de25c7b0f5f1167a705098e85617cfa6 Mon Sep 17 00:00:00 2001 From: Kangping Dong Date: Fri, 27 Jun 2025 16:25:40 +0800 Subject: [PATCH 05/21] [connectivity] move redaction to createWithLocationInfoSanitizedIfNecessaryWhenParceled This commit has almost no behavior changes, because: 1. usage of networkCapabilitiesRestrictedForCallerPermissions is either wrapped by createWithLocationInfoSanitizedIfNecessaryWhenParceled or getNetworkCapabilitiesInternal. 2. All usages of getNetworkCapabilitiesInternal is either wrapped by createWithLocationInfoSanitizedIfNecessaryWhenParceled or the redacted info in the return value is not used by its caller 3. The only exception for getNetworkCapabilitiesInternal is that it is used in simulateDataStall() which checks the underlying network but underlying network in the NetworkCapabilities can be redacted by networkCapabilitiesRestrictedForCallerPermissions. Removing redaction from getNetworkCapabilitiesInternal() will loose the current permission requirement but this is wanted as the existing implementation has stricter permission requirements than the API signature declares and this CL fixes it Bug: 374037595 Flag: NONE no behavior change but only bug fix Change-Id: I80491d26ed7d11cb3fe1850ef7be9ae7600d5dd5 --- .../android/server/ConnectivityService.java | 80 ++++++++----------- .../server/ConnectivityServiceTest.java | 5 +- 2 files changed, 37 insertions(+), 48 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 72981f4f50..9f71588990 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -3120,8 +3120,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private NetworkCapabilities getNetworkCapabilitiesInternal(NetworkAgentInfo nai) { if (nai == null) return null; synchronized (nai) { - return networkCapabilitiesRestrictedForCallerPermissions( - nai.networkCapabilities, Binder.getCallingPid(), mDeps.getCallingUid()); + return new NetworkCapabilities(nai.networkCapabilities); } } @@ -3147,8 +3146,7 @@ public class ConnectivityService extends IConnectivityManager.Stub return null; } return createWithLocationInfoSanitizedIfNecessaryWhenParceled( - networkCapabilitiesRestrictedForCallerPermissions(nc, -1 /* callerPid */, uid), - true /* includeLocationSensitiveInfo */, -1 /* callingPid */, uid, packageName, + nc, true /* includeLocationSensitiveInfo */, -1 /* callingPid */, uid, packageName, callingAttributionTag); } @@ -3166,35 +3164,6 @@ public class ConnectivityService extends IConnectivityManager.Stub android.Manifest.permission.NETWORK_FACTORY); } - @VisibleForTesting - NetworkCapabilities networkCapabilitiesRestrictedForCallerPermissions( - NetworkCapabilities nc, int callerPid, int callerUid) { - // Note : here it would be nice to check ACCESS_NETWORK_STATE and return null, but - // this would be expensive (one more permission check every time any NC callback is - // sent) and possibly dangerous : apps normally can't lose ACCESS_NETWORK_STATE, if - // it happens for some reason (e.g. the package is uninstalled while CS is trying to - // send the callback) it would crash the system server with NPE. - final NetworkCapabilities newNc = new NetworkCapabilities(nc); - if (!hasSettingsPermission(callerPid, callerUid)) { - newNc.setUids(null); - newNc.setSSID(null); - } - if (newNc.getNetworkSpecifier() != null) { - newNc.setNetworkSpecifier(newNc.getNetworkSpecifier().redact()); - } - if (!hasAnyPermissionOf(mContext, callerPid, callerUid, - android.Manifest.permission.NETWORK_STACK, - NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK)) { - newNc.setAdministratorUids(new int[0]); - } - if (!canSeeAllowedUids(callerPid, callerUid, newNc.getOwnerUid())) { - newNc.setAllowedUids(new ArraySet<>()); - } - redactUnderlyingNetworksForCapabilities(newNc, callerPid, callerUid); - - return newNc; - } - /** * Wrapper used to cache the permission check results performed for the corresponding * app. This avoids performing multiple permission checks for different fields in @@ -3347,6 +3316,32 @@ public class ConnectivityService extends IConnectivityManager.Stub nc.getApplicableRedactions(), redactionPermissionChecker, includeLocationSensitiveInfo); final NetworkCapabilities newNc = new NetworkCapabilities(nc, redactions); + + // Note : here it would be nice to check ACCESS_NETWORK_STATE and return null, but + // this would be expensive (one more permission check every time any NC callback is + // sent) and possibly dangerous : apps normally can't lose ACCESS_NETWORK_STATE, if + // it happens for some reason (e.g. the package is uninstalled while CS is trying to + // send the callback) it would crash the system server with NPE. + if (!hasSettingsPermission(callingPid, callingUid)) { + newNc.setUids(null); + newNc.setSSID(null); + } + // The NetworkCapabilities constructor above will only have redacted the + // specifier if it implements the getApplicableRedactions method. If it does + // not, then redact() must be called. + if (newNc.getNetworkSpecifier() != null) { + newNc.setNetworkSpecifier(newNc.getNetworkSpecifier().redact()); + } + if (!hasAnyPermissionOf(mContext, callingPid, callingUid, + android.Manifest.permission.NETWORK_STACK, + NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK)) { + newNc.setAdministratorUids(new int[0]); + } + if (!canSeeAllowedUids(callingPid, callingUid, newNc.getOwnerUid())) { + newNc.setAllowedUids(new ArraySet<>()); + } + redactUnderlyingNetworksForCapabilities(newNc, callingPid, callingUid); + // Reset owner uid if not destined for the owner app. // TODO : calling UID is redacted because apps should generally not know what UID is // bringing up the VPN, but this should not apply to some very privileged apps like settings @@ -11815,10 +11810,8 @@ public class ConnectivityService extends IConnectivityManager.Stub case CALLBACK_RESERVED: { final NetworkCapabilities nc = createWithLocationInfoSanitizedIfNecessaryWhenParceled( - networkCapabilitiesRestrictedForCallerPermissions( - nri.getReservedCapabilities(), nri.mPid, nri.mUid), - includeLocationSensitiveInfo, nri.mPid, nri.mUid, - nrForCallback.getRequestorPackageName(), + nri.getReservedCapabilities(), includeLocationSensitiveInfo, + nri.mPid, nri.mUid, nrForCallback.getRequestorPackageName(), nri.mCallingAttributionTag); putParcelable(bundle, nc); break; @@ -11826,10 +11819,8 @@ public class ConnectivityService extends IConnectivityManager.Stub case CALLBACK_AVAILABLE: { final NetworkCapabilities nc = createWithLocationInfoSanitizedIfNecessaryWhenParceled( - networkCapabilitiesRestrictedForCallerPermissions( - networkAgent.networkCapabilities, nri.mPid, nri.mUid), - includeLocationSensitiveInfo, nri.mPid, nri.mUid, - nrForCallback.getRequestorPackageName(), + networkAgent.networkCapabilities, includeLocationSensitiveInfo, + nri.mPid, nri.mUid, nrForCallback.getRequestorPackageName(), nri.mCallingAttributionTag); putParcelable(bundle, nc); putParcelable(bundle, linkPropertiesRestrictedForCallerPermissions( @@ -11842,14 +11833,11 @@ public class ConnectivityService extends IConnectivityManager.Stub } case CALLBACK_CAP_CHANGED: { // networkAgent can't be null as it has been accessed a few lines above. - final NetworkCapabilities netCap = - networkCapabilitiesRestrictedForCallerPermissions( - networkAgent.networkCapabilities, nri.mPid, nri.mUid); putParcelable( bundle, createWithLocationInfoSanitizedIfNecessaryWhenParceled( - netCap, includeLocationSensitiveInfo, nri.mPid, nri.mUid, - nrForCallback.getRequestorPackageName(), + networkAgent.networkCapabilities, includeLocationSensitiveInfo, + nri.mPid, nri.mUid, nrForCallback.getRequestorPackageName(), nri.mCallingAttributionTag)); break; } diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index e95406b9ea..63e7fe950f 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -17505,8 +17505,9 @@ public class ConnectivityServiceTest { nc.setSubscriptionIds(subIds); final NetworkCapabilities result = - mService.networkCapabilitiesRestrictedForCallerPermissions( - nc, Process.myPid(), Process.myUid()); + mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( + nc, false /* includeLocationSensitiveInfo */, Process.myPid(), + Process.myUid(), mContext.getPackageName(), getAttributionTag()); assertEquals(subIds, result.getSubscriptionIds()); } From 997c5651d0606e7974045bb26a9f891bb9651d92 Mon Sep 17 00:00:00 2001 From: Kangping Dong Date: Tue, 2 Sep 2025 20:59:45 +0800 Subject: [PATCH 06/21] [connectivity] use new redact(RedactionType) for output NetworkSpecifier This commit replace the existing usage of NetworkSpecifier#redact() to the new NetworkSpecifier#redact(RedactionType) API. This doesn't change the existing behavior because redact() is still called for NetworkSpecifiers which doesn't oevrride getApplicableRedactions() method. Bug: 374037595 Flag: NONE can't use simply flag guarding because the code needs to maintain compatibility across different mainline and platform versions Change-Id: I30d7257b8e5880218b315ade60d2d77fed1f89fb --- .../android/server/ConnectivityService.java | 20 +++-- .../CSNetworkSpecifierTest.kt | 86 +++++++++++++++++++ 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 9f71588990..1317fc869d 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -3326,12 +3326,22 @@ public class ConnectivityService extends IConnectivityManager.Stub newNc.setUids(null); newNc.setSSID(null); } - // The NetworkCapabilities constructor above will only have redacted the - // specifier if it implements the getApplicableRedactions method. If it does - // not, then redact() must be called. - if (newNc.getNetworkSpecifier() != null) { - newNc.setNetworkSpecifier(newNc.getNetworkSpecifier().redact()); + // This code does not need to call redact(long) because that was already done above during + // the construction of newNc. + // If {@code getApplicableRedactions()} of a NetworkSpecifier subclass returns + // {@code REDACT_NONE}, it's because either 1) it doesn't override it or 2) it does + // override it, and the implementation returned REDACT_NONE. For the first case, call + // {@code redact()} for backward compatibility with existing NetworkSpecifier such as + // WifiNetworkSpecifier. For the second case, it's still okay to call {@code redact()} as + // it's suggested that all new NetworkSpecifier subclasses should not override the default + // {@code redact()} and it will do nothing (this is consistent with what REDACT_NONE + // indicates). + final NetworkSpecifier specifier = newNc.getNetworkSpecifier(); + if (specifier != null + && RedactionHelper.getSpecifierApplicableRedactions(specifier) == REDACT_NONE) { + newNc.setNetworkSpecifier(specifier.redact()); } + if (!hasAnyPermissionOf(mContext, callingPid, callingUid, android.Manifest.permission.NETWORK_STACK, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK)) { diff --git a/tests/unit/java/com/android/server/connectivityservice/CSNetworkSpecifierTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSNetworkSpecifierTest.kt index ba77fcc995..780d24079e 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSNetworkSpecifierTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSNetworkSpecifierTest.kt @@ -35,6 +35,7 @@ import com.android.testutils.DevSdkIgnoreRule.IgnoreAfter import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo import com.android.testutils.DevSdkIgnoreRunner import com.android.testutils.TestableNetworkCallback +import com.android.testutils.TestableNetworkCallback.Event.CapabilitiesChanged import com.android.testutils.assertThrows import org.junit.Rule import org.junit.Test @@ -84,10 +85,40 @@ class CSNetworkSpecifierTest : CSTest() { } } + /** Legacy NetworkSpecifier subclass which overrides only the {@code redact()} method. */ + data class LegacyNetworkSpecifier(val macAddress: MacAddress?) : + NetworkSpecifier(), Parcelable { + constructor( + p: Parcel + ) : this(p.readParcelable(MacAddress::class.java.classLoader, MacAddress::class.java)) + + override fun canBeSatisfiedBy(other: NetworkSpecifier?) = equals(other) + + override fun redact(): NetworkSpecifier? = + LegacyNetworkSpecifier(null) + + override fun writeToParcel(dest: Parcel, flags: Int) { + dest.writeParcelable(macAddress, 0 /* parcelableFlags */) + } + + override fun describeContents() = 0 + companion object CREATOR : Parcelable.Creator { + override fun createFromParcel(source: Parcel) = LegacyNetworkSpecifier(source) + override fun newArray(size: Int) = arrayOfNulls(size) + } + } + fun newWifiRequestBuilder(): NetworkRequest.Builder { return NetworkRequest.Builder().addTransportType(TRANSPORT_WIFI) } + fun newWifiCapabilities(specifier: NetworkSpecifier): NetworkCapabilities { + return defaultNc().apply { + addTransportType(TRANSPORT_WIFI) + setNetworkSpecifier(specifier) + } + } + @Test @IgnoreUpTo(VERSION_CODES.UPSIDE_DOWN_CAKE) fun testRequestNetworkPermission_permissionDenied_throwsSecurityException() { @@ -147,6 +178,61 @@ class CSNetworkSpecifierTest : CSTest() { } } + @Test + @IgnoreUpTo(VERSION_CODES.UPSIDE_DOWN_CAKE) + fun testNetworkCallback_permissionGranted_notRedacted() { + context.setPermission(LOCAL_MAC_ADDRESS, PERMISSION_GRANTED) + context.setPermission(THREAD_NETWORK_PRIVILEGED, PERMISSION_GRANTED) + val specifier = RedactableNetworkSpecifier(DEFAULT_MAC_ADDR, DEFAULT_THREAD_KEY) + val request = newWifiRequestBuilder().setNetworkSpecifier(specifier).build() + + val callback = TestableNetworkCallback() + cm.registerNetworkCallback(request, callback) + Agent(TRANSPORT_WIFI, baseNc = newWifiCapabilities(specifier)).connect() + + callback.eventuallyExpect { + it.caps.networkSpecifier == specifier + } + cm.unregisterNetworkCallback(callback) + } + + @Test + @IgnoreUpTo(VERSION_CODES.UPSIDE_DOWN_CAKE) + fun testNetworkCallback_permissionDenied_redacted() { + context.setPermission(LOCAL_MAC_ADDRESS, PERMISSION_DENIED) + context.setPermission(THREAD_NETWORK_PRIVILEGED, PERMISSION_DENIED) + val specifier = RedactableNetworkSpecifier(DEFAULT_MAC_ADDR, DEFAULT_THREAD_KEY) + val request = newWifiRequestBuilder().build() + + val callback = TestableNetworkCallback() + cm.registerNetworkCallback(request, callback) + Agent(TRANSPORT_WIFI, baseNc = newWifiCapabilities(specifier)).connect() + + callback.eventuallyExpect { + (it.caps.networkSpecifier as RedactableNetworkSpecifier).macAddress == null + (it.caps.networkSpecifier as RedactableNetworkSpecifier).threadPrivilegedKey == null + } + cm.unregisterNetworkCallback(callback) + } + + @Test + @IgnoreUpTo(VERSION_CODES.UPSIDE_DOWN_CAKE) + fun testNetworkCallback_permissionGrantedForLegacyNetworkSpecifier_redacted() { + context.setPermission(LOCAL_MAC_ADDRESS, PERMISSION_GRANTED) + context.setPermission(THREAD_NETWORK_PRIVILEGED, PERMISSION_GRANTED) + val specifier = LegacyNetworkSpecifier(DEFAULT_MAC_ADDR) + val request = newWifiRequestBuilder().build() + + val callback = TestableNetworkCallback() + cm.registerNetworkCallback(request, callback) + Agent(TRANSPORT_WIFI, baseNc = newWifiCapabilities(specifier)).connect() + + callback.eventuallyExpect { + (it.caps.networkSpecifier as LegacyNetworkSpecifier).macAddress == null + } + cm.unregisterNetworkCallback(callback) + } + companion object { private val DEFAULT_MAC_ADDR = MacAddress.fromString("0:1:2:3:4:5") private val DEFAULT_THREAD_KEY = "thread-key" From 6fd0624452f738d31dbd47562712ad5f8143400b Mon Sep 17 00:00:00 2001 From: Kangping Dong Date: Fri, 27 Jun 2025 12:28:25 +0800 Subject: [PATCH 07/21] [connectivity] rename createWithLocationInfoSanitizedIfNecessaryWhenParceled It's already redacting more sensitive information other than just location info. This is a follow up CL for https://googleplex-android-review.git.corp.google.com/c/platform/packages/modules/Connectivity/+/34224156/comment/98b93666_a824d7bc/ Bug: 374037595 Flag: EXEMPT PURE_REFACTOR Change-Id: I6524c4f9ee9b5273e4ab421f54ae3115dea2cc8a --- .../src/android/net/NetworkCapabilities.java | 2 +- .../com/android/server/ConnectivityService.java | 16 ++++++++-------- .../android/server/ConnectivityServiceTest.java | 14 +++++++------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java index ca2db5123c..a4aac2f082 100644 --- a/framework/src/android/net/NetworkCapabilities.java +++ b/framework/src/android/net/NetworkCapabilities.java @@ -3566,7 +3566,7 @@ public final class NetworkCapabilities implements Parcelable { * mainline version but deployed to older platforms which don't have those two * NetworkSpecifier methods. *

- * See {@link #createWithLocationInfoSanitizedIfNecessaryWhenParceled} and {@link + * See {@link #createWithSensitiveInfoSanitizedIfNecessaryWhenParceled} and {@link * #ensureSufficientPermissionsForRequest} for how this method should be used. */ @Nullable diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 1317fc869d..4d70a593d4 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -2906,7 +2906,7 @@ public class ConnectivityService extends IConnectivityManager.Stub && !result.containsKey(nai.network)) { result.put( nai.network, - createWithLocationInfoSanitizedIfNecessaryWhenParceled( + createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( nc, false /* includeLocationSensitiveInfo */, getCallingPid(), mDeps.getCallingUid(), callingPackageName, callingAttributionTag)); @@ -2921,7 +2921,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (null != nc) { result.put( network, - createWithLocationInfoSanitizedIfNecessaryWhenParceled( + createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( nc, false /* includeLocationSensitiveInfo */, getCallingPid(), mDeps.getCallingUid(), callingPackageName, @@ -3129,7 +3129,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Nullable String callingAttributionTag) { mAppOpsManager.checkPackage(mDeps.getCallingUid(), callingPackageName); enforceAccessPermission(); - return createWithLocationInfoSanitizedIfNecessaryWhenParceled( + return createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( getNetworkCapabilitiesInternal(network), false /* includeLocationSensitiveInfo */, getCallingPid(), mDeps.getCallingUid(), callingPackageName, callingAttributionTag); @@ -3145,7 +3145,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (!hasAccessPermission(-1 /* pid */, uid)) { return null; } - return createWithLocationInfoSanitizedIfNecessaryWhenParceled( + return createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( nc, true /* includeLocationSensitiveInfo */, -1 /* callingPid */, uid, packageName, callingAttributionTag); } @@ -3300,7 +3300,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @VisibleForTesting @Nullable - NetworkCapabilities createWithLocationInfoSanitizedIfNecessaryWhenParceled( + NetworkCapabilities createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( @Nullable NetworkCapabilities nc, boolean includeLocationSensitiveInfo, int callingPid, int callingUid, @NonNull String callingPkgName, @Nullable String callingAttributionTag) { @@ -11819,7 +11819,7 @@ public class ConnectivityService extends IConnectivityManager.Stub switch (notificationType) { case CALLBACK_RESERVED: { final NetworkCapabilities nc = - createWithLocationInfoSanitizedIfNecessaryWhenParceled( + createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( nri.getReservedCapabilities(), includeLocationSensitiveInfo, nri.mPid, nri.mUid, nrForCallback.getRequestorPackageName(), nri.mCallingAttributionTag); @@ -11828,7 +11828,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } case CALLBACK_AVAILABLE: { final NetworkCapabilities nc = - createWithLocationInfoSanitizedIfNecessaryWhenParceled( + createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( networkAgent.networkCapabilities, includeLocationSensitiveInfo, nri.mPid, nri.mUid, nrForCallback.getRequestorPackageName(), nri.mCallingAttributionTag); @@ -11845,7 +11845,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // networkAgent can't be null as it has been accessed a few lines above. putParcelable( bundle, - createWithLocationInfoSanitizedIfNecessaryWhenParceled( + createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( networkAgent.networkCapabilities, includeLocationSensitiveInfo, nri.mPid, nri.mUid, nrForCallback.getRequestorPackageName(), nri.mCallingAttributionTag)); diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 63e7fe950f..b3f4a7fd54 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -12452,7 +12452,7 @@ public class ConnectivityServiceTest { boolean includeLocationSensitiveInfo) { final NetworkCapabilities netCap = new NetworkCapabilities().setOwnerUid(ownerUid); - return mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( + return mService.createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( netCap, includeLocationSensitiveInfo, Process.myUid(), callerUid, mContext.getPackageName(), getAttributionTag()) .getOwnerUid(); @@ -12466,7 +12466,7 @@ public class ConnectivityServiceTest { final NetworkCapabilities netCap = new NetworkCapabilities().setTransportInfo(transportInfo); - mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( + mService.createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( netCap, includeLocationSensitiveInfo, Process.myPid(), callerUid, mContext.getPackageName(), getAttributionTag()); if (shouldMakeCopyWithLocationSensitiveFieldsParcelable) { @@ -12624,7 +12624,7 @@ public class ConnectivityServiceTest { final NetworkCapabilities netCap = new NetworkCapabilities().setTransportInfo(transportInfo); - mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( + mService.createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( netCap, false /* includeLocationSensitiveInfoInTransportInfo */, Process.myPid(), Process.myUid(), mContext.getPackageName(), getAttributionTag()); @@ -12643,7 +12643,7 @@ public class ConnectivityServiceTest { final NetworkCapabilities netCap = new NetworkCapabilities().setTransportInfo(transportInfo); - mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( + mService.createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( netCap, false /* includeLocationSensitiveInfoInTransportInfo */, Process.myPid(), Process.myUid(), mContext.getPackageName(), getAttributionTag()); @@ -12663,7 +12663,7 @@ public class ConnectivityServiceTest { final NetworkCapabilities netCap = new NetworkCapabilities().setTransportInfo(transportInfo); - mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( + mService.createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( netCap, false /* includeLocationSensitiveInfoInTransportInfo */, Process.myPid(), Process.myUid(), mContext.getPackageName(), getAttributionTag()); @@ -12682,7 +12682,7 @@ public class ConnectivityServiceTest { final NetworkCapabilities netCap = new NetworkCapabilities().setTransportInfo(transportInfo); - mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( + mService.createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( netCap, false /* includeLocationSensitiveInfoInTransportInfo */, Process.myPid(), Process.myUid(), mContext.getPackageName(), getAttributionTag()); @@ -17505,7 +17505,7 @@ public class ConnectivityServiceTest { nc.setSubscriptionIds(subIds); final NetworkCapabilities result = - mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled( + mService.createWithSensitiveInfoSanitizedIfNecessaryWhenParceled( nc, false /* includeLocationSensitiveInfo */, Process.myPid(), Process.myUid(), mContext.getPackageName(), getAttributionTag()); assertEquals(subIds, result.getSubscriptionIds()); From 95b78f8e6bc4f6354b431402bc04c97d592b6ecb Mon Sep 17 00:00:00 2001 From: Kangping Dong Date: Thu, 11 Sep 2025 21:29:27 +0800 Subject: [PATCH 08/21] [connectivity] support specifying flags in TestableNetworkCallback This is to allow specifying redaction flags in NetworkCallback for tests which requires the location permissions. The change is not used in this CL but its Child CL ag/34224157 Bug: 374037595 Flag: EXEMPT PURE_REFACTOR Change-Id: I143f8877ec62ff50ce4073727aad24e252a2af66 --- .../android/testutils/TestableNetworkCallback.kt | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkCallback.kt b/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkCallback.kt index 04efbe0378..3af33d4de2 100644 --- a/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkCallback.kt +++ b/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkCallback.kt @@ -17,6 +17,7 @@ package com.android.testutils import android.net.ConnectivityManager.NetworkCallback +import android.net.ConnectivityManager.NetworkCallback.FLAG_NONE import android.net.LinkProperties import android.net.LocalNetworkInfo import android.net.Network @@ -67,8 +68,9 @@ open class TestableNetworkCallback private constructor( val callbackTracker: TestableCallback = TestableCallback( defaultCallbackTimeoutMs, defaultCallbackNoEventTimeoutMs - ) -) : NetworkCallback(), Expectable by callbackTracker { + ), + private val flags: Int +) : NetworkCallback(flags), Expectable by callbackTracker { val mark get() = history.mark sealed class Event { @@ -223,19 +225,22 @@ open class TestableNetworkCallback private constructor( * make sure the loopers are drained before asserting no callback, since * one of them may cause a callback to be called. @see ConnectivityServiceTest * for such an example. + * @param flags the NetworkCallback#FLAG_* */ @JvmOverloads constructor( timeoutMs: Long = DEFAULT_TIMEOUT, noCallbackTimeoutMs: Long = DEFAULT_NO_CALLBACK_TIMEOUT, waiterFunc: Runnable = NOOP, // "() -> Unit" would forbid calling with a void func from Java - logTag: String = DEFAULT_TAG + logTag: String = DEFAULT_TAG, + flags: Int = FLAG_NONE, ) : this( timeoutMs, noCallbackTimeoutMs, waiterFunc, logTag, - TestableCallback(timeoutMs, noCallbackTimeoutMs) + TestableCallback(timeoutMs, noCallbackTimeoutMs), + flags ) fun createLinkedCopy() = TestableNetworkCallback( @@ -243,7 +248,8 @@ open class TestableNetworkCallback private constructor( defaultNoEventTimeoutMs, waiterFunc, logTag, - callbackTracker + callbackTracker, + flags ) // The last available network, or null if any network was lost since the last call to From ee24054e2b52280c05ee756af4eeaa8c2c44619b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Fri, 12 Sep 2025 08:19:40 -0700 Subject: [PATCH 09/21] KernelUtils.h: isAtLeastKernelVersion(x,y,z) - default 'z' to 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This seems to be the most common usecase. Test: TreeHugger Flag: EXEMPT PURE_REFACTOR Signed-off-by: Maciej Żenczykowski Change-Id: I2de4d067b2f20680539cbddb31555864bcf957b1 --- bpf/headers/BpfRingbufTest.cpp | 2 +- bpf/headers/include/bpf/BpfMap.h | 10 ++-- bpf/headers/include/bpf/KernelUtils.h | 4 +- bpf/loader/NetBpfLoad.cpp | 48 +++++++++---------- bpf/netd/BpfHandler.cpp | 28 +++++------ bpf/netd/BpfHandlerTest.cpp | 2 +- bpf/tests/mts/bpf_existence_test.cpp | 24 +++++----- .../libs/libnetworkstats/BpfNetworkStats.cpp | 2 +- .../com_android_net_module_util_Bpf.cpp | 2 +- ...net_module_util_ServiceConnectivityJni.cpp | 2 +- staticlibs/native/tcutils/tcutils.cpp | 2 +- .../native/tcutils/tests/tcutils_test.cpp | 4 +- .../connectivity_native_test.cpp | 2 +- 13 files changed, 66 insertions(+), 66 deletions(-) diff --git a/bpf/headers/BpfRingbufTest.cpp b/bpf/headers/BpfRingbufTest.cpp index e81fb9261c..2dac7edc5e 100644 --- a/bpf/headers/BpfRingbufTest.cpp +++ b/bpf/headers/BpfRingbufTest.cpp @@ -46,7 +46,7 @@ class BpfRingbufTest : public ::testing::Test { mRingbufPath("/sys/fs/bpf/map_bpfRingbufProg_test_ringbuf") {} void SetUp() { - if (!android::bpf::isAtLeastKernelVersion(5, 8, 0)) { + if (!android::bpf::isAtLeastKernelVersion(5, 8)) { GTEST_SKIP() << "BPF ring buffers not supported below 5.8"; } diff --git a/bpf/headers/include/bpf/BpfMap.h b/bpf/headers/include/bpf/BpfMap.h index 00b4477650..8ef564dbf1 100644 --- a/bpf/headers/include/bpf/BpfMap.h +++ b/bpf/headers/include/bpf/BpfMap.h @@ -107,7 +107,7 @@ class BpfMapRO { protected: void abortOnMismatch(bool writable) const { if (!mMapFd.ok()) Abort(errno, "mMapFd %d is not valid", mMapFd.get()); - if (isAtLeastKernelVersion(4, 14, 0)) { + if (isAtLeastKernelVersion(4, 14)) { int flags = bpfGetFdMapFlags(mMapFd); if (flags < 0) Abort(errno, "bpfGetFdMapFlags fail: flags=%d", flags); if (flags & BPF_F_WRONLY) Abort(0, "map is write-only (flags=0x%X)", flags); @@ -228,7 +228,7 @@ class BpfMapRO { // Does not allow early termination (via f erroring out) - may be implemented with bulk api Result forAll(const function &f) const { // No kernel bpfmap bulk lookup api which doesn't return both keys & values. - if (isAtLeastKernelVersion(5, 10, 0)) return doBulkLookupAndMaybeDelete(/*delete*/ false, + if (isAtLeastKernelVersion(5, 10)) return doBulkLookupAndMaybeDelete(/*delete*/ false, [&f](const Key &key, const Value &) { f(key); } @@ -258,7 +258,7 @@ class BpfMapRO { // Does not allow early termination (via f erroring out) - maybe implemented with bulk api Result forAll(const function &f) const { - if (isAtLeastKernelVersion(5, 10, 0)) return doBulkLookupAndMaybeDelete(/*delete*/ false, f); + if (isAtLeastKernelVersion(5, 10)) return doBulkLookupAndMaybeDelete(/*delete*/ false, f); return iterate( [&f](const Key &key, const Value &value) -> Result { f(key, value); @@ -379,7 +379,7 @@ class BpfMap : public BpfMapRW { } Result readAndDeleteValue(const Key& key) { - if (isAtLeastKernelVersion(5, 4, 0)) { + if (isAtLeastKernelVersion(5, 4)) { Value value; if (!findAndDeleteMapEntry(mMapFd, &key, &value)) return value; if (errno == ENOENT) return ERROR_FROM_ERRNO("read&DeleteVal"); @@ -423,7 +423,7 @@ class BpfMap : public BpfMapRW { // Does not allow early termination (via f erroring out) - maybe implemented with bulk api Result consume(const std::function& f) { - if (isAtLeastKernelVersion(5, 10, 0)) return doBulkLookupAndMaybeDelete(/*delete*/true, f); + if (isAtLeastKernelVersion(5, 10)) return doBulkLookupAndMaybeDelete(/*delete*/true, f); Result curKey = getFirstKey(); while (curKey.ok()) { const Result &nextKey = getNextKey(curKey.value()); diff --git a/bpf/headers/include/bpf/KernelUtils.h b/bpf/headers/include/bpf/KernelUtils.h index 8af0165d44..1f2b251751 100644 --- a/bpf/headers/include/bpf/KernelUtils.h +++ b/bpf/headers/include/bpf/KernelUtils.h @@ -81,14 +81,14 @@ static constexpr unsigned minSupportedKernelVer = #error "__ANDROID_API__ is pre-R" #endif -static inline bool isAtLeastKernelVersion(unsigned major, unsigned minor, unsigned sub) { +static inline bool isAtLeastKernelVersion(unsigned major, unsigned minor, unsigned sub = 0) { unsigned k = KVER(major, minor, sub); if (k <= minSupportedKernelVer) return true; return kernelVer >= k; } static inline bool isKernelVersion(unsigned major, unsigned minor) { - return isAtLeastKernelVersion(major, minor, 0) && !isAtLeastKernelVersion(major, minor + 1, 0); + return isAtLeastKernelVersion(major, minor) && !isAtLeastKernelVersion(major, minor + 1); } static inline bool __unused isLtsKernel() { diff --git a/bpf/loader/NetBpfLoad.cpp b/bpf/loader/NetBpfLoad.cpp index dda9c0db7c..a7ff9c2f5e 100644 --- a/bpf/loader/NetBpfLoad.cpp +++ b/bpf/loader/NetBpfLoad.cpp @@ -452,7 +452,7 @@ int readCodeSections(ElfObject& elfObj, vector& cs) { } static unsigned int sanitizeMapFlags(unsigned int map_flags) { - if (!isAtLeastKernelVersion(5, 10, 0)) map_flags &= ~BPF_F_MMAPABLE; + if (!isAtLeastKernelVersion(5, 10)) map_flags &= ~BPF_F_MMAPABLE; return map_flags; } @@ -464,7 +464,7 @@ static bool mapMatchesExpectations(const unique_fd& fd, // This is because the primary failure mode we're trying to detect here // is either a source code misconfiguration (which is likely kernel independent) // or a newly introduced kernel feature/bug (which is unlikely to get backported to 4.9). - if (!isAtLeastKernelVersion(4, 14, 0)) return true; + if (!isAtLeastKernelVersion(4, 14)) return true; // Assuming fd is a valid Bpf Map file descriptor then // all the following should always succeed on a 4.14+ kernel. @@ -642,7 +642,7 @@ static int loadBtf(ElfObject &elfObj, struct btf *btf) { if (ret) return ret; } - if (!isAtLeastKernelVersion(5, 10, 0)) { + if (!isAtLeastKernelVersion(5, 10)) { // Likely unnecessary on kernel 5.4 but untested. sanitizeBtf(btf); } @@ -778,7 +778,7 @@ static int pinMap(const borrowed_fd& fd, const struct bpf_map_def& mapDef) { return -err; } - if (isAtLeastKernelVersion(4, 14, 0)) { + if (isAtLeastKernelVersion(4, 14)) { int mapId = bpfGetFdMapId(fd); if (mapId == -1) { const int err = errno; @@ -791,7 +791,7 @@ static int pinMap(const borrowed_fd& fd, const struct bpf_map_def& mapDef) { } static bool isMapTypeSupported(enum bpf_map_type type) { - if (type == BPF_MAP_TYPE_LPM_TRIE && !isAtLeastKernelVersion(4, 14, 0)) { + if (type == BPF_MAP_TYPE_LPM_TRIE && !isAtLeastKernelVersion(4, 14)) { // On Linux Kernels older than 4.14 this map type doesn't exist - autoskip. return false; } @@ -799,7 +799,7 @@ static bool isMapTypeSupported(enum bpf_map_type type) { } static enum bpf_map_type sanitizeMapType(enum bpf_map_type type) { - if (type == BPF_MAP_TYPE_DEVMAP && !isAtLeastKernelVersion(4, 14, 0)) { + if (type == BPF_MAP_TYPE_DEVMAP && !isAtLeastKernelVersion(4, 14)) { // On Linux Kernels older than 4.14 this map type doesn't exist, but it can kind // of be approximated: ARRAY has the same userspace api, though it is not usable // by the same ebpf programs. However, that's okay because the bpf_redirect_map() @@ -809,7 +809,7 @@ static enum bpf_map_type sanitizeMapType(enum bpf_map_type type) { // Hence using an ARRAY instead of a DEVMAP simply makes life easier for userspace. return BPF_MAP_TYPE_ARRAY; } - if (type == BPF_MAP_TYPE_DEVMAP_HASH && !isAtLeastKernelVersion(5, 4, 0)) { + if (type == BPF_MAP_TYPE_DEVMAP_HASH && !isAtLeastKernelVersion(5, 4)) { // On Linux Kernels older than 5.4 this map type doesn't exist, but it can kind // of be approximated: HASH has the same userspace visible api. // However it cannot be used by ebpf programs in the same way. @@ -829,7 +829,7 @@ static int createMaps(ElfObject& elfObj, vector& md, vector< vector btfData; struct btf *btf = NULL; auto btfGuard = base::make_scope_guard([&btf] { if (btf) btf__free(btf); }); - if (isAtLeastKernelVersion(4, 19, 0)) { + if (isAtLeastKernelVersion(4, 19)) { // On Linux Kernels older than 4.18 BPF_BTF_LOAD command doesn't exist. ret = elfObj.readSectionByName(".BTF", btfData); if (ret) { @@ -906,7 +906,7 @@ static int createMaps(ElfObject& elfObj, vector& md, vector< .max_entries = max_entries, .map_flags = sanitizeMapFlags(md[i].map_flags), }; - if (isAtLeastKernelVersion(4, 15, 0)) + if (isAtLeastKernelVersion(4, 15)) strlcpy(req.map_name, md[i].name(), sizeof(req.map_name)); bool haveBtf = btf && isBtfSupported(type); @@ -1038,7 +1038,7 @@ static int pinProg(const borrowed_fd& fd, const struct bpf_prog_def& progDef) { static int validateProg(const borrowed_fd& fd, const char* const progPinLoc, const unsigned int bpfloader_ver) { - if (!isAtLeastKernelVersion(4, 14, 0)) { + if (!isAtLeastKernelVersion(4, 14)) { return 0; } int progId = bpfGetFdProgId(fd); @@ -1110,7 +1110,7 @@ static int loadCodeSections(ElfObject& elfObj, vector& cs, const st .log_buf = ptr_to_u64(log_buf), .expected_attach_type = cs[i].prog_def->attach_type, }; - if (isAtLeastKernelVersion(4, 15, 0)) + if (isAtLeastKernelVersion(4, 15)) strlcpy(req.prog_name, cs[i].prog_def->name(), sizeof(req.prog_name)); fd.reset(bpf(BPF_PROG_LOAD, req)); @@ -1317,7 +1317,7 @@ static int loadProgByLibbpf(const char* const elfPath, const unsigned int bpfloa ret = bpf_object__load(obj); if (ret) return ret; // On Linux Kernels older than 4.18 BPF_BTF_LOAD command doesn't exist. - if (isAtLeastKernelVersion(4, 19, 0) && bpf_object__btf_fd(obj) < 0) return -1; + if (isAtLeastKernelVersion(4, 19) && bpf_object__btf_fd(obj) < 0) return -1; ret = pinMaps(obj, md); if (ret) return ret; @@ -1630,41 +1630,41 @@ static int doLoad(char** argv, char * const envp[]) { // both S and T require kernel 4.9 (and eBpf support) // (this also guarantees 'kernelVer' isn't an invalid uninitialized 0) - if (!isAtLeastKernelVersion(4, 9, 0)) { + if (!isAtLeastKernelVersion(4, 9)) { ALOGE("Android S & T require kernel 4.9."); return 3; } // U bumps the kernel requirement up to 4.14 - if (isAtLeastU && !isAtLeastKernelVersion(4, 14, 0)) { + if (isAtLeastU && !isAtLeastKernelVersion(4, 14)) { ALOGE("Android U requires kernel 4.14."); return 4; } // V bumps the kernel requirement up to 4.19 // see also: //system/netd/tests/kernel_test.cpp TestKernel419 - if (isAtLeastV && !isAtLeastKernelVersion(4, 19, 0)) { + if (isAtLeastV && !isAtLeastKernelVersion(4, 19)) { ALOGE("Android V requires kernel 4.19."); return 5; } // 25Q2 bumps the kernel requirement up to 5.4 // see also: //system/netd/tests/kernel_test.cpp TestKernel54 - if (isAtLeast25Q2 && !isAtLeastKernelVersion(5, 4, 0)) { + if (isAtLeast25Q2 && !isAtLeastKernelVersion(5, 4)) { ALOGE("Android 25Q2 requires kernel 5.4."); return 6; } // 25Q4 bumps the kernel requirement up to 5.10 // see also: //system/netd/tests/kernel_test.cpp TestKernel510 - if (isAtLeast25Q4 && !isAtLeastKernelVersion(5, 10, 0)) { + if (isAtLeast25Q4 && !isAtLeastKernelVersion(5, 10)) { ALOGE("Android 25Q4 requires kernel 5.10."); return 7; } // Technically already required by U, but only enforce on V+ // see also: //system/netd/tests/kernel_test.cpp TestKernel64Bit - if (isAtLeastV && isKernel32Bit() && isAtLeastKernelVersion(5, 16, 0)) { + if (isAtLeastV && isKernel32Bit() && isAtLeastKernelVersion(5, 16)) { ALOGE("Android V+ platform with 32 bit kernel version >= 5.16.0 is unsupported"); if (!isTV()) return 8; } @@ -1675,7 +1675,7 @@ static int doLoad(char** argv, char * const envp[]) { } // 6.6 is highest version supported by Android V, so this is effectively W+ (sdk=36+) - if (isKernel32Bit() && isAtLeastKernelVersion(6, 7, 0)) { + if (isKernel32Bit() && isAtLeastKernelVersion(6, 7)) { ALOGE("Android platform with 32 bit kernel version >= 6.7.0 is unsupported"); return 10; } @@ -1740,7 +1740,7 @@ static int doLoad(char** argv, char * const envp[]) { * Note, however, that TV and Wear devices will continue to support 32-bit userspace * on ARM64. */ - if (isUserspace32bit() && isAtLeastKernelVersion(6, 2, 0)) { + if (isUserspace32bit() && isAtLeastKernelVersion(6, 2)) { // Stuff won't work reliably, but... if (isArm() && (isTV() || isWear())) { // exempt Arm TV or Wear devices (arm32 ABI is far less problematic than x86-32) @@ -1765,7 +1765,7 @@ static int doLoad(char** argv, char * const envp[]) { // Since officially Android only supports LTS, 6.13+ really means 6.18+, // and won't be supported before 2026, most likely Android 17 / 26Q2. // 6.13+ (implying 26Q2+) requires 64-bit userspace. - if (isUserspace32bit() && isAtLeastKernelVersion(6, 13, 0)) { + if (isUserspace32bit() && isAtLeastKernelVersion(6, 13)) { // due to previous check only reachable on Arm && (<=T kernel uprev || TV || Wear) ALOGE("64-bit userspace required on 6.13+ kernels."); return 14; @@ -1804,7 +1804,7 @@ static int doLoad(char** argv, char * const envp[]) { // (this writeFile is known to fail on at least 4.19, but always defaults to 0 on // pre-5.13, on 5.13+ it depends on CONFIG_BPF_UNPRIV_DEFAULT_OFF) if (!writeFile("/proc/sys/kernel/unprivileged_bpf_disabled", "0\n") && - isAtLeastKernelVersion(5, 13, 0)) return 23; + isAtLeastKernelVersion(5, 13)) return 23; } if (isAtLeastU) { @@ -1858,7 +1858,7 @@ static int doLoad(char** argv, char * const envp[]) { ALOGE("bpfGetNextMapId(zero) returned %u (errno %d)", mapId, errno); return 33; } - } else if (isAtLeastKernelVersion(4, 14, 0)) { // implies S through U QPR2 + } else if (isAtLeastKernelVersion(4, 14)) { // implies S through U QPR2 // bpfGetNext{Prog,Map}Id require 4.14+ // furthermore since we're not running as root, we're not the initial // platform bpfloader, so there may already be some maps & programs. @@ -1883,7 +1883,7 @@ static int doLoad(char** argv, char * const envp[]) { // and thus it should return 0 with errno == ENOENT. ALOGE("bpfGetNextMapId(final %d) returned %d errno %d", mapId, next, errno); if (next || errno != ENOENT) return 35; - if (isAtLeastT || isAtLeastKernelVersion(4, 20, 0)) return 36; + if (isAtLeastT || isAtLeastKernelVersion(4, 20)) return 36; // implies Android S with 4.14 or 4.19 kernel ALOGW("Detected kernel with invalid BPF UAPI - disabling mainline use of eBPF."); // leave a flag that we're 'done' diff --git a/bpf/netd/BpfHandler.cpp b/bpf/netd/BpfHandler.cpp index c073d0dfb4..7367ec5e97 100644 --- a/bpf/netd/BpfHandler.cpp +++ b/bpf/netd/BpfHandler.cpp @@ -82,12 +82,12 @@ static Status initPrograms(const char* cg2_path) { if (!isAtLeastT) return Status("S- platform is unsupported"); // S requires eBPF support which was only added in 4.9, so this should be satisfied. - if (!isAtLeastKernelVersion(4, 9, 0)) { + if (!isAtLeastKernelVersion(4, 9)) { return Status("kernel version < 4.9.0 is unsupported"); } // U bumps the kernel requirement up to 4.14 - if (isAtLeastU && !isAtLeastKernelVersion(4, 14, 0)) { + if (isAtLeastU && !isAtLeastKernelVersion(4, 14)) { return Status("U+ platform with kernel version < 4.14.0 is unsupported"); } @@ -97,12 +97,12 @@ static Status initPrograms(const char* cg2_path) { } // V bumps the kernel requirement up to 4.19 - if (isAtLeastV && !isAtLeastKernelVersion(4, 19, 0)) { + if (isAtLeastV && !isAtLeastKernelVersion(4, 19)) { return Status("V+ platform with kernel version < 4.19.0 is unsupported"); } // 25Q2 bumps the kernel requirement up to 5.4 - if (isAtLeast25Q2 && !isAtLeastKernelVersion(5, 4, 0)) { + if (isAtLeast25Q2 && !isAtLeastKernelVersion(5, 4)) { return Status("25Q2+ platform with kernel version < 5.4.0 is unsupported"); } @@ -121,12 +121,12 @@ static Status initPrograms(const char* cg2_path) { // cgroup if the program is pinned properly. // TODO: delete the if statement once all devices should support cgroup // socket filter (ie. the minimum kernel version required is 4.14). - if (isAtLeastKernelVersion(4, 14, 0)) { + if (isAtLeastKernelVersion(4, 14)) { RETURN_IF_NOT_OK(attachProgramToCgroup(CGROUP_INET_CREATE_PROG_PATH, cg_fd, BPF_CGROUP_INET_SOCK_CREATE)); } - if (isAtLeastKernelVersion(5, 10, 0)) { + if (isAtLeastKernelVersion(5, 10)) { RETURN_IF_NOT_OK(attachProgramToCgroup(CGROUP_INET_RELEASE_PROG_PATH, cg_fd, BPF_CGROUP_INET_SOCK_RELEASE)); } @@ -134,7 +134,7 @@ static Status initPrograms(const char* cg2_path) { if (isAtLeastV) { // V requires 4.19+, so technically this 2nd 'if' is not required, but it // doesn't hurt us to try to support AOSP forks that try to support older kernels. - if (isAtLeastKernelVersion(4, 19, 0)) { + if (isAtLeastKernelVersion(4, 19)) { RETURN_IF_NOT_OK(attachProgramToCgroup(CGROUP_CONNECT4_PROG_PATH, cg_fd, BPF_CGROUP_INET4_CONNECT)); RETURN_IF_NOT_OK(attachProgramToCgroup(CGROUP_CONNECT6_PROG_PATH, @@ -149,7 +149,7 @@ static Status initPrograms(const char* cg2_path) { cg_fd, BPF_CGROUP_UDP6_SENDMSG)); } - if (isAtLeastKernelVersion(5, 4, 0)) { + if (isAtLeastKernelVersion(5, 4)) { RETURN_IF_NOT_OK(attachProgramToCgroup(CGROUP_GETSOCKOPT_PROG_PATH, cg_fd, BPF_CGROUP_GETSOCKOPT)); RETURN_IF_NOT_OK(attachProgramToCgroup(CGROUP_SETSOCKOPT_PROG_PATH, @@ -157,7 +157,7 @@ static Status initPrograms(const char* cg2_path) { } } - if (isAtLeastKernelVersion(4, 19, 0)) { + if (isAtLeastKernelVersion(4, 19)) { RETURN_IF_NOT_OK(attachProgramToCgroup(CGROUP_BIND4_PROG_PATH, cg_fd, BPF_CGROUP_INET4_BIND)); RETURN_IF_NOT_OK(attachProgramToCgroup(CGROUP_BIND6_PROG_PATH, @@ -172,14 +172,14 @@ static Status initPrograms(const char* cg2_path) { if (queryProgram(cg_fd, BPF_CGROUP_INET6_BIND) <= 0) abort(); } - if (isAtLeastKernelVersion(5, 10, 0)) { + if (isAtLeastKernelVersion(5, 10)) { if (queryProgram(cg_fd, BPF_CGROUP_INET_SOCK_RELEASE) <= 0) abort(); } if (isAtLeastV) { // V requires 4.19+, so technically this 2nd 'if' is not required, but it // doesn't hurt us to try to support AOSP forks that try to support older kernels. - if (isAtLeastKernelVersion(4, 19, 0)) { + if (isAtLeastKernelVersion(4, 19)) { if (queryProgram(cg_fd, BPF_CGROUP_INET4_CONNECT) <= 0) abort(); if (queryProgram(cg_fd, BPF_CGROUP_INET6_CONNECT) <= 0) abort(); if (queryProgram(cg_fd, BPF_CGROUP_UDP4_RECVMSG) <= 0) abort(); @@ -188,7 +188,7 @@ static Status initPrograms(const char* cg2_path) { if (queryProgram(cg_fd, BPF_CGROUP_UDP6_SENDMSG) <= 0) abort(); } - if (isAtLeastKernelVersion(5, 4, 0)) { + if (isAtLeastKernelVersion(5, 4)) { if (queryProgram(cg_fd, BPF_CGROUP_GETSOCKOPT) <= 0) abort(); if (queryProgram(cg_fd, BPF_CGROUP_SETSOCKOPT) <= 0) abort(); } @@ -319,7 +319,7 @@ static void mapLockTest(void) { Status BpfHandler::initMaps() { // bpfLock() requires bpfGetFdMapId which is only available on 4.14+ kernels. - if (isAtLeastKernelVersion(4, 14, 0)) { + if (isAtLeastKernelVersion(4, 14)) { mapLockTest(); } @@ -378,7 +378,7 @@ int BpfHandler::tagSocket(int sockFd, uint32_t tag, uid_t chargeUid, uid_t realU // On 5.10+ the BPF_CGROUP_INET_SOCK_RELEASE hook takes care of cookie tag map cleanup // during socket destruction. As such the socket destroy listener is superfluous. - if (!isAtLeastKernelVersion(5, 10, 0)) { + if (!isAtLeastKernelVersion(5, 10)) { int socketProto; socklen_t protoLen = sizeof(socketProto); if (getsockopt(sockFd, SOL_SOCKET, SO_PROTOCOL, &socketProto, &protoLen)) { diff --git a/bpf/netd/BpfHandlerTest.cpp b/bpf/netd/BpfHandlerTest.cpp index 4002b4c7b6..c537806f20 100644 --- a/bpf/netd/BpfHandlerTest.cpp +++ b/bpf/netd/BpfHandlerTest.cpp @@ -191,7 +191,7 @@ TEST_F(BpfHandlerTest, TestTagSocketWithUnsupportedProtocol) { int rawSocket = socket(AF_INET, SOCK_RAW | SOCK_CLOEXEC, IPPROTO_RAW); EXPECT_LE(0, rawSocket); EXPECT_NE(NONEXISTENT_COOKIE, getSocketCookie(rawSocket)); - if (isAtLeastKernelVersion(5, 10, 0)) { + if (isAtLeastKernelVersion(5, 10)) { EXPECT_EQ(0, mBh.tagSocket(rawSocket, TEST_TAG, TEST_UID, TEST_UID)); } else { EXPECT_EQ(-EPROTONOSUPPORT, mBh.tagSocket(rawSocket, TEST_TAG, TEST_UID, TEST_UID)); diff --git a/bpf/tests/mts/bpf_existence_test.cpp b/bpf/tests/mts/bpf_existence_test.cpp index 2ab155a676..6849dd414c 100644 --- a/bpf/tests/mts/bpf_existence_test.cpp +++ b/bpf/tests/mts/bpf_existence_test.cpp @@ -203,7 +203,7 @@ TEST_F(BpfExistenceTest, TestPrograms) { ASSERT_TRUE(isAtLeastS); // Q & R are no longer supported by mainline // S requires Linux Kernel 4.9+ and thus requires eBPF support. - if (isAtLeastS) ASSERT_TRUE(isAtLeastKernelVersion(4, 9, 0)); + if (isAtLeastS) ASSERT_TRUE(isAtLeastKernelVersion(4, 9)); // on S without a new enough DnsResolver apex, NetBpfLoad doesn't get triggered, // and thus no mainline programs get loaded. @@ -215,28 +215,28 @@ TEST_F(BpfExistenceTest, TestPrograms) { // T still only requires Linux Kernel 4.9+. DO_EXPECT(isAtLeastT, MAINLINE_FOR_T_PLUS); - DO_EXPECT(isAtLeastT && isAtLeastKernelVersion(4, 14, 0), MAINLINE_FOR_T_4_14_PLUS); - DO_EXPECT(isAtLeastT && isAtLeastKernelVersion(4, 19, 0), MAINLINE_FOR_T_4_19_PLUS); - DO_EXPECT(isAtLeastT && isAtLeastKernelVersion(5, 10, 0), MAINLINE_FOR_T_5_10_PLUS); - DO_EXPECT(isAtLeastT && isAtLeastKernelVersion(5, 15, 0), MAINLINE_FOR_T_5_15_PLUS); + DO_EXPECT(isAtLeastT && isAtLeastKernelVersion(4, 14), MAINLINE_FOR_T_4_14_PLUS); + DO_EXPECT(isAtLeastT && isAtLeastKernelVersion(4, 19), MAINLINE_FOR_T_4_19_PLUS); + DO_EXPECT(isAtLeastT && isAtLeastKernelVersion(5, 10), MAINLINE_FOR_T_5_10_PLUS); + DO_EXPECT(isAtLeastT && isAtLeastKernelVersion(5, 15), MAINLINE_FOR_T_5_15_PLUS); // U requires Linux Kernel 4.14+, but nothing (as yet) added or removed in U. - if (isAtLeastU) ASSERT_TRUE(isAtLeastKernelVersion(4, 14, 0)); + if (isAtLeastU) ASSERT_TRUE(isAtLeastKernelVersion(4, 14)); DO_EXPECT(isAtLeastU, MAINLINE_FOR_U_PLUS); - DO_EXPECT(isAtLeastU && isAtLeastKernelVersion(5, 10, 0), MAINLINE_FOR_U_5_10_PLUS); + DO_EXPECT(isAtLeastU && isAtLeastKernelVersion(5, 10), MAINLINE_FOR_U_5_10_PLUS); // V requires Linux Kernel 4.19+, but nothing (as yet) added or removed in V. - if (isAtLeastV) ASSERT_TRUE(isAtLeastKernelVersion(4, 19, 0)); + if (isAtLeastV) ASSERT_TRUE(isAtLeastKernelVersion(4, 19)); DO_EXPECT(isAtLeastV, MAINLINE_FOR_V_PLUS); - DO_EXPECT(isAtLeastV && isAtLeastKernelVersion(5, 4, 0), MAINLINE_FOR_V_5_4_PLUS); + DO_EXPECT(isAtLeastV && isAtLeastKernelVersion(5, 4), MAINLINE_FOR_V_5_4_PLUS); - if (isAtLeast25Q2) ASSERT_TRUE(isAtLeastKernelVersion(5, 4, 0)); + if (isAtLeast25Q2) ASSERT_TRUE(isAtLeastKernelVersion(5, 4)); DO_EXPECT(isAtLeast25Q2, MAINLINE_FOR_25Q2_PLUS); - if (isAtLeast25Q4) ASSERT_TRUE(isAtLeastKernelVersion(5, 10, 0)); + if (isAtLeast25Q4) ASSERT_TRUE(isAtLeastKernelVersion(5, 10)); DO_EXPECT(isAtLeast26Q2, MAINLINE_FOR_26Q2_PLUS); - DO_EXPECT(isAtLeast26Q2 && isAtLeastKernelVersion(6, 1, 0), MAINLINE_FOR_26Q2_6_1_PLUS); + DO_EXPECT(isAtLeast26Q2 && isAtLeastKernelVersion(6, 1), MAINLINE_FOR_26Q2_6_1_PLUS); for (const auto& file : mustExist) { EXPECT_EQ(0, access(file.c_str(), R_OK)) << file << " does not exist"; diff --git a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp index 52499acf77..7d8d8ec8f0 100644 --- a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp +++ b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp @@ -118,7 +118,7 @@ int bpfGetIfaceStatsInternal(const char* iface, StatsValue* stats, // specific iface int64_t unknownIfaceBytesTotal = 0; - if (isAtLeastKernelVersion(5, 10, 0)) { + if (isAtLeastKernelVersion(5, 10)) { // On 5.10+ bulk lookup api returns values for free, so just use forAll(f(K,V)) auto res = ifaceStatsMap.forAll( [iface, stats, ifindex2name, &unknownIfaceBytesTotal, &ifaceStatsMap](const uint32_t& key, const StatsValue& value) { diff --git a/staticlibs/native/bpfmapjni/com_android_net_module_util_Bpf.cpp b/staticlibs/native/bpfmapjni/com_android_net_module_util_Bpf.cpp index 9e7afe1bc0..055caa25b8 100644 --- a/staticlibs/native/bpfmapjni/com_android_net_module_util_Bpf.cpp +++ b/staticlibs/native/bpfmapjni/com_android_net_module_util_Bpf.cpp @@ -61,7 +61,7 @@ static jint com_android_net_module_util_BpfMap_nativeBpfFdGet(JNIEnv *env, jclas return -1; } - if (bpf::isAtLeastKernelVersion(4, 14, 0)) { + if (bpf::isAtLeastKernelVersion(4, 14)) { // These likely fail with -1 and set errno to EINVAL on <4.14 if (bpf::bpfGetFdKeySize(fd) != keySize) { jniThrowErrnoException(env, "nativeBpfFdGet KeySize", EBADFD); diff --git a/staticlibs/native/serviceconnectivityjni/com_android_net_module_util_ServiceConnectivityJni.cpp b/staticlibs/native/serviceconnectivityjni/com_android_net_module_util_ServiceConnectivityJni.cpp index 3f152ef33f..14cc51c006 100644 --- a/staticlibs/native/serviceconnectivityjni/com_android_net_module_util_ServiceConnectivityJni.cpp +++ b/staticlibs/native/serviceconnectivityjni/com_android_net_module_util_ServiceConnectivityJni.cpp @@ -104,7 +104,7 @@ static int createTunTapImpl(JNIEnv *env, bool isTun, bool hasCarrier, if (!hasCarrier) { // Using IFF_NO_CARRIER is supported starting in kernel version >= 6.0 // Up until then, unsupported flags are ignored. - if (!bpf::isAtLeastKernelVersion(6, 0, 0)) { + if (!bpf::isAtLeastKernelVersion(6, 0)) { throwException(env, EOPNOTSUPP, "IFF_NO_CARRIER not supported", ifr.ifr_name); return -1; diff --git a/staticlibs/native/tcutils/tcutils.cpp b/staticlibs/native/tcutils/tcutils.cpp index 5425d0e4e5..320dbe046b 100644 --- a/staticlibs/native/tcutils/tcutils.cpp +++ b/staticlibs/native/tcutils/tcutils.cpp @@ -497,7 +497,7 @@ int isEthernet(const char *iface, bool &isEthernet) { // shipped with Android S, so (for safety) let's limit ourselves to // >5.10, ie. 5.11+ as a guarantee we're on Android T+ and thus no // longer need this non-upstream compatibility logic - static bool is_pre_5_11_kernel = !bpf::isAtLeastKernelVersion(5, 11, 0); + static bool is_pre_5_11_kernel = !bpf::isAtLeastKernelVersion(5, 11); if (is_pre_5_11_kernel) return false; } diff --git a/staticlibs/native/tcutils/tests/tcutils_test.cpp b/staticlibs/native/tcutils/tests/tcutils_test.cpp index 77322479b2..c4a567d9fe 100644 --- a/staticlibs/native/tcutils/tests/tcutils_test.cpp +++ b/staticlibs/native/tcutils/tests/tcutils_test.cpp @@ -82,7 +82,7 @@ TEST(LibTcUtilsTest, AddAndDeleteBpfFilter) { // TODO: this should likely be in the tethering module, where using netd.h would be ok static constexpr char bpfProgPath[] = "/sys/fs/bpf/tethering/prog_offload_schedcls_tether_downstream6_ether"; - const int errNOENT = bpf::isAtLeastKernelVersion(4, 19, 0) ? ENOENT : EINVAL; + const int errNOENT = bpf::isAtLeastKernelVersion(4, 19) ? ENOENT : EINVAL; // static test values static constexpr bool ingress = true; @@ -118,7 +118,7 @@ TEST(LibTcUtilsTest, AddAndDeleteIngressPoliceFilter) { ASSERT_LE(3, fd); close(fd); - const int errNOENT = bpf::isAtLeastKernelVersion(4, 19, 0) ? ENOENT : EINVAL; + const int errNOENT = bpf::isAtLeastKernelVersion(4, 19) ? ENOENT : EINVAL; // static test values static constexpr unsigned rateInBytesPerSec = diff --git a/tests/native/connectivity_native_test/connectivity_native_test.cpp b/tests/native/connectivity_native_test/connectivity_native_test.cpp index faaf150f12..b264b0438c 100644 --- a/tests/native/connectivity_native_test/connectivity_native_test.cpp +++ b/tests/native/connectivity_native_test/connectivity_native_test.cpp @@ -39,7 +39,7 @@ class ConnectivityNativeBinderTest : public ::testing::Test { GTEST_SKIP() << "Should be at least U device."; // Skip test case if not on 5.4 kernel which is required by bpf prog. - if (!android::bpf::isAtLeastKernelVersion(5, 4, 0)) + if (!android::bpf::isAtLeastKernelVersion(5, 4)) GTEST_SKIP() << "Kernel should be at least 5.4."; // If there are already ports being blocked on device store the From c2667777e15137912e2f4898fa592675c342c3fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Fri, 12 Sep 2025 11:15:08 -0700 Subject: [PATCH 10/21] BpfNetworkStats: apparently RiscV isn't actually a 64-bit architecture. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I'll follow up with some sort of VTS test to block RiscV until the compiler/language/libraries etc can support something like CMPXCHG16B or ldp Bug: 444697997 Flag: EXEMPT BUGFIX Signed-off-by: Maciej Żenczykowski Change-Id: I553cffe3c4181842c38baf37258262e7b43b9ca4 --- service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp index 52499acf77..389b936ed1 100644 --- a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp +++ b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp @@ -50,10 +50,12 @@ const BpfMapRO& getIfaceStatsMap() { return ifaceStatsMap; } +#ifndef __riscv #ifndef __ILP32__ static_assert(std::atomic::is_always_lock_free, "Error: 16-byte IfaceValue is not lock-free on this platform!"); #endif +#endif static constexpr unsigned cacheSize = 1024; // see 'iface_index_name_map' bpf map size static IfaceValue cache[cacheSize]; From 48eb28f7eecdf7cc38003757f78e469a6f1b81bb Mon Sep 17 00:00:00 2001 From: Pedro Loureiro Date: Fri, 12 Sep 2025 12:20:36 +0000 Subject: [PATCH 11/21] Bump the min sdk of Tethering This is required because we are bumping the min sdk of NetworkStack which this depends on. Test: build locally + aapt2 dump badging Bug: 444168252 Flag: build.RELEASE_DEPRECATE_MAINLINE_R_SUPPORT Change-Id: Idbff03cdcca9955a4759c19866895f755abedaaf --- Tethering/Android.bp | 5 ++++- Tethering/tests/privileged/Android.bp | 5 ++++- Tethering/tests/unit/Android.bp | 5 ++++- tests/common/Android.bp | 11 ++++++++--- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/Tethering/Android.bp b/Tethering/Android.bp index ad6b28c36a..2ffae71763 100644 --- a/Tethering/Android.bp +++ b/Tethering/Android.bp @@ -22,7 +22,10 @@ package { java_defaults { name: "TetheringApiLevel", sdk_version: "module_current", - min_sdk_version: "30", + min_sdk_version: select(release_flag("RELEASE_DEPRECATE_MAINLINE_R_SUPPORT"), { + true: "31", + default: "30", + }), } java_defaults { diff --git a/Tethering/tests/privileged/Android.bp b/Tethering/tests/privileged/Android.bp index 3597a91546..76b1521bf8 100644 --- a/Tethering/tests/privileged/Android.bp +++ b/Tethering/tests/privileged/Android.bp @@ -53,5 +53,8 @@ android_test { "TetheringApiCurrentLib", ], compile_multilib: "both", - min_sdk_version: "30", + min_sdk_version: select(release_flag("RELEASE_DEPRECATE_MAINLINE_R_SUPPORT"), { + true: "31", + default: "30", + }), } diff --git a/Tethering/tests/unit/Android.bp b/Tethering/tests/unit/Android.bp index c2826186e3..de4c176576 100644 --- a/Tethering/tests/unit/Android.bp +++ b/Tethering/tests/unit/Android.bp @@ -45,7 +45,10 @@ java_library { java_defaults { name: "TetheringTestsDefaults", - min_sdk_version: "30", + min_sdk_version: select(release_flag("RELEASE_DEPRECATE_MAINLINE_R_SUPPORT"), { + true: "31", + default: "30", + }), srcs: [ "src/**/*.java", "src/**/*.kt", diff --git a/tests/common/Android.bp b/tests/common/Android.bp index 420cd2bb0f..49e4e8251d 100644 --- a/tests/common/Android.bp +++ b/tests/common/Android.bp @@ -77,7 +77,10 @@ java_genrule { android_library { name: "ConnectivityCoverageTestsLib", - min_sdk_version: "30", + min_sdk_version: select(release_flag("RELEASE_DEPRECATE_MAINLINE_R_SUPPORT"), { + true: "31", + default: "30", + }), static_libs: [ "ConnectivityUnitTestsLib", "NetdStaticLibTestsLib", @@ -93,8 +96,10 @@ android_library { android_test { name: "ConnectivityCoverageTests", - // Tethering started on SDK 30 - min_sdk_version: "30", + min_sdk_version: select(release_flag("RELEASE_DEPRECATE_MAINLINE_R_SUPPORT"), { + true: "31", + default: "30", + }), test_suites: [ "general-tests", "mts-tethering", From 81a472014a3b4b640134e19808899491797496f6 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Thu, 11 Sep 2025 11:47:56 -0700 Subject: [PATCH 12/21] Reduce log level for BluetoothSocket errors This exception is also logged when the other side disconnects gracefully (because there is no way to distinguish this case using the Bluetooth APIs). This change reduces the log level to a warning because it is almost always a red herring. Test: TH Flag: NONE yes Change-Id: I94f20be91ff8481fdd1a50fbbc27200c57885426 --- service/src/com/android/server/net/L2capPacketForwarder.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/service/src/com/android/server/net/L2capPacketForwarder.java b/service/src/com/android/server/net/L2capPacketForwarder.java index a2a04b16d1..d6072d3ae2 100644 --- a/service/src/com/android/server/net/L2capPacketForwarder.java +++ b/service/src/com/android/server/net/L2capPacketForwarder.java @@ -227,7 +227,10 @@ public class L2capPacketForwarder { mWriteFd.write(mBuffer, 0 /*off*/, readBytes); } catch (IOException|BufferUnderflowException e) { - Log.e(mLogTag, "L2capThread exception", e); + // This exception almost always indicates that the other side disconnected. + // Unfortunately, there is no way to distinguish between an EOF triggered by + // some disconnect or some actual error. + Log.w(mLogTag, "L2capThread exception", e); // Tear down the network on any error. mIsRunning = false; // Notify provider that forwarding has stopped. From 01640f15a4b22f8c3a1a3d5638aca4691bb4baf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Fri, 12 Sep 2025 09:19:43 -0700 Subject: [PATCH 13/21] bpf: move BPFLOADER_MAINLINE_S_VERSION into bpf_map_def.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit also while at it renumber things so that in the future we can autoderive the version from full api level. Test: TreeHugger Flag: EXEMPT PURE_REFACTOR Signed-off-by: Maciej Żenczykowski Change-Id: Ie12638a3dcbde9db3e50d1736efa37154b22341c --- bpf/loader/NetBpfLoad.cpp | 24 ++++++++++-------------- bpf/progs/include/bpf_helpers.h | 31 ------------------------------- bpf/progs/include/bpf_map_def.h | 12 ++++++++++++ 3 files changed, 22 insertions(+), 45 deletions(-) diff --git a/bpf/loader/NetBpfLoad.cpp b/bpf/loader/NetBpfLoad.cpp index a7ff9c2f5e..348733004c 100644 --- a/bpf/loader/NetBpfLoad.cpp +++ b/bpf/loader/NetBpfLoad.cpp @@ -66,10 +66,6 @@ #include "bpf/BpfUtils.h" #include "bpf_map_def.h" -// The following matches bpf_helpers.h, which is only for inclusion in bpf code -#define BPFLOADER_MAINLINE_S_VERSION 42u -#define BPFLOADER_MAINLINE_25Q2_VERSION 47u - using android::base::borrowed_fd; using android::base::EndsWith; using android::base::GetIntProperty; @@ -1598,16 +1594,16 @@ static int doLoad(char** argv, char * const envp[]) { const bool has_platform_netbpfload_rc = exists("/system/etc/init/netbpfload.rc"); // Version of Network BpfLoader depends on the Android OS version - unsigned int bpfloader_ver = BPFLOADER_MAINLINE_S_VERSION; // [42u] - if (isAtLeastT) ++bpfloader_ver; // [43] BPFLOADER_MAINLINE_T_VERSION - if (isAtLeastU) ++bpfloader_ver; // [44] BPFLOADER_MAINLINE_U_VERSION - if (runningAsRoot) ++bpfloader_ver; // [45] BPFLOADER_MAINLINE_U_QPR3_VERSION - if (isAtLeastV) ++bpfloader_ver; // [46] BPFLOADER_MAINLINE_V_VERSION - if (isAtLeast25Q2) ++bpfloader_ver; // [47] BPFLOADER_MAINLINE_25Q2_VERSION - if (isAtLeast25Q3) ++bpfloader_ver; // [48] BPFLOADER_MAINLINE_25Q3_VERSION - if (isAtLeast25Q4) ++bpfloader_ver; // [49] BPFLOADER_MAINLINE_25Q4_VERSION - if (isAtLeast26Q1) ++bpfloader_ver; // [50] BPFLOADER_MAINLINE_26Q1_VERSION - if (isAtLeast26Q2) ++bpfloader_ver; // [51] BPFLOADER_MAINLINE_26Q2_VERSION + unsigned int bpfloader_ver = BPFLOADER_MAINLINE_S_VERSION; + if (isAtLeastT) bpfloader_ver = BPFLOADER_MAINLINE_T_VERSION; + if (isAtLeastU) bpfloader_ver = BPFLOADER_MAINLINE_U_VERSION; + if (runningAsRoot) bpfloader_ver = BPFLOADER_MAINLINE_U_QPR3_VERSION; + if (isAtLeastV) bpfloader_ver = BPFLOADER_MAINLINE_V_VERSION; + if (isAtLeast25Q2) bpfloader_ver = BPFLOADER_MAINLINE_25Q2_VERSION; + if (isAtLeast25Q3) bpfloader_ver = BPFLOADER_MAINLINE_25Q3_VERSION; + if (isAtLeast25Q4) bpfloader_ver = BPFLOADER_MAINLINE_25Q4_VERSION; + if (isAtLeast26Q1) bpfloader_ver = BPFLOADER_MAINLINE_26Q1_VERSION; + if (isAtLeast26Q2) bpfloader_ver = BPFLOADER_MAINLINE_26Q2_VERSION; ALOGI("NetBpfLoad v0.%u (%s) api:%d/%d kver:%07x (%s) libbpf: v%u.%u " "uid:%d rc:%d%d", diff --git a/bpf/progs/include/bpf_helpers.h b/bpf/progs/include/bpf_helpers.h index a08b0c08b4..9a7dbd701f 100644 --- a/bpf/progs/include/bpf_helpers.h +++ b/bpf/progs/include/bpf_helpers.h @@ -6,37 +6,6 @@ #include "bpf_map_def.h" -// Android Mainline BpfLoader when running on Android S (sdk=31) -// Note: this value (and the following +1u's) are hardcoded in NetBpfLoad.cpp -#define BPFLOADER_MAINLINE_S_VERSION 42u - -// Android Mainline BpfLoader when running on Android T (sdk=33) -#define BPFLOADER_MAINLINE_T_VERSION (BPFLOADER_MAINLINE_S_VERSION + 1u) - -// Android Mainline BpfLoader when running on Android U (sdk=34) -#define BPFLOADER_MAINLINE_U_VERSION (BPFLOADER_MAINLINE_T_VERSION + 1u) - -// Android Mainline BpfLoader when running on Android U QPR3 -#define BPFLOADER_MAINLINE_U_QPR3_VERSION (BPFLOADER_MAINLINE_U_VERSION + 1u) - -// Android Mainline BpfLoader when running on Android V (sdk=35) -#define BPFLOADER_MAINLINE_V_VERSION (BPFLOADER_MAINLINE_U_QPR3_VERSION + 1u) - -// Android Mainline BpfLoader when running on Android 25Q2 (sdk=36 aka 36.0) -#define BPFLOADER_MAINLINE_25Q2_VERSION (BPFLOADER_MAINLINE_V_VERSION + 1u) - -// Android Mainline BpfLoader when running on Android 25Q3 (sdk 36.0+) -#define BPFLOADER_MAINLINE_25Q3_VERSION (BPFLOADER_MAINLINE_25Q2_VERSION + 1u) - -// Android Mainline BpfLoader when running on Android 25Q4 (sdk 36.1) -#define BPFLOADER_MAINLINE_25Q4_VERSION (BPFLOADER_MAINLINE_25Q3_VERSION + 1u) - -// Android Mainline BpfLoader when running on Android 26Q1 (sdk 36.1+) -#define BPFLOADER_MAINLINE_26Q1_VERSION (BPFLOADER_MAINLINE_25Q4_VERSION + 1u) - -// Android Mainline BpfLoader when running on Android 26Q2 (sdk 37.0) -#define BPFLOADER_MAINLINE_26Q2_VERSION (BPFLOADER_MAINLINE_26Q1_VERSION + 1u) - /* You should #define BPFLOADER_{MIN/MAX}_VER before #include "bpf_helpers.h" * to change which bpfloaders will process the resulting .o file. */ diff --git a/bpf/progs/include/bpf_map_def.h b/bpf/progs/include/bpf_map_def.h index e4ab6917b3..1fd2c1e436 100644 --- a/bpf/progs/include/bpf_map_def.h +++ b/bpf/progs/include/bpf_map_def.h @@ -203,3 +203,15 @@ static_assert(std::is_standard_layout_v); _Static_assert(sizeof(struct bpf_prog_def) == 40 + 2 * BPF_DEF_CHAR_ARRAY_SIZE, "wrong sizeof struct bpf_prog_def"); _Static_assert(__alignof__(struct bpf_prog_def) == 4, "__alignof__ struct bpf_prog_def != 4"); _Static_assert(_Alignof(struct bpf_prog_def) == 4, "_Alignof struct bpf_prog_def != 4"); + +// Android Mainline BpfLoader version when running on: +#define BPFLOADER_MAINLINE_S_VERSION 3100u // Android S (31) +#define BPFLOADER_MAINLINE_T_VERSION 3300u // Android T (33) +#define BPFLOADER_MAINLINE_U_VERSION 3400u // Android U (34) +#define BPFLOADER_MAINLINE_U_QPR3_VERSION 3403u // Android U QPR3 +#define BPFLOADER_MAINLINE_V_VERSION 3500u // Android V (35) +#define BPFLOADER_MAINLINE_25Q2_VERSION 3600u // Android 25Q2 (36.0) +#define BPFLOADER_MAINLINE_25Q3_VERSION 3601u // Android 25Q3 (36.0+) +#define BPFLOADER_MAINLINE_25Q4_VERSION 3610u // Android 25Q4 (36.1) +#define BPFLOADER_MAINLINE_26Q1_VERSION 3611u // Android 26Q1 (36.1+) +#define BPFLOADER_MAINLINE_26Q2_VERSION 3700u // Android 26Q2 (37.0) From 867eb61b9f4014566d8e50af567cf264c5dabe16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Fri, 12 Sep 2025 10:16:14 -0700 Subject: [PATCH 14/21] bpf: remove BPFLOADER_MAINLINE_U_QPR3_VERSION MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is not used, and it seems unlikely we would ever want to differentiate based on this. Test: TreeHugger Flag: EXEMPT PURE_REFACTOR Signed-off-by: Maciej Żenczykowski Change-Id: I1ab48e0e4dc7a8979a34509ebc3d901a0bce3840 --- bpf/loader/NetBpfLoad.cpp | 1 - bpf/progs/include/bpf_map_def.h | 1 - 2 files changed, 2 deletions(-) diff --git a/bpf/loader/NetBpfLoad.cpp b/bpf/loader/NetBpfLoad.cpp index 348733004c..26b3da339b 100644 --- a/bpf/loader/NetBpfLoad.cpp +++ b/bpf/loader/NetBpfLoad.cpp @@ -1597,7 +1597,6 @@ static int doLoad(char** argv, char * const envp[]) { unsigned int bpfloader_ver = BPFLOADER_MAINLINE_S_VERSION; if (isAtLeastT) bpfloader_ver = BPFLOADER_MAINLINE_T_VERSION; if (isAtLeastU) bpfloader_ver = BPFLOADER_MAINLINE_U_VERSION; - if (runningAsRoot) bpfloader_ver = BPFLOADER_MAINLINE_U_QPR3_VERSION; if (isAtLeastV) bpfloader_ver = BPFLOADER_MAINLINE_V_VERSION; if (isAtLeast25Q2) bpfloader_ver = BPFLOADER_MAINLINE_25Q2_VERSION; if (isAtLeast25Q3) bpfloader_ver = BPFLOADER_MAINLINE_25Q3_VERSION; diff --git a/bpf/progs/include/bpf_map_def.h b/bpf/progs/include/bpf_map_def.h index 1fd2c1e436..d289e89e5f 100644 --- a/bpf/progs/include/bpf_map_def.h +++ b/bpf/progs/include/bpf_map_def.h @@ -208,7 +208,6 @@ _Static_assert(_Alignof(struct bpf_prog_def) == 4, "_Alignof struct bpf_prog_def #define BPFLOADER_MAINLINE_S_VERSION 3100u // Android S (31) #define BPFLOADER_MAINLINE_T_VERSION 3300u // Android T (33) #define BPFLOADER_MAINLINE_U_VERSION 3400u // Android U (34) -#define BPFLOADER_MAINLINE_U_QPR3_VERSION 3403u // Android U QPR3 #define BPFLOADER_MAINLINE_V_VERSION 3500u // Android V (35) #define BPFLOADER_MAINLINE_25Q2_VERSION 3600u // Android 25Q2 (36.0) #define BPFLOADER_MAINLINE_25Q3_VERSION 3601u // Android 25Q3 (36.0+) From 715ef661e4ac142c0c0e26aaa2eeb0fc419f766b Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Fri, 12 Sep 2025 12:32:56 -0700 Subject: [PATCH 15/21] Add RTMGRP_IPV6_IFINFO This is needed to receive IF_RA_MANAGED and IF_RA_OTHERCONF notifications from netlink. See inet6_ifinfo_notify() in addrconf.c. Test: TH Flag: NONE yes Change-Id: I525eef4eb305c08aa2480e91321ea5fd514d4d65 --- .../com/android/net/module/util/netlink/NetlinkConstants.java | 1 + 1 file changed, 1 insertion(+) diff --git a/staticlibs/device/com/android/net/module/util/netlink/NetlinkConstants.java b/staticlibs/device/com/android/net/module/util/netlink/NetlinkConstants.java index 1896de60b8..37738f443c 100644 --- a/staticlibs/device/com/android/net/module/util/netlink/NetlinkConstants.java +++ b/staticlibs/device/com/android/net/module/util/netlink/NetlinkConstants.java @@ -149,6 +149,7 @@ public class NetlinkConstants { public static final int RTMGRP_IPV4_IFADDR = 0x10; public static final int RTMGRP_IPV6_IFADDR = 0x100; public static final int RTMGRP_IPV6_ROUTE = 0x400; + public static final int RTMGRP_IPV6_IFINFO = 0x800; public static final int RTNLGRP_IPV6_PREFIX = 18; public static final int RTMGRP_IPV6_PREFIX = 1 << (RTNLGRP_IPV6_PREFIX - 1); public static final int RTNLGRP_ND_USEROPT = 20; From 942ea04d1fe0aac9f235892654420d5b9bf29e90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Fri, 12 Sep 2025 07:03:01 -0700 Subject: [PATCH 16/21] BpfUtils: fix sdk level detection logic for 25Q3+ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test: TreeHugger Flag: EXEMPT BUGFIX Signed-off-by: Maciej Żenczykowski Change-Id: I52ee176dc84e9c0e3e1bffd480841f1f1247eaa3 --- bpf/headers/include/bpf/BpfUtils.h | 54 +++++++++++++++++++++++------- bpf/loader/NetBpfLoad.cpp | 19 +++-------- bpf/progs/include/bpf_map_def.h | 4 +-- 3 files changed, 48 insertions(+), 29 deletions(-) diff --git a/bpf/headers/include/bpf/BpfUtils.h b/bpf/headers/include/bpf/BpfUtils.h index 85502ecbd5..9bb5e5ac96 100644 --- a/bpf/headers/include/bpf/BpfUtils.h +++ b/bpf/headers/include/bpf/BpfUtils.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -34,19 +35,46 @@ namespace android { namespace bpf { -const bool unreleased = (base::GetProperty("ro.build.version.codename", "REL") != "REL"); -const int api_level = unreleased ? 10000 : android_get_device_api_level(); -const bool isAtLeastR = (api_level >= 30); -const bool isAtLeastS = (api_level >= 31); -// Sv2 is 32 -const bool isAtLeastT = (api_level >= 33); -const bool isAtLeastU = (api_level >= 34); -const bool isAtLeastV = (api_level >= 35); -const bool isAtLeast25Q2 = (api_level >= 36); // 36.0 -const bool isAtLeast25Q3 = (api_level > 36); // 36.0+ (TODO: fix this!) -const bool isAtLeast25Q4 = false; // 36.1 -const bool isAtLeast26Q1 = false; // 36.1+ -const bool isAtLeast26Q2 = false; // 37.0 +static inline int get_api_level_full() { + // This fetches/parses 'ro.build.version.sdk' system property. + const int api_level = android_get_device_api_level(); + // Before Baklava/25Q2 there is no 'sdk_full' + if (api_level < 36) return api_level * 100; // 3x -> 3x00 + + // Fetch and parse the 'sdk_full' system property. + char value[92] = {}; + if (__system_property_get("ro.build.version.sdk_full", value) < 1) abort(); + int major, minor; + if (sscanf(value, "%d.%d", &major, &minor) != 2) abort(); + if (major < 36 || minor < 0 || minor > 9) abort(); + const int api_level_full = major * 100 + minor * 10; // 3x.y -> 3xy0 + + // Fetch and parse our platform provided .rc file - this provides quarterly info as well + FILE * f = fopen("/system/etc/init/netbpfload.rc", "re"); + if (!f) abort(); + int y, q, a, b, c; + if (fscanf(f, "# %d %d %d %d %d #", &y, &q, &a, &b, &c) != 5) abort(); + if (a < 36 || b < 0 || b > 9 || c < 0 || c > 4) abort(); + fclose(f); + const int api_level_fuller = a * 100 + b * 10 + c * 2; // 3x.y.z -> 3xy[2z] + + const bool unreleased = (base::GetProperty("ro.build.version.codename", "REL") != "REL"); + return std::max(api_level_fuller, api_level_full) + unreleased; +} + +const int api_level_full = get_api_level_full(); + +const bool isAtLeastR = (api_level_full >= 3000); // 30 +const bool isAtLeastS = (api_level_full >= 3100); // 31 +const bool isAtLeastSv2 = (api_level_full >= 3200); // 32 +const bool isAtLeastT = (api_level_full >= 3300); // 33 +const bool isAtLeastU = (api_level_full >= 3400); // 34 +const bool isAtLeastV = (api_level_full >= 3500); // 35 +const bool isAtLeast25Q2 = (api_level_full >= 3600); // 36.0 +const bool isAtLeast25Q3 = (api_level_full >= 3602); // 36.0+ +const bool isAtLeast25Q4 = (api_level_full >= 3610); // 36.1 +const bool isAtLeast26Q1 = (api_level_full >= 3612); // 36.1+ +const bool isAtLeast26Q2 = (api_level_full >= 3700); // 37.0 // See kernel's net/core/sock_diag.c __sock_gen_cookie() // the implementation of which guarantees 0 will never be returned, diff --git a/bpf/loader/NetBpfLoad.cpp b/bpf/loader/NetBpfLoad.cpp index 26b3da339b..7245adbf24 100644 --- a/bpf/loader/NetBpfLoad.cpp +++ b/bpf/loader/NetBpfLoad.cpp @@ -1394,7 +1394,7 @@ static bool loadObject(const unsigned int bpfloader_ver, #define BPFROOT APEXROOT "/etc/bpf/mainline/" static bool loadAllObjects(const unsigned int bpfloader_ver) { - bool libbpf = isAtLeast25Q3 || useLibBpf; + bool libbpf = isAtLeast26Q1 || useLibBpf; if (!loadObject(bpfloader_ver, BPFROOT "offload.o")) return false; if (!loadObject(bpfloader_ver, BPFROOT "test.o", libbpf)) return false; if (isAtLeastT) { @@ -1586,7 +1586,7 @@ static int doLoad(char** argv, char * const envp[]) { const bool runningAsRoot = !getuid(); // true iff U QPR3 or V+ - const int first_api_level = GetIntProperty("ro.board.first_api_level", api_level); + const int first_api_level = GetIntProperty("ro.board.first_api_level", api_level_full / 100); // last in U QPR2 beta1 const bool has_platform_bpfloader_rc = exists("/system/etc/init/bpfloader.rc"); @@ -1594,19 +1594,10 @@ static int doLoad(char** argv, char * const envp[]) { const bool has_platform_netbpfload_rc = exists("/system/etc/init/netbpfload.rc"); // Version of Network BpfLoader depends on the Android OS version - unsigned int bpfloader_ver = BPFLOADER_MAINLINE_S_VERSION; - if (isAtLeastT) bpfloader_ver = BPFLOADER_MAINLINE_T_VERSION; - if (isAtLeastU) bpfloader_ver = BPFLOADER_MAINLINE_U_VERSION; - if (isAtLeastV) bpfloader_ver = BPFLOADER_MAINLINE_V_VERSION; - if (isAtLeast25Q2) bpfloader_ver = BPFLOADER_MAINLINE_25Q2_VERSION; - if (isAtLeast25Q3) bpfloader_ver = BPFLOADER_MAINLINE_25Q3_VERSION; - if (isAtLeast25Q4) bpfloader_ver = BPFLOADER_MAINLINE_25Q4_VERSION; - if (isAtLeast26Q1) bpfloader_ver = BPFLOADER_MAINLINE_26Q1_VERSION; - if (isAtLeast26Q2) bpfloader_ver = BPFLOADER_MAINLINE_26Q2_VERSION; + unsigned int bpfloader_ver = api_level_full; - ALOGI("NetBpfLoad v0.%u (%s) api:%d/%d kver:%07x (%s) libbpf: v%u.%u " - "uid:%d rc:%d%d", - bpfloader_ver, argv[0], android_get_device_api_level(), api_level, + ALOGI("NetBpfLoad v%u (%s) api:%d kver:%07x (%s) libbpf: v%u.%u uid:%d rc:%d%d", + bpfloader_ver, argv[0], android_get_device_api_level(), kernelVer, describeArch(), libbpf_major_version(), libbpf_minor_version(), getuid(), has_platform_bpfloader_rc, has_platform_netbpfload_rc); diff --git a/bpf/progs/include/bpf_map_def.h b/bpf/progs/include/bpf_map_def.h index d289e89e5f..7d54d639f6 100644 --- a/bpf/progs/include/bpf_map_def.h +++ b/bpf/progs/include/bpf_map_def.h @@ -210,7 +210,7 @@ _Static_assert(_Alignof(struct bpf_prog_def) == 4, "_Alignof struct bpf_prog_def #define BPFLOADER_MAINLINE_U_VERSION 3400u // Android U (34) #define BPFLOADER_MAINLINE_V_VERSION 3500u // Android V (35) #define BPFLOADER_MAINLINE_25Q2_VERSION 3600u // Android 25Q2 (36.0) -#define BPFLOADER_MAINLINE_25Q3_VERSION 3601u // Android 25Q3 (36.0+) +#define BPFLOADER_MAINLINE_25Q3_VERSION 3602u // Android 25Q3 (36.0+) #define BPFLOADER_MAINLINE_25Q4_VERSION 3610u // Android 25Q4 (36.1) -#define BPFLOADER_MAINLINE_26Q1_VERSION 3611u // Android 26Q1 (36.1+) +#define BPFLOADER_MAINLINE_26Q1_VERSION 3612u // Android 26Q1 (36.1+) #define BPFLOADER_MAINLINE_26Q2_VERSION 3700u // Android 26Q2 (37.0) From 4ab80490ae9937997fe8db8eb13d4d59618d4bbe Mon Sep 17 00:00:00 2001 From: Hnin Naing Date: Thu, 11 Sep 2025 23:46:54 +0000 Subject: [PATCH 17/21] Log validated underlying network count that's available for VCN underlying network selection Flag: EXEMPT METRICS Test: atest FrameworksVcnTests, CtsVcnTestCases Bug: 288600040 Change-Id: Ia1b51aa64388bc5c167f3a1081d8683d98fe0bef --- .../server/vcn/VcnGatewayConnection.java | 9 +++++- .../server/vcn/metrics/VcnMetrics.java | 11 ++++++-- .../UnderlyingNetworkController.java | 28 ++++++++++++++++++- .../vcn/VcnGatewayConnectionTestBase.java | 3 +- .../UnderlyingNetworkControllerTest.java | 28 +++++++++++++++++++ 5 files changed, 74 insertions(+), 5 deletions(-) diff --git a/Vcn/service-b/src/com/android/server/vcn/VcnGatewayConnection.java b/Vcn/service-b/src/com/android/server/vcn/VcnGatewayConnection.java index 57ef5da9a9..0a160fc097 100644 --- a/Vcn/service-b/src/com/android/server/vcn/VcnGatewayConnection.java +++ b/Vcn/service-b/src/com/android/server/vcn/VcnGatewayConnection.java @@ -834,6 +834,7 @@ public class VcnGatewayConnection extends StateMachine { mDeps.newUnderlyingNetworkController( mVcnContext, mConnectionConfig, + mId, subscriptionGroup, mLastSnapshot, mUnderlyingNetworkControllerCallback); @@ -2815,11 +2816,17 @@ public class VcnGatewayConnection extends StateMachine { public UnderlyingNetworkController newUnderlyingNetworkController( VcnContext vcnContext, VcnGatewayConnectionConfig connectionConfig, + int gatewayConnectionId, ParcelUuid subscriptionGroup, TelephonySubscriptionSnapshot snapshot, UnderlyingNetworkControllerCallback callback) { return new UnderlyingNetworkController( - vcnContext, connectionConfig, subscriptionGroup, snapshot, callback); + vcnContext, + connectionConfig, + gatewayConnectionId, + subscriptionGroup, + snapshot, + callback); } /** Builds a new IkeSession. */ diff --git a/Vcn/service-b/src/com/android/server/vcn/metrics/VcnMetrics.java b/Vcn/service-b/src/com/android/server/vcn/metrics/VcnMetrics.java index 0ccd4ecd4d..021606bb6d 100644 --- a/Vcn/service-b/src/com/android/server/vcn/metrics/VcnMetrics.java +++ b/Vcn/service-b/src/com/android/server/vcn/metrics/VcnMetrics.java @@ -16,7 +16,6 @@ package com.android.server.vcn.metrics; -import static com.android.server.vcn.metrics.VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED; import static com.android.server.vcn.metrics.VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_INTERNAL_ERROR; import static com.android.server.vcn.metrics.VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_NETWORK_AGENT_UNWANTED; import static com.android.server.vcn.metrics.VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED__GW_TEARDOWN_REASON__GATEWAY_TEARDOWN_REASON_NONE; @@ -56,7 +55,7 @@ public class VcnMetrics { /** Log an atom when a VcnGatewayConnection has entered safe mode. */ public void logEnterSafeMode(int gatewayConnectionId) { VcnStatsLog.write( - VCN_GATEWAY_CONNECTION_STATE_CHANGED, + VcnStatsLog.VCN_GATEWAY_CONNECTION_STATE_CHANGED, gatewayConnectionId, GATEWAY_TEARDOWN_REASON_NONE, true /* isInSafeMode */); @@ -125,4 +124,12 @@ public class VcnMetrics { true /* isConnected */, false /* isValidated */); } + + /** + * Log an atom about number of validated underlying network available for VCN network selection. + */ + public void logValidatedUnderlyingNetworkCount(int gatewayConnectionId, int count) { + VcnStatsLog.write( + VcnStatsLog.VCN_UNDERLYING_NETWORKS_STATE_CHANGED, gatewayConnectionId, count); + } } diff --git a/Vcn/service-b/src/com/android/server/vcn/routeselection/UnderlyingNetworkController.java b/Vcn/service-b/src/com/android/server/vcn/routeselection/UnderlyingNetworkController.java index 37ec0e8f40..1012ba4823 100644 --- a/Vcn/service-b/src/com/android/server/vcn/routeselection/UnderlyingNetworkController.java +++ b/Vcn/service-b/src/com/android/server/vcn/routeselection/UnderlyingNetworkController.java @@ -56,6 +56,7 @@ import com.android.internal.annotations.VisibleForTesting.Visibility; import com.android.modules.utils.HandlerExecutor; import com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionSnapshot; import com.android.server.vcn.VcnContext; +import com.android.server.vcn.metrics.VcnMetrics; import com.android.server.vcn.routeselection.UnderlyingNetworkEvaluator.NetworkEvaluatorCallback; import java.util.ArrayList; @@ -105,19 +106,31 @@ public class UnderlyingNetworkController { @Nullable private UnderlyingNetworkRecord mCurrentRecord; @Nullable private UnderlyingNetworkRecord.Builder mRecordInProgress; + private final int mGatewayConnectionId; + private final VcnMetrics mVcnMetrics; + public UnderlyingNetworkController( @NonNull VcnContext vcnContext, @NonNull VcnGatewayConnectionConfig connectionConfig, + int gatewayConnectionId, @NonNull ParcelUuid subscriptionGroup, @NonNull TelephonySubscriptionSnapshot snapshot, @NonNull UnderlyingNetworkControllerCallback cb) { - this(vcnContext, connectionConfig, subscriptionGroup, snapshot, cb, new Dependencies()); + this( + vcnContext, + connectionConfig, + gatewayConnectionId, + subscriptionGroup, + snapshot, + cb, + new Dependencies()); } @VisibleForTesting(visibility = Visibility.PRIVATE) UnderlyingNetworkController( @NonNull VcnContext vcnContext, @NonNull VcnGatewayConnectionConfig connectionConfig, + int gatewayConnectionId, @NonNull ParcelUuid subscriptionGroup, @NonNull TelephonySubscriptionSnapshot snapshot, @NonNull UnderlyingNetworkControllerCallback cb, @@ -128,6 +141,8 @@ public class UnderlyingNetworkController { mLastSnapshot = Objects.requireNonNull(snapshot, "Missing snapshot"); mCb = Objects.requireNonNull(cb, "Missing cb"); mDeps = Objects.requireNonNull(deps, "Missing deps"); + mGatewayConnectionId = gatewayConnectionId; + mVcnMetrics = mDeps.newVcnMetrics(); mHandler = new Handler(mVcnContext.getLooper()); @@ -468,6 +483,9 @@ public class UnderlyingNetworkController { .getContext() .getSystemService(TelephonyManager.class) .unregisterTelephonyCallback(mActiveDataSubIdListener); + + // Log empty validated network count left, as a clean up state. + mVcnMetrics.logValidatedUnderlyingNetworkCount(mGatewayConnectionId, 0); } private TreeSet getSortedUnderlyingNetworks() { @@ -489,6 +507,7 @@ public class UnderlyingNetworkController { } TreeSet sorted = getSortedUnderlyingNetworks(); + mVcnMetrics.logValidatedUnderlyingNetworkCount(mGatewayConnectionId, sorted.size()); UnderlyingNetworkEvaluator candidateEvaluator = sorted.isEmpty() ? null : sorted.first(); UnderlyingNetworkRecord candidate = @@ -734,7 +753,9 @@ public class UnderlyingNetworkController { } @VisibleForTesting(visibility = Visibility.PRIVATE) + /* External dependencies, for injection in tests */ public static class Dependencies { + /** Builds a new UnderlyingNetworkEvaluator. */ public UnderlyingNetworkEvaluator newUnderlyingNetworkEvaluator( @NonNull VcnContext vcnContext, @NonNull Network network, @@ -752,5 +773,10 @@ public class UnderlyingNetworkController { carrierConfig, evaluatorCallback); } + + /** Builds a new VcnMetrics. */ + public VcnMetrics newVcnMetrics() { + return new VcnMetrics(); + } } } diff --git a/Vcn/tests/unit/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java b/Vcn/tests/unit/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java index 461fa9cd9c..33c6f833da 100644 --- a/Vcn/tests/unit/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java +++ b/Vcn/tests/unit/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java @@ -25,6 +25,7 @@ import static com.android.server.vcn.VcnTestUtils.setupIpSecManager; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.atLeastOnce; @@ -228,7 +229,7 @@ public class VcnGatewayConnectionTestBase { doReturn(mUnderlyingNetworkController) .when(mDeps) - .newUnderlyingNetworkController(any(), any(), any(), any(), any()); + .newUnderlyingNetworkController(any(), any(), anyInt(), any(), any(), any()); doReturn(mVcnMetrics).when(mDeps).newVcnMetrics(); doReturn(mWakeLock) .when(mDeps) diff --git a/Vcn/tests/unit/java/com/android/server/vcn/routeselection/UnderlyingNetworkControllerTest.java b/Vcn/tests/unit/java/com/android/server/vcn/routeselection/UnderlyingNetworkControllerTest.java index b67a2fd21b..17c1ba9c86 100644 --- a/Vcn/tests/unit/java/com/android/server/vcn/routeselection/UnderlyingNetworkControllerTest.java +++ b/Vcn/tests/unit/java/com/android/server/vcn/routeselection/UnderlyingNetworkControllerTest.java @@ -71,6 +71,7 @@ import androidx.test.runner.AndroidJUnit4; import com.android.server.vcn.TelephonySubscriptionTracker.TelephonySubscriptionSnapshot; import com.android.server.vcn.VcnContext; import com.android.server.vcn.VcnNetworkProvider; +import com.android.server.vcn.metrics.VcnMetrics; import com.android.server.vcn.routeselection.UnderlyingNetworkController.Dependencies; import com.android.server.vcn.routeselection.UnderlyingNetworkController.NetworkBringupCallback; import com.android.server.vcn.routeselection.UnderlyingNetworkController.UnderlyingNetworkControllerCallback; @@ -100,6 +101,7 @@ public class UnderlyingNetworkControllerTest { private static final int INITIAL_SUB_ID_1 = 1; private static final int INITIAL_SUB_ID_2 = 2; private static final int UPDATED_SUB_ID = 3; + private static final int GATEWAY_CONNECTION_ID = 1234; private static final Set INITIAL_SUB_IDS = new ArraySet<>(Arrays.asList(INITIAL_SUB_ID_1, INITIAL_SUB_ID_2)); @@ -164,6 +166,7 @@ public class UnderlyingNetworkControllerTest { @Mock private UnderlyingNetworkControllerCallback mNetworkControllerCb; @Mock private NetworkEvaluatorCallback mEvaluatorCallback; @Mock private Network mNetwork; + @Mock private VcnMetrics mVcnMetrics; @Spy private Dependencies mDependencies = new Dependencies(); @@ -218,11 +221,13 @@ public class UnderlyingNetworkControllerTest { doReturn(mNetworkEvaluator) .when(mDependencies) .newUnderlyingNetworkEvaluator(any(), any(), any(), any(), any(), any(), any()); + doReturn(mVcnMetrics).when(mDependencies).newVcnMetrics(); mUnderlyingNetworkController = new UnderlyingNetworkController( mVcnContext, VcnGatewayConnectionConfigTest.buildTestConfig(), + GATEWAY_CONNECTION_ID, SUB_GROUP, mSubscriptionSnapshot, mNetworkControllerCb, @@ -268,6 +273,7 @@ public class UnderlyingNetworkControllerTest { new UnderlyingNetworkController( vcnContext, VcnGatewayConnectionConfigTest.buildTestConfig(), + GATEWAY_CONNECTION_ID, SUB_GROUP, mSubscriptionSnapshot, mNetworkControllerCb); @@ -320,6 +326,7 @@ public class UnderlyingNetworkControllerTest { new UnderlyingNetworkController( mVcnContext, VcnGatewayConnectionConfigTest.buildTestConfig(networkTemplates), + GATEWAY_CONNECTION_ID, SUB_GROUP, mSubscriptionSnapshot, mNetworkControllerCb); @@ -361,6 +368,7 @@ public class UnderlyingNetworkControllerTest { new UnderlyingNetworkController( mVcnContext, VcnGatewayConnectionConfigTest.buildTestConfig(networkTemplates), + GATEWAY_CONNECTION_ID, SUB_GROUP, mSubscriptionSnapshot, mNetworkControllerCb); @@ -515,6 +523,7 @@ public class UnderlyingNetworkControllerTest { .unregisterNetworkCallback(any(NetworkBringupCallback.class)); verify(mConnectivityManager) .unregisterNetworkCallback(any(UnderlyingNetworkListener.class)); + verify(mVcnMetrics).logValidatedUnderlyingNetworkCount(eq(GATEWAY_CONNECTION_ID), eq(0)); } private static UnderlyingNetworkRecord getTestNetworkRecord( @@ -780,6 +789,7 @@ public class UnderlyingNetworkControllerTest { new UnderlyingNetworkController( mVcnContext, VcnGatewayConnectionConfigTest.buildTestConfig(networkTemplates), + GATEWAY_CONNECTION_ID, SUB_GROUP, mSubscriptionSnapshot, mNetworkControllerCb, @@ -834,6 +844,24 @@ public class UnderlyingNetworkControllerTest { verify(expectedEvaluator, never()).setIsSelected(eq(true), any(), any(), any(), any()); } + @Test + public void testValidatedUnderlyingNetworkCountsLogged() { + final List networkTemplates = new ArrayList(); + networkTemplates.add(CELL_TEMPLATE_DUN); + networkTemplates.add(CELL_TEMPLATE_CBS); + + // Bring up CBS network + UnderlyingNetworkListener cb = setupControllerAndGetNetworkListener(networkTemplates); + bringupNetworkAndGetEvaluator(cb, CBS_NETWORK_CAPABILITIES, networkTemplates); + + verify(mVcnMetrics).logValidatedUnderlyingNetworkCount(eq(GATEWAY_CONNECTION_ID), eq(1)); + + // Bring up DUN network + bringupNetworkAndGetEvaluator(cb, DUN_NETWORK_CAPABILITIES, networkTemplates); + + verify(mVcnMetrics).logValidatedUnderlyingNetworkCount(eq(GATEWAY_CONNECTION_ID), eq(2)); + } + @Test public void testSelectMorePreferredNetwork() { final List networkTemplates = new ArrayList(); From 68744c4a9a4d5124f59fe69660290672716c2f4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Sat, 13 Sep 2025 00:35:32 -0700 Subject: [PATCH 18/21] NetBpfLoad: use global `api_level_full` directly in NetBpfLoad MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change removes the `bpfloader_ver` parameter from several internal functions and replaces its usage with the global `api_level_full` variable, simplifying function signatures. Test: TreeHugger Flag: EXEMPT PURE_REFACTOR Signed-off-by: Maciej Żenczykowski Change-Id: I6165e525d7aa49fbd7d12be0c3ea85015cfc6f3d --- bpf/loader/NetBpfLoad.cpp | 86 +++++++++++++++------------------ bpf/progs/include/bpf_map_def.h | 26 +++++----- 2 files changed, 51 insertions(+), 61 deletions(-) diff --git a/bpf/loader/NetBpfLoad.cpp b/bpf/loader/NetBpfLoad.cpp index 7245adbf24..3b0e22b68d 100644 --- a/bpf/loader/NetBpfLoad.cpp +++ b/bpf/loader/NetBpfLoad.cpp @@ -819,8 +819,7 @@ static enum bpf_map_type sanitizeMapType(enum bpf_map_type type) { return type; } -static int createMaps(ElfObject& elfObj, vector& md, vector& mapFds, - const unsigned int bpfloader_ver) { +static int createMaps(ElfObject& elfObj, vector& md, vector& mapFds) { int ret = 0; vector btfData; struct btf *btf = NULL; @@ -843,14 +842,14 @@ static int createMaps(ElfObject& elfObj, vector& md, vector< } for (unsigned i = 0; i < md.size(); i++) { - if (bpfloader_ver < md[i].bpfloader_min_ver) { + if (api_level_full < md[i].bpfloader_min_ver) { ALOGD("skipping map %s which requires bpfloader min ver 0x%05x", md[i].name(), md[i].bpfloader_min_ver); mapFds.push_back(unique_fd()); continue; } - if (bpfloader_ver >= md[i].bpfloader_max_ver) { + if (api_level_full >= md[i].bpfloader_max_ver) { ALOGD("skipping map %s which requires bpfloader max ver 0x%05x", md[i].name(), md[i].bpfloader_max_ver); mapFds.push_back(unique_fd()); @@ -1032,8 +1031,7 @@ static int pinProg(const borrowed_fd& fd, const struct bpf_prog_def& progDef) { return 0; } -static int validateProg(const borrowed_fd& fd, const char* const progPinLoc, - const unsigned int bpfloader_ver) { +static int validateProg(const borrowed_fd& fd, const char* const progPinLoc) { if (!isAtLeastKernelVersion(4, 14)) { return 0; } @@ -1059,15 +1057,14 @@ static int validateProg(const borrowed_fd& fd, const char* const progPinLoc, } ALOGI("prog %s id %d len jit:%d xlat:%d", progPinLoc, progId, jitLen, xlatLen); - if (!jitLen && bpfloader_ver >= BPFLOADER_MAINLINE_25Q2_VERSION) { + if (!jitLen && api_level_full >= BPFLOADER_MAINLINE_25Q2_VERSION) { ALOGE("Kernel eBPF JIT failure for %s", progPinLoc); return -ENOTSUP; } return 0; } -static int loadCodeSections(ElfObject& elfObj, vector& cs, const string& license, - const unsigned int bpfloader_ver) { +static int loadCodeSections(ElfObject& elfObj, vector& cs, const string& license) { for (int i = 0; i < (int)cs.size(); i++) { unique_fd& fd = cs[i].prog_fd; int ret; @@ -1084,8 +1081,8 @@ static int loadCodeSections(ElfObject& elfObj, vector& cs, const st if (kernelVer < cs[i].prog_def->min_kver) continue; if (kernelVer >= cs[i].prog_def->max_kver) continue; - if (bpfloader_ver < cs[i].prog_def->bpfloader_min_ver) continue; - if (bpfloader_ver >= cs[i].prog_def->bpfloader_max_ver) continue; + if (api_level_full < cs[i].prog_def->bpfloader_min_ver) continue; + if (api_level_full >= cs[i].prog_def->bpfloader_max_ver) continue; bool reuse = false; if (access(cs[i].prog_def->pin_location, F_OK) == 0) { @@ -1148,15 +1145,14 @@ static int loadCodeSections(ElfObject& elfObj, vector& cs, const st ret = pinProg(fd, cs[i].prog_def.value()); if (ret) return ret; } - ret = validateProg(fd, cs[i].prog_def->pin_location, bpfloader_ver); + ret = validateProg(fd, cs[i].prog_def->pin_location); if (ret) return ret; } return 0; } -static int prepareLoadMaps(const struct bpf_object* obj, const vector& md, - const unsigned int bpfloader_ver) { +static int prepareLoadMaps(const struct bpf_object* obj, const vector& md) { for (unsigned i = 0; i < md.size(); i++) { struct bpf_map* m = bpf_object__find_map_by_name(obj, md[i].name()); if (!m) { @@ -1164,9 +1160,9 @@ static int prepareLoadMaps(const struct bpf_object* obj, const vector= md[i].bpfloader_max_ver) { + if (api_level_full < md[i].bpfloader_min_ver || api_level_full >= md[i].bpfloader_max_ver) { ALOGD("skipping map %s: bpfloader 0x%05x is outside required range [0x%05x, 0x%05x)", - md[i].name(), bpfloader_ver, + md[i].name(), api_level_full, md[i].bpfloader_min_ver, md[i].bpfloader_max_ver); bpf_map__set_autocreate(m, false); continue; @@ -1191,8 +1187,7 @@ static int prepareLoadMaps(const struct bpf_object* obj, const vector& cs, - const unsigned int bpfloader_ver) { +static int prepareLoadProgs(const struct bpf_object* obj, const vector& cs) { for (int i = 0; i < (int)cs.size(); i++) { if (!cs[i].prog_def.has_value()) { ALOGE("[%d] missing program definition! bad bpf.o build?", i); @@ -1214,11 +1209,11 @@ static int prepareLoadProgs(const struct bpf_object* obj, const vectorbpfloader_min_ver; - unsigned bpfMaxVer = cs[i].prog_def->bpfloader_max_ver; - if (bpfloader_ver < bpfMinVer || bpfloader_ver >= bpfMaxVer) { + int bpfMinVer = cs[i].prog_def->bpfloader_min_ver; + int bpfMaxVer = cs[i].prog_def->bpfloader_max_ver; + if (api_level_full < bpfMinVer || api_level_full >= bpfMaxVer) { ALOGD("skipping prog %s: bpfloader 0x%05x is outside required range [0x%05x, 0x%05x)", - cs[i].prog_def->name(), bpfloader_ver, bpfMinVer, bpfMaxVer); + cs[i].prog_def->name(), api_level_full, bpfMinVer, bpfMaxVer); bpf_program__set_autoload(prog, false); continue; } @@ -1257,7 +1252,7 @@ static int pinMaps(const struct bpf_object* obj, const vector& cs, const unsigned int bpfloader_ver) { + const vector& cs) { int ret; for (int i = 0; i < (int)cs.size(); i++) { @@ -1279,13 +1274,13 @@ static int pinProgs(const struct bpf_object * obj, int fd = bpf_program__fd(prog); ret = pinProg(fd, cs[i].prog_def.value()); if (ret) return ret; - ret = validateProg(fd, cs[i].prog_def->pin_location, bpfloader_ver); + ret = validateProg(fd, cs[i].prog_def->pin_location); if (ret) return ret; } return 0; } -static int loadProgByLibbpf(const char* const elfPath, const unsigned int bpfloader_ver) { +static int loadProgByLibbpf(const char* const elfPath) { ElfObject elfObj(elfPath); vector md; vector cs; @@ -1301,13 +1296,13 @@ static int loadProgByLibbpf(const char* const elfPath, const unsigned int bpfloa ret = elfObj.readSectionByName(".android_maps", md); if (ret) return ret; - ret = prepareLoadMaps(obj, md, bpfloader_ver); + ret = prepareLoadMaps(obj, md); if (ret) return ret; ret = readCodeSections(elfObj, cs); if (ret && ret != -ENOENT) return ret; - ret = prepareLoadProgs(obj, cs, bpfloader_ver); + ret = prepareLoadProgs(obj, cs); if (ret) return ret; ret = bpf_object__load(obj); @@ -1318,13 +1313,13 @@ static int loadProgByLibbpf(const char* const elfPath, const unsigned int bpfloa ret = pinMaps(obj, md); if (ret) return ret; - ret = pinProgs(obj, cs, bpfloader_ver); + ret = pinProgs(obj, cs); if (ret) return ret; return 0; } -int loadProg(const char* const elfPath, const unsigned int bpfloader_ver) { +int loadProg(const char* const elfPath) { ElfObject elfObj(elfPath); vector license; vector cs; @@ -1341,13 +1336,13 @@ int loadProg(const char* const elfPath, const unsigned int bpfloader_ver) { elfPath, (char*)license.data()); } - ALOGD("BpfLoader ver 0x%05x processing ELF object %s", bpfloader_ver, elfPath); + ALOGD("Processing ELF object %s", elfPath); ret = elfObj.readSectionByName(".android_maps", md); if (ret == -2) ret = 0; // -2 means there were no maps to read if (ret) return ret; - ret = createMaps(elfObj, md, mapFds, bpfloader_ver); + ret = createMaps(elfObj, md, mapFds); if (ret) { ALOGE("Failed to create maps: (ret=%d) in %s", ret, elfPath); return ret; @@ -1365,7 +1360,7 @@ int loadProg(const char* const elfPath, const unsigned int bpfloader_ver) { applyMapRelo(elfObj, md, mapFds, cs); - ret = loadCodeSections(elfObj, cs, string(license.data()), bpfloader_ver); + ret = loadCodeSections(elfObj, cs, string(license.data())); if (ret) ALOGE("Failed to load programs, loadCodeSections ret=%d", ret); return ret; @@ -1379,10 +1374,8 @@ static bool exists(const char* const path) { abort(); // can only hit this if permissions (likely selinux) are screwed up } -static bool loadObject(const unsigned int bpfloader_ver, - const char* const progPath, const bool useLibbpf = false) { - if (useLibbpf ? loadProgByLibbpf(progPath, bpfloader_ver) : - loadProg(progPath, bpfloader_ver)) { +static bool loadObject(const char* const progPath, const bool useLibbpf = false) { + if (useLibbpf ? loadProgByLibbpf(progPath) : loadProg(progPath)) { ALOGE("Failed to load object: %s, libbpf: %d", progPath, useLibbpf); return false; } @@ -1393,14 +1386,14 @@ static bool loadObject(const unsigned int bpfloader_ver, #define APEXROOT "/apex/com.android.tethering" #define BPFROOT APEXROOT "/etc/bpf/mainline/" -static bool loadAllObjects(const unsigned int bpfloader_ver) { +static bool loadAllObjects() { bool libbpf = isAtLeast26Q1 || useLibBpf; - if (!loadObject(bpfloader_ver, BPFROOT "offload.o")) return false; - if (!loadObject(bpfloader_ver, BPFROOT "test.o", libbpf)) return false; + if (!loadObject(BPFROOT "offload.o")) return false; + if (!loadObject(BPFROOT "test.o", libbpf)) return false; if (isAtLeastT) { - if (!loadObject(bpfloader_ver, BPFROOT "clatd.o", libbpf)) return false; - if (!loadObject(bpfloader_ver, BPFROOT "dscpPolicy.o", libbpf)) return false; - if (!loadObject(bpfloader_ver, BPFROOT "netd.o", libbpf)) return false; + if (!loadObject(BPFROOT "clatd.o", libbpf)) return false; + if (!loadObject(BPFROOT "dscpPolicy.o", libbpf)) return false; + if (!loadObject(BPFROOT "netd.o", libbpf)) return false; } return true; } @@ -1593,11 +1586,8 @@ static int doLoad(char** argv, char * const envp[]) { // first in U QPR2 beta~2 const bool has_platform_netbpfload_rc = exists("/system/etc/init/netbpfload.rc"); - // Version of Network BpfLoader depends on the Android OS version - unsigned int bpfloader_ver = api_level_full; - - ALOGI("NetBpfLoad v%u (%s) api:%d kver:%07x (%s) libbpf: v%u.%u uid:%d rc:%d%d", - bpfloader_ver, argv[0], android_get_device_api_level(), + ALOGI("NetBpfLoad (%s) api:%d/%d kver:%07x (%s) libbpf: v%u.%u uid:%d rc:%d%d", + argv[0], android_get_device_api_level(), api_level_full, kernelVer, describeArch(), libbpf_major_version(), libbpf_minor_version(), getuid(), has_platform_bpfloader_rc, has_platform_netbpfload_rc); @@ -1881,7 +1871,7 @@ static int doLoad(char** argv, char * const envp[]) { } // Load all ELF objects, create programs and maps, and pin them - if (!loadAllObjects(bpfloader_ver)) { + if (!loadAllObjects()) { ALOGE("=== CRITICAL FAILURE LOADING BPF PROGRAMS ==="); ALOGE("If this triggers reliably, you're probably missing kernel options or patches."); ALOGE("If this triggers randomly, you might be hitting some memory allocation " diff --git a/bpf/progs/include/bpf_map_def.h b/bpf/progs/include/bpf_map_def.h index 7d54d639f6..dc34aac9a5 100644 --- a/bpf/progs/include/bpf_map_def.h +++ b/bpf/progs/include/bpf_map_def.h @@ -138,8 +138,8 @@ struct bpf_map_def { unsigned int gid; // gid_t unsigned int mode; // mode_t - unsigned int bpfloader_min_ver; - unsigned int bpfloader_max_ver; + int bpfloader_min_ver; + int bpfloader_max_ver; // kernelVer must be >= min_kver and < max_kver unsigned int min_kver; @@ -181,8 +181,8 @@ struct bpf_prog_def { char pad0[3]; // manually pad up to 4 byte alignment, may be used for extensions in the future - unsigned int bpfloader_min_ver; - unsigned int bpfloader_max_ver; + int bpfloader_min_ver; + int bpfloader_max_ver; char create_location[BPF_DEF_CHAR_ARRAY_SIZE]; char pin_location[BPF_DEF_CHAR_ARRAY_SIZE]; @@ -205,12 +205,12 @@ _Static_assert(__alignof__(struct bpf_prog_def) == 4, "__alignof__ struct bpf_pr _Static_assert(_Alignof(struct bpf_prog_def) == 4, "_Alignof struct bpf_prog_def != 4"); // Android Mainline BpfLoader version when running on: -#define BPFLOADER_MAINLINE_S_VERSION 3100u // Android S (31) -#define BPFLOADER_MAINLINE_T_VERSION 3300u // Android T (33) -#define BPFLOADER_MAINLINE_U_VERSION 3400u // Android U (34) -#define BPFLOADER_MAINLINE_V_VERSION 3500u // Android V (35) -#define BPFLOADER_MAINLINE_25Q2_VERSION 3600u // Android 25Q2 (36.0) -#define BPFLOADER_MAINLINE_25Q3_VERSION 3602u // Android 25Q3 (36.0+) -#define BPFLOADER_MAINLINE_25Q4_VERSION 3610u // Android 25Q4 (36.1) -#define BPFLOADER_MAINLINE_26Q1_VERSION 3612u // Android 26Q1 (36.1+) -#define BPFLOADER_MAINLINE_26Q2_VERSION 3700u // Android 26Q2 (37.0) +#define BPFLOADER_MAINLINE_S_VERSION 3100 // Android S (31) +#define BPFLOADER_MAINLINE_T_VERSION 3300 // Android T (33) +#define BPFLOADER_MAINLINE_U_VERSION 3400 // Android U (34) +#define BPFLOADER_MAINLINE_V_VERSION 3500 // Android V (35) +#define BPFLOADER_MAINLINE_25Q2_VERSION 3600 // Android 25Q2 (36.0) +#define BPFLOADER_MAINLINE_25Q3_VERSION 3602 // Android 25Q3 (36.0+) +#define BPFLOADER_MAINLINE_25Q4_VERSION 3610 // Android 25Q4 (36.1) +#define BPFLOADER_MAINLINE_26Q1_VERSION 3612 // Android 26Q1 (36.1+) +#define BPFLOADER_MAINLINE_26Q2_VERSION 3700 // Android 26Q2 (37.0) From 8e730ec9e0ac8ee8218dc8ea8b5ff64f5bd2bf5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Sat, 13 Sep 2025 00:18:47 -0700 Subject: [PATCH 19/21] bpf: remove obsolete isAtLeastR/Sv2/25Q3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We no longer support mainline on R, hence checks for R are not required. We don't want any S vs Sv2 and 25Q2 vs 25Q3 differentiation. Test: TreeHugger Flag: EXEMPT PURE_REFACTOR Signed-off-by: Maciej Żenczykowski Change-Id: I746b404520ccb12fddf02c6956d7cf1824f21bbd --- bpf/headers/include/bpf/BpfUtils.h | 4 +--- bpf/progs/include/bpf_map_def.h | 1 - bpf/tests/mts/bpf_existence_test.cpp | 1 - 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/bpf/headers/include/bpf/BpfUtils.h b/bpf/headers/include/bpf/BpfUtils.h index 9bb5e5ac96..2cf1d06350 100644 --- a/bpf/headers/include/bpf/BpfUtils.h +++ b/bpf/headers/include/bpf/BpfUtils.h @@ -64,14 +64,12 @@ static inline int get_api_level_full() { const int api_level_full = get_api_level_full(); -const bool isAtLeastR = (api_level_full >= 3000); // 30 const bool isAtLeastS = (api_level_full >= 3100); // 31 -const bool isAtLeastSv2 = (api_level_full >= 3200); // 32 + // 32 is Sv2 const bool isAtLeastT = (api_level_full >= 3300); // 33 const bool isAtLeastU = (api_level_full >= 3400); // 34 const bool isAtLeastV = (api_level_full >= 3500); // 35 const bool isAtLeast25Q2 = (api_level_full >= 3600); // 36.0 -const bool isAtLeast25Q3 = (api_level_full >= 3602); // 36.0+ const bool isAtLeast25Q4 = (api_level_full >= 3610); // 36.1 const bool isAtLeast26Q1 = (api_level_full >= 3612); // 36.1+ const bool isAtLeast26Q2 = (api_level_full >= 3700); // 37.0 diff --git a/bpf/progs/include/bpf_map_def.h b/bpf/progs/include/bpf_map_def.h index dc34aac9a5..39f5b00c3f 100644 --- a/bpf/progs/include/bpf_map_def.h +++ b/bpf/progs/include/bpf_map_def.h @@ -210,7 +210,6 @@ _Static_assert(_Alignof(struct bpf_prog_def) == 4, "_Alignof struct bpf_prog_def #define BPFLOADER_MAINLINE_U_VERSION 3400 // Android U (34) #define BPFLOADER_MAINLINE_V_VERSION 3500 // Android V (35) #define BPFLOADER_MAINLINE_25Q2_VERSION 3600 // Android 25Q2 (36.0) -#define BPFLOADER_MAINLINE_25Q3_VERSION 3602 // Android 25Q3 (36.0+) #define BPFLOADER_MAINLINE_25Q4_VERSION 3610 // Android 25Q4 (36.1) #define BPFLOADER_MAINLINE_26Q1_VERSION 3612 // Android 26Q1 (36.1+) #define BPFLOADER_MAINLINE_26Q2_VERSION 3700 // Android 26Q2 (37.0) diff --git a/bpf/tests/mts/bpf_existence_test.cpp b/bpf/tests/mts/bpf_existence_test.cpp index 6849dd414c..948f23b3bd 100644 --- a/bpf/tests/mts/bpf_existence_test.cpp +++ b/bpf/tests/mts/bpf_existence_test.cpp @@ -32,7 +32,6 @@ using std::set; using std::string; using android::bpf::isAtLeastKernelVersion; -using android::bpf::isAtLeastR; using android::bpf::isAtLeastS; using android::bpf::isAtLeastT; using android::bpf::isAtLeastU; From b47900b7c504fa4f7af0b241781c38cb6d61a327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Sat, 13 Sep 2025 03:51:43 -0700 Subject: [PATCH 20/21] NetBpfLoad: remove pointless 0 write. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BPF array maps are zero initialized. Test: TreeHugger Flag: EXEMPT PURE_REFACTOR Signed-off-by: Maciej Żenczykowski Change-Id: I11b3b1db8e3fdf5e926c0e164d4c00c7b84684ce --- bpf/loader/NetBpfLoad.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/bpf/loader/NetBpfLoad.cpp b/bpf/loader/NetBpfLoad.cpp index 7245adbf24..5132409c9e 100644 --- a/bpf/loader/NetBpfLoad.cpp +++ b/bpf/loader/NetBpfLoad.cpp @@ -1873,7 +1873,7 @@ static int doLoad(char** argv, char * const envp[]) { // implies Android S with 4.14 or 4.19 kernel ALOGW("Detected kernel with invalid BPF UAPI - disabling mainline use of eBPF."); // leave a flag that we're 'done' - if (!createDir("/sys/fs/bpf/netd_shared/mainline_done")) return 43; // same as below + if (!createDir("/sys/fs/bpf/netd_shared/mainline_done")) return 37; return 0; } } else { // implies S/T with 4.9 kernel @@ -1888,20 +1888,13 @@ static int doLoad(char** argv, char * const envp[]) { "problems or startup script race."); ALOGE("--- DO NOT EXPECT SYSTEM TO BOOT SUCCESSFULLY ---"); sleep(20); - return 37; + return 38; } { // Create a trivial bpf map: a two element array [int->int] unique_fd map(createMap(BPF_MAP_TYPE_ARRAY, sizeof(int), sizeof(int), 2, 0)); - int zero = 0; - int kernel_bugs = 0; - if (writeToMapEntry(map, &zero, &kernel_bugs, BPF_ANY)) { - ALOGE("Failure to write into index 0 of kernel bugs array."); - return 38; - } - int one = 1; int value = 123; if (writeToMapEntry(map, &one, &value, BPF_ANY)) { From a22d4e1061eeb69484025208ec2198ab32b31368 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Sat, 13 Sep 2025 03:21:47 -0700 Subject: [PATCH 21/21] BpfNetworkStats: use `atomic` for ifindex cache when possible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test: TreeHugger Flag: EXEMPT BUGFIX Signed-off-by: Maciej Żenczykowski Change-Id: Ibc24f079711c36a8bc6ff850fc5b07fb6546b3ae --- .../libs/libnetworkstats/BpfNetworkStats.cpp | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp index da01ed6400..ae8dbbe321 100644 --- a/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp +++ b/service-t/native/libs/libnetworkstats/BpfNetworkStats.cpp @@ -50,23 +50,38 @@ const BpfMapRO& getIfaceStatsMap() { return ifaceStatsMap; } -#ifndef __riscv -#ifndef __ILP32__ +static constexpr unsigned cacheSize = 1024; // see 'iface_index_name_map' bpf map size + +#if !defined(__ILP32__) && !defined(__riscv) static_assert(std::atomic::is_always_lock_free, "Error: 16-byte IfaceValue is not lock-free on this platform!"); -#endif +#define CACHE_IS_ATOMIC +static std::atomic cache[cacheSize]; +#else +static IfaceValue cache[cacheSize]; #endif -static constexpr unsigned cacheSize = 1024; // see 'iface_index_name_map' bpf map size -static IfaceValue cache[cacheSize]; static inline const IfaceValue& updateCache(unsigned i, const IfaceValue &v) { - if (i < cacheSize) cache[i] = v; + if (i < cacheSize) { +#ifdef CACHE_IS_ATOMIC + cache[i].store(v, std::memory_order_relaxed); +#else + cache[i] = v; +#endif + } return v; } Result ifindex2name(const uint32_t ifindex) { - if (ifindex < cacheSize && cache[ifindex].name[0]) return cache[ifindex]; + if (ifindex < cacheSize) { +#ifdef CACHE_IS_ATOMIC + IfaceValue c = cache[ifindex].load(std::memory_order_relaxed); +#else + IfaceValue c = cache[ifindex]; +#endif + if (c.name[0]) return c; + } Result v = getIfaceIndexNameMap().readValue(ifindex); if (v.ok()) return updateCache(ifindex, v.value());