From 69b967f5d0687b043a66ed9a8343ad447bd594aa Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Sun, 13 Jan 2019 03:10:49 +0900 Subject: [PATCH 1/2] Allow Bluetooth to bind to NetworkStack Test: flashed, verified bluetooth tethering gets IP address Bug: b/112869080 Change-Id: Idfbfdf54754fea46eb0099b9b9a3bdc29dd241e0 --- core/java/android/net/NetworkStack.java | 47 ++++++++++++++++++- .../android/server/NetworkStackService.java | 3 +- .../android/server/util/PermissionUtil.java | 17 ++++++- 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/core/java/android/net/NetworkStack.java b/core/java/android/net/NetworkStack.java index 2eac6de56346e..601ee0f54117b 100644 --- a/core/java/android/net/NetworkStack.java +++ b/core/java/android/net/NetworkStack.java @@ -50,6 +50,8 @@ public class NetworkStack { public static final String NETWORKSTACK_PACKAGE_NAME = "com.android.mainline.networkstack"; + private static final int NETWORKSTACK_TIMEOUT_MS = 10_000; + @NonNull @GuardedBy("mPendingNetStackRequests") private final ArrayList mPendingNetStackRequests = new ArrayList<>(); @@ -57,6 +59,8 @@ public class NetworkStack { @GuardedBy("mPendingNetStackRequests") private INetworkStackConnector mConnector; + private volatile boolean mNetworkStackStartRequested = false; + private interface NetworkStackCallback { void onNetworkStackConnected(INetworkStackConnector connector); } @@ -134,6 +138,7 @@ public class NetworkStack { * started. */ public void start(Context context) { + mNetworkStackStartRequested = true; // Try to bind in-process if the library is available IBinder connector = null; try { @@ -170,15 +175,53 @@ public class NetworkStack { } } - // TODO: use this method to obtain the connector when implementing network stack operations + /** + * For non-system server clients, get the connector registered by the system server. + */ + private INetworkStackConnector getRemoteConnector() { + // Block until the NetworkStack connector is registered in ServiceManager. + //

This is only useful for non-system processes that do not have a way to be notified of + // registration completion. Adding a callback system would be too heavy weight considering + // that the connector is registered on boot, so it is unlikely that a client would request + // it before it is registered. + // TODO: consider blocking boot on registration and simplify much of the logic in this class + IBinder connector; + try { + final long before = System.currentTimeMillis(); + while ((connector = ServiceManager.getService(Context.NETWORK_STACK_SERVICE)) == null) { + Thread.sleep(20); + if (System.currentTimeMillis() - before > NETWORKSTACK_TIMEOUT_MS) { + Slog.e(TAG, "Timeout waiting for NetworkStack connector"); + return null; + } + } + } catch (InterruptedException e) { + Slog.e(TAG, "Error waiting for NetworkStack connector", e); + return null; + } + + synchronized (mPendingNetStackRequests) { + return INetworkStackConnector.Stub.asInterface(connector); + } + } + private void requestConnector(@NonNull NetworkStackCallback request) { // TODO: PID check. - if (Binder.getCallingUid() != Process.SYSTEM_UID) { + final int caller = Binder.getCallingUid(); + if (caller != Process.SYSTEM_UID && caller != Process.BLUETOOTH_UID) { // Don't even attempt to obtain the connector and give a nice error message throw new SecurityException( "Only the system server should try to bind to the network stack."); } + if (!mNetworkStackStartRequested) { + // The network stack is not being started in this process, e.g. this process is not + // the system server. Get a remote connector registered by the system server. + mConnector = getRemoteConnector(); + request.onNetworkStackConnected(mConnector); + return; + } + final INetworkStackConnector connector; synchronized (mPendingNetStackRequests) { connector = mConnector; diff --git a/packages/NetworkStack/src/com/android/server/NetworkStackService.java b/packages/NetworkStack/src/com/android/server/NetworkStackService.java index 057012de433a1..cca71e759bdd2 100644 --- a/packages/NetworkStack/src/com/android/server/NetworkStackService.java +++ b/packages/NetworkStack/src/com/android/server/NetworkStackService.java @@ -20,6 +20,7 @@ import static android.net.dhcp.IDhcpServer.STATUS_INVALID_ARGUMENT; import static android.net.dhcp.IDhcpServer.STATUS_SUCCESS; import static android.net.dhcp.IDhcpServer.STATUS_UNKNOWN_ERROR; +import static com.android.server.util.PermissionUtil.checkDumpPermission; import static com.android.server.util.PermissionUtil.checkNetworkStackCallingPermission; import android.annotation.NonNull; @@ -139,7 +140,7 @@ public class NetworkStackService extends Service { @Override protected void dump(@NonNull FileDescriptor fd, @NonNull PrintWriter fout, @Nullable String[] args) { - checkNetworkStackCallingPermission(); + checkDumpPermission(); final IndentingPrintWriter pw = new IndentingPrintWriter(fout, " "); pw.println("NetworkStack logs:"); mLog.dump(fd, pw, args); diff --git a/packages/NetworkStack/src/com/android/server/util/PermissionUtil.java b/packages/NetworkStack/src/com/android/server/util/PermissionUtil.java index 733f87393c328..82bf038073c7e 100644 --- a/packages/NetworkStack/src/com/android/server/util/PermissionUtil.java +++ b/packages/NetworkStack/src/com/android/server/util/PermissionUtil.java @@ -31,8 +31,21 @@ public final class PermissionUtil { */ public static void checkNetworkStackCallingPermission() { // TODO: check that the calling PID is the system server. - if (getCallingUid() != Process.SYSTEM_UID && getCallingUid() != Process.ROOT_UID) { - throw new SecurityException("Invalid caller: " + getCallingUid()); + final int caller = getCallingUid(); + if (caller != Process.SYSTEM_UID && caller != Process.BLUETOOTH_UID) { + throw new SecurityException("Invalid caller: " + caller); + } + } + + /** + * Check that the caller is allowed to dump the network stack, e.g. dumpsys. + * @throws SecurityException The caller is not allowed to dump the network stack. + */ + public static void checkDumpPermission() { + final int caller = getCallingUid(); + if (caller != Process.SYSTEM_UID && caller != Process.ROOT_UID + && caller != Process.SHELL_UID) { + throw new SecurityException("No dump permissions for caller: " + caller); } } From 800e843862cfaac46dd1cc3b6e09aad1de911942 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Mon, 21 Jan 2019 20:57:05 +0900 Subject: [PATCH 2/2] Acquire lock to write NetworkStackConnector As Java reference writes are atomic, the previous code should not have issues, but since mConnector is marked @GuardedBy it could trigger static analysis. Test: m Change-Id: I5a6174a4f32c349e593d0a16d58293d608a487c7 --- core/java/android/net/NetworkStack.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/core/java/android/net/NetworkStack.java b/core/java/android/net/NetworkStack.java index 601ee0f54117b..af043eeccde2c 100644 --- a/core/java/android/net/NetworkStack.java +++ b/core/java/android/net/NetworkStack.java @@ -200,9 +200,7 @@ public class NetworkStack { return null; } - synchronized (mPendingNetStackRequests) { - return INetworkStackConnector.Stub.asInterface(connector); - } + return INetworkStackConnector.Stub.asInterface(connector); } private void requestConnector(@NonNull NetworkStackCallback request) { @@ -217,8 +215,11 @@ public class NetworkStack { if (!mNetworkStackStartRequested) { // The network stack is not being started in this process, e.g. this process is not // the system server. Get a remote connector registered by the system server. - mConnector = getRemoteConnector(); - request.onNetworkStackConnected(mConnector); + final INetworkStackConnector connector = getRemoteConnector(); + synchronized (mPendingNetStackRequests) { + mConnector = connector; + } + request.onNetworkStackConnected(connector); return; }