Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
exec spawning loses capabilities, breaking bluetooth and wakelocks #398
Comments
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment
thestinger
Sep 1, 2016
Contributor
Exec spawning has been disabled for com.android.bluetooth for the time being.
|
Exec spawning has been disabled for com.android.bluetooth for the time being. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment
jgeerds
Sep 17, 2016
I think I hit this issue on my Nexus 5X. com.android.bluetooth is constantly crashing since the latest update (2016-09-16). Is there a workaround? The 2016-09-14 build worked fine for me. Is there a way to downgrade?
jgeerds
commented
Sep 17, 2016
|
I think I hit this issue on my Nexus 5X. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment
thestinger
Sep 17, 2016
Contributor
No, you didn't hit this issue. If it's crashing, it's for another reason. Exec spawning is not enabled for it.
|
No, you didn't hit this issue. If it's crashing, it's for another reason. Exec spawning is not enabled for it. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment
thestinger
Sep 17, 2016
Contributor
Is there a way to downgrade?
No, and it was a security update so that wouldn't be a good idea. File a bug with logs provided and the issue can be fixed. There are already some workarounds for -fsanitize=bounds queued up and it might be solved by one of those.
No, and it was a security update so that wouldn't be a good idea. File a bug with logs provided and the issue can be fixed. There are already some workarounds for -fsanitize=bounds queued up and it might be solved by one of those. |
jgeerds
commented
Sep 17, 2016
|
I opened a issue for it. Thank you very much for your quick response! |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment
brianmcgillion
Nov 2, 2016
I encountered a similar issue today, seemingly solving a similar problem :) Below is what I think might be causing the issue, I have not investigated too far, I was trying to understand the call flow.
ZygoteInit.java :: runSelectLoop()
ZygoteConnection.runonce()
Zygote.forkAndSpecialize()
com_android_internal_os_Zygote.cpp :: nativeForkAndSpecialize()
grant capabilities to the Bluetooth
if (multiuser_get_app_id(uid) == AID_BLUETOOTH) {
capabilities |= (1LL << CAP_WAKE_ALARM);
capabilities |= (1LL << CAP_NET_RAW);
capabilities |= (1LL << CAP_NET_BIND_SERVICE);
}
com_android_internal_os_Zygote.cpp :: ForkAndspecializeCommon()
SetCapabilities(Permitted, Effective)
So in ZygoteConnection.handleChildProc() I believe an Exec drops the uninheritable capabilities.
Also, the AID_WAKELOCK uid/gid will lose the CAP_BLOCK_SUSPEND cap.
would the solution be to backport the ambient capabilities patch [1] and then add the newly acquired caps to the ambient set, by modifying SetCapabilities?
brianmcgillion
commented
Nov 2, 2016
|
I encountered a similar issue today, seemingly solving a similar problem :) Below is what I think might be causing the issue, I have not investigated too far, I was trying to understand the call flow. ZygoteInit.java :: runSelectLoop()
So in ZygoteConnection.handleChildProc() I believe an Exec drops the uninheritable capabilities. would the solution be to backport the ambient capabilities patch [1] and then add the newly acquired caps to the ambient set, by modifying SetCapabilities? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment
thestinger
Nov 4, 2016
Contributor
@brianmcgillion Yeah, that explains why it doesn't work. There's a similar issue when trying to use exec spawning for system_server itself. Ambient capabilities seem like a good way to fix this. Maybe someone feels like working on it.
|
@brianmcgillion Yeah, that explains why it doesn't work. There's a similar issue when trying to use exec spawning for system_server itself. Ambient capabilities seem like a good way to fix this. Maybe someone feels like working on it. |
thestinger
changed the title from
exec spawning breaks the bluetooth app
to
exec spawning loses capabilities, breaking bluetooth and wakelocks
Nov 6, 2016
thestinger
referenced this issue
Nov 6, 2016
Closed
give system_server different ASLR bases than the zygote #45
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment
brianmcgillion
Nov 6, 2016
minijail to the rescue. I have a PoC working and the moment and tested for bluetooth, I have not tackled the system_server yet. It may be as easy as to pass another paramater like targetSdkVersion is passed so that on the callback the classloader is generated and then forwarded onto applicationInit() in RuntimeInit.java.
I will send you a mail to see if we can synchronize this first part.
brianmcgillion
commented
Nov 6, 2016
|
minijail to the rescue. I have a PoC working and the moment and tested for bluetooth, I have not tackled the system_server yet. It may be as easy as to pass another paramater like targetSdkVersion is passed so that on the callback the classloader is generated and then forwarded onto applicationInit() in RuntimeInit.java. I will send you a mail to see if we can synchronize this first part. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment|
I have the code for classLoader with system_server already. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment
thestinger
Nov 8, 2016
Contributor
From 6de903d7c9666d5d07b8fbc73b3f7bbb06ce798a Mon Sep 17 00:00:00 2001
From: Daniel Micay <danielmicay@gmail.com>
Date: Thu, 9 Jun 2016 22:18:57 -0400
Subject: [PATCH] attempt to fix system_server exec spawning
Change-Id: I97d528ab304debeb473aadc76d459323369a38ed
---
core/java/com/android/internal/os/ExecInit.java | 22 +++++++++++++++++-----
core/java/com/android/internal/os/RuntimeInit.java | 4 ++--
.../com/android/internal/os/ZygoteConnection.java | 2 +-
core/java/com/android/internal/os/ZygoteInit.java | 12 ++----------
services/java/com/android/server/SystemServer.java | 17 -----------------
5 files changed, 22 insertions(+), 35 deletions(-)
diff --git a/core/java/com/android/internal/os/ExecInit.java b/core/java/com/android/internal/os/ExecInit.java
index adb779e..6cceaa3 100644
--- a/core/java/com/android/internal/os/ExecInit.java
+++ b/core/java/com/android/internal/os/ExecInit.java
@@ -16,6 +16,7 @@
package com.android.internal.os;
+import dalvik.system.PathClassLoader;
import dalvik.system.VMRuntime;
import android.system.ErrnoException;
import android.system.Os;
@@ -44,11 +45,21 @@ public class ExecInit {
try {
// Parse our mandatory argument.
int targetSdkVersion = Integer.parseInt(args[0], 10);
+ boolean isSystemServer = Boolean.parseBoolean(args[1]);
+
+ ClassLoader cl = null;
+ if (isSystemServer) {
+ final String systemServerClasspath = Os.getenv("SYSTEMSERVERCLASSPATH");
+ if (systemServerClasspath != null) {
+ cl = new PathClassLoader(systemServerClasspath, ClassLoader.getSystemClassLoader());
+ Thread.currentThread().setContextClassLoader(cl);
+ }
+ }
// Launch the application.
- String[] runtimeArgs = new String[args.length - 1];
- System.arraycopy(args, 1, runtimeArgs, 0, runtimeArgs.length);
- RuntimeInit.execInit(targetSdkVersion, runtimeArgs);
+ String[] runtimeArgs = new String[args.length - 2];
+ System.arraycopy(args, 2, runtimeArgs, 0, runtimeArgs.length);
+ RuntimeInit.execInit(targetSdkVersion, runtimeArgs, cl);
} catch (ZygoteInit.MethodAndArgsCaller caller) {
caller.run();
}
@@ -63,9 +74,9 @@ public class ExecInit {
* @param args Arguments for {@link RuntimeInit#main}.
*/
public static void execApplication(String niceName, int targetSdkVersion,
- String instructionSet, String[] args) {
+ String instructionSet, boolean isSystemServer, String[] args) {
int niceArgs = niceName == null ? 0 : 1;
- int baseArgs = 5 + niceArgs;
+ int baseArgs = 6 + niceArgs;
String[] argv = new String[baseArgs + args.length];
if (VMRuntime.is64BitInstructionSet(instructionSet)) {
argv[0] = "/system/bin/app_process64";
@@ -79,6 +90,7 @@ public class ExecInit {
}
argv[3 + niceArgs] = "com.android.internal.os.ExecInit";
argv[4 + niceArgs] = Integer.toString(targetSdkVersion);
+ argv[5 + niceArgs] = Boolean.toString(isSystemServer);
System.arraycopy(args, 0, argv, baseArgs, args.length);
try {
diff --git a/core/java/com/android/internal/os/RuntimeInit.java b/core/java/com/android/internal/os/RuntimeInit.java
index baaf0f5..f46fb8d 100644
--- a/core/java/com/android/internal/os/RuntimeInit.java
+++ b/core/java/com/android/internal/os/RuntimeInit.java
@@ -306,11 +306,11 @@ public class RuntimeInit {
* @param targetSdkVersion target SDK version
* @param argv arg strings
*/
- public static void execInit(int targetSdkVersion, String[] argv)
+ public static void execInit(int targetSdkVersion, String[] argv, ClassLoader classLoader)
throws ZygoteInit.MethodAndArgsCaller {
if (DEBUG) Slog.d(TAG, "RuntimeInit: Starting application from zygote with exec");
- applicationInit(targetSdkVersion, argv, null);
+ applicationInit(targetSdkVersion, argv, classLoader);
}
private static void applicationInit(int targetSdkVersion, String[] argv, ClassLoader classLoader)
diff --git a/core/java/com/android/internal/os/ZygoteConnection.java b/core/java/com/android/internal/os/ZygoteConnection.java
index a33d678..d425c6c 100644
--- a/core/java/com/android/internal/os/ZygoteConnection.java
+++ b/core/java/com/android/internal/os/ZygoteConnection.java
@@ -753,7 +753,7 @@ class ZygoteConnection {
pipeFd, parsedArgs.remainingArgs);
} else {
ExecInit.execApplication(parsedArgs.niceName, parsedArgs.targetSdkVersion,
- VMRuntime.getCurrentInstructionSet(), parsedArgs.remainingArgs);
+ VMRuntime.getCurrentInstructionSet(), false, parsedArgs.remainingArgs);
}
}
diff --git a/core/java/com/android/internal/os/ZygoteInit.java b/core/java/com/android/internal/os/ZygoteInit.java
index a9c7028..55d2747 100644
--- a/core/java/com/android/internal/os/ZygoteInit.java
+++ b/core/java/com/android/internal/os/ZygoteInit.java
@@ -446,16 +446,8 @@ public class ZygoteInit {
parsedArgs.niceName, parsedArgs.targetSdkVersion,
VMRuntime.getCurrentInstructionSet(), null, args);
} else {
- ClassLoader cl = null;
- if (systemServerClasspath != null) {
- cl = new PathClassLoader(systemServerClasspath, ClassLoader.getSystemClassLoader());
- Thread.currentThread().setContextClassLoader(cl);
- }
-
- /*
- * Pass the remaining arguments to SystemServer.
- */
- RuntimeInit.zygoteInit(parsedArgs.targetSdkVersion, parsedArgs.remainingArgs, cl);
+ ExecInit.execApplication(parsedArgs.niceName, parsedArgs.targetSdkVersion,
+ VMRuntime.getCurrentInstructionSet(), true, parsedArgs.remainingArgs);
}
/* should never reach here */
diff --git a/services/java/com/android/server/SystemServer.java b/services/java/com/android/server/SystemServer.java
index 7dd16d1..5e8715a 100644
--- a/services/java/com/android/server/SystemServer.java
+++ b/services/java/com/android/server/SystemServer.java
@@ -540,7 +540,6 @@ public final class SystemServer {
CountryDetectorService countryDetector = null;
TextServicesManagerService tsms = null;
LockSettingsService lockSettings = null;
- AssetAtlasService atlas = null;
MediaRouterService mediaRouter = null;
// Bring up services needed for UI.
@@ -932,16 +931,6 @@ public final class SystemServer {
mSystemServiceManager.startService(DreamManagerService.class);
}
- if (!disableNonCoreServices) {
- try {
- Slog.i(TAG, "Assets Atlas Service");
- atlas = new AssetAtlasService(context);
- ServiceManager.addService(AssetAtlasService.ASSET_ATLAS_SERVICE, atlas);
- } catch (Throwable e) {
- reportWtf("starting AssetAtlasService", e);
- }
- }
-
if (!disableNonCoreServices) {
ServiceManager.addService(GraphicsStatsService.GRAPHICS_STATS_SERVICE,
new GraphicsStatsService(context));
@@ -1081,7 +1070,6 @@ public final class SystemServer {
final CommonTimeManagementService commonTimeMgmtServiceF = commonTimeMgmtService;
final TextServicesManagerService textServiceManagerServiceF = tsms;
final StatusBarManagerService statusBarF = statusBar;
- final AssetAtlasService atlasF = atlas;
final InputManagerService inputManagerF = inputManager;
final TelephonyRegistry telephonyRegistryF = telephonyRegistry;
final MediaRouterService mediaRouterF = mediaRouter;
@@ -1190,11 +1178,6 @@ public final class SystemServer {
reportWtf("Notifying TextServicesManagerService running", e);
}
try {
- if (atlasF != null) atlasF.systemRunning();
- } catch (Throwable e) {
- reportWtf("Notifying AssetAtlasService running", e);
- }
- try {
// TODO(BT) Pass parameter to input manager
if (inputManagerF != null) inputManagerF.systemRunning();
} catch (Throwable e) {
--
2.10.2
From 6de903d7c9666d5d07b8fbc73b3f7bbb06ce798a Mon Sep 17 00:00:00 2001
From: Daniel Micay <danielmicay@gmail.com>
Date: Thu, 9 Jun 2016 22:18:57 -0400
Subject: [PATCH] attempt to fix system_server exec spawning
Change-Id: I97d528ab304debeb473aadc76d459323369a38ed
---
core/java/com/android/internal/os/ExecInit.java | 22 +++++++++++++++++-----
core/java/com/android/internal/os/RuntimeInit.java | 4 ++--
.../com/android/internal/os/ZygoteConnection.java | 2 +-
core/java/com/android/internal/os/ZygoteInit.java | 12 ++----------
services/java/com/android/server/SystemServer.java | 17 -----------------
5 files changed, 22 insertions(+), 35 deletions(-)
diff --git a/core/java/com/android/internal/os/ExecInit.java b/core/java/com/android/internal/os/ExecInit.java
index adb779e..6cceaa3 100644
--- a/core/java/com/android/internal/os/ExecInit.java
+++ b/core/java/com/android/internal/os/ExecInit.java
@@ -16,6 +16,7 @@
package com.android.internal.os;
+import dalvik.system.PathClassLoader;
import dalvik.system.VMRuntime;
import android.system.ErrnoException;
import android.system.Os;
@@ -44,11 +45,21 @@ public class ExecInit {
try {
// Parse our mandatory argument.
int targetSdkVersion = Integer.parseInt(args[0], 10);
+ boolean isSystemServer = Boolean.parseBoolean(args[1]);
+
+ ClassLoader cl = null;
+ if (isSystemServer) {
+ final String systemServerClasspath = Os.getenv("SYSTEMSERVERCLASSPATH");
+ if (systemServerClasspath != null) {
+ cl = new PathClassLoader(systemServerClasspath, ClassLoader.getSystemClassLoader());
+ Thread.currentThread().setContextClassLoader(cl);
+ }
+ }
// Launch the application.
- String[] runtimeArgs = new String[args.length - 1];
- System.arraycopy(args, 1, runtimeArgs, 0, runtimeArgs.length);
- RuntimeInit.execInit(targetSdkVersion, runtimeArgs);
+ String[] runtimeArgs = new String[args.length - 2];
+ System.arraycopy(args, 2, runtimeArgs, 0, runtimeArgs.length);
+ RuntimeInit.execInit(targetSdkVersion, runtimeArgs, cl);
} catch (ZygoteInit.MethodAndArgsCaller caller) {
caller.run();
}
@@ -63,9 +74,9 @@ public class ExecInit {
* @param args Arguments for {@link RuntimeInit#main}.
*/
public static void execApplication(String niceName, int targetSdkVersion,
- String instructionSet, String[] args) {
+ String instructionSet, boolean isSystemServer, String[] args) {
int niceArgs = niceName == null ? 0 : 1;
- int baseArgs = 5 + niceArgs;
+ int baseArgs = 6 + niceArgs;
String[] argv = new String[baseArgs + args.length];
if (VMRuntime.is64BitInstructionSet(instructionSet)) {
argv[0] = "/system/bin/app_process64";
@@ -79,6 +90,7 @@ public class ExecInit {
}
argv[3 + niceArgs] = "com.android.internal.os.ExecInit";
argv[4 + niceArgs] = Integer.toString(targetSdkVersion);
+ argv[5 + niceArgs] = Boolean.toString(isSystemServer);
System.arraycopy(args, 0, argv, baseArgs, args.length);
try {
diff --git a/core/java/com/android/internal/os/RuntimeInit.java b/core/java/com/android/internal/os/RuntimeInit.java
index baaf0f5..f46fb8d 100644
--- a/core/java/com/android/internal/os/RuntimeInit.java
+++ b/core/java/com/android/internal/os/RuntimeInit.java
@@ -306,11 +306,11 @@ public class RuntimeInit {
* @param targetSdkVersion target SDK version
* @param argv arg strings
*/
- public static void execInit(int targetSdkVersion, String[] argv)
+ public static void execInit(int targetSdkVersion, String[] argv, ClassLoader classLoader)
throws ZygoteInit.MethodAndArgsCaller {
if (DEBUG) Slog.d(TAG, "RuntimeInit: Starting application from zygote with exec");
- applicationInit(targetSdkVersion, argv, null);
+ applicationInit(targetSdkVersion, argv, classLoader);
}
private static void applicationInit(int targetSdkVersion, String[] argv, ClassLoader classLoader)
diff --git a/core/java/com/android/internal/os/ZygoteConnection.java b/core/java/com/android/internal/os/ZygoteConnection.java
index a33d678..d425c6c 100644
--- a/core/java/com/android/internal/os/ZygoteConnection.java
+++ b/core/java/com/android/internal/os/ZygoteConnection.java
@@ -753,7 +753,7 @@ class ZygoteConnection {
pipeFd, parsedArgs.remainingArgs);
} else {
ExecInit.execApplication(parsedArgs.niceName, parsedArgs.targetSdkVersion,
- VMRuntime.getCurrentInstructionSet(), parsedArgs.remainingArgs);
+ VMRuntime.getCurrentInstructionSet(), false, parsedArgs.remainingArgs);
}
}
diff --git a/core/java/com/android/internal/os/ZygoteInit.java b/core/java/com/android/internal/os/ZygoteInit.java
index a9c7028..55d2747 100644
--- a/core/java/com/android/internal/os/ZygoteInit.java
+++ b/core/java/com/android/internal/os/ZygoteInit.java
@@ -446,16 +446,8 @@ public class ZygoteInit {
parsedArgs.niceName, parsedArgs.targetSdkVersion,
VMRuntime.getCurrentInstructionSet(), null, args);
} else {
- ClassLoader cl = null;
- if (systemServerClasspath != null) {
- cl = new PathClassLoader(systemServerClasspath, ClassLoader.getSystemClassLoader());
- Thread.currentThread().setContextClassLoader(cl);
- }
-
- /*
- * Pass the remaining arguments to SystemServer.
- */
- RuntimeInit.zygoteInit(parsedArgs.targetSdkVersion, parsedArgs.remainingArgs, cl);
+ ExecInit.execApplication(parsedArgs.niceName, parsedArgs.targetSdkVersion,
+ VMRuntime.getCurrentInstructionSet(), true, parsedArgs.remainingArgs);
}
/* should never reach here */
diff --git a/services/java/com/android/server/SystemServer.java b/services/java/com/android/server/SystemServer.java
index 7dd16d1..5e8715a 100644
--- a/services/java/com/android/server/SystemServer.java
+++ b/services/java/com/android/server/SystemServer.java
@@ -540,7 +540,6 @@ public final class SystemServer {
CountryDetectorService countryDetector = null;
TextServicesManagerService tsms = null;
LockSettingsService lockSettings = null;
- AssetAtlasService atlas = null;
MediaRouterService mediaRouter = null;
// Bring up services needed for UI.
@@ -932,16 +931,6 @@ public final class SystemServer {
mSystemServiceManager.startService(DreamManagerService.class);
}
- if (!disableNonCoreServices) {
- try {
- Slog.i(TAG, "Assets Atlas Service");
- atlas = new AssetAtlasService(context);
- ServiceManager.addService(AssetAtlasService.ASSET_ATLAS_SERVICE, atlas);
- } catch (Throwable e) {
- reportWtf("starting AssetAtlasService", e);
- }
- }
-
if (!disableNonCoreServices) {
ServiceManager.addService(GraphicsStatsService.GRAPHICS_STATS_SERVICE,
new GraphicsStatsService(context));
@@ -1081,7 +1070,6 @@ public final class SystemServer {
final CommonTimeManagementService commonTimeMgmtServiceF = commonTimeMgmtService;
final TextServicesManagerService textServiceManagerServiceF = tsms;
final StatusBarManagerService statusBarF = statusBar;
- final AssetAtlasService atlasF = atlas;
final InputManagerService inputManagerF = inputManager;
final TelephonyRegistry telephonyRegistryF = telephonyRegistry;
final MediaRouterService mediaRouterF = mediaRouter;
@@ -1190,11 +1178,6 @@ public final class SystemServer {
reportWtf("Notifying TextServicesManagerService running", e);
}
try {
- if (atlasF != null) atlasF.systemRunning();
- } catch (Throwable e) {
- reportWtf("Notifying AssetAtlasService running", e);
- }
- try {
// TODO(BT) Pass parameter to input manager
if (inputManagerF != null) inputManagerF.systemRunning();
} catch (Throwable e) {
--
2.10.2 |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment
thestinger
Nov 8, 2016
Contributor
That's old, so it might not apply cleanly anymore. IIRC, there's also an additional SELinux policy rule that's required. It needs to be able to execute app_process.
|
That's old, so it might not apply cleanly anymore. IIRC, there's also an additional SELinux policy rule that's required. It needs to be able to execute app_process. |
thestinger
added
the
Priority: high
label
Jun 11, 2017
thestinger
added
History: past-feature
and removed
nougat-port
labels
Jul 12, 2017
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment|
This has been implemented upstream which we can leverage. |
thestinger
closed this
Aug 24, 2017
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment
brianmcgillion
commented
Aug 27, 2017
|
Do you have a link to the upstream patches? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Show comment Hide comment
thestinger
Aug 27, 2017
Contributor
It's in oreo-r6-release. I don't have a link to when they added it as I didn't bother looking for the history of it.
|
It's in oreo-r6-release. I don't have a link to when they added it as I didn't bother looking for the history of it. |
thestinger commentedAug 31, 2016
•
edited
Edited 1 time
-
thestinger
edited Sep 1, 2016
This appears to be a permission issue. It can't initialize the functionality it needs when it hasn't been inherited from the Zygote.