Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1126a08d authored by Mitch Phillips's avatar Mitch Phillips
Browse files

New ActivityManager<->debuggerd protocol for recoverable crashes. 

Recoverable GWP-ASan catches memory safety bugs, prints debuggerd
reports, but then is able to patch up the process so that a crash
doesn't have to occur. Currently, because AM receives an IPC from
debuggerd, this results in the app being killed.

We previously worked around this by just making debuggerd not send an
IPC for GWP-ASan crashes, but this was overzealous and resulted in the
debuggerd crash not finding its way to dropbox.

So, to handle this, we introduce a new API between debuggerd and
ActivityManager.

For recoverable crashes, introduce a new log tag and extend other stats
logging areas to also add fields to indicate the crash was recoverable.
Readers of these fields should assume that, if the recoverable field is
set, the app didn't actually crash but the app had a recoverable memory
safety violation.

The GWP-ASan CTS additions (in this topic) cover the behaviour and
ensure that the recoverable crashes make their way to dropbox.

Bug: 270720163
Test: atest CtsGwpAsanTestCases
Change-Id: If3515d30a8dcacd9434b30751ae735b050d88e7d
parent 627b6459

services/core/java/com/android/server/am/ActivityManagerService.java

+11 −2
Original line number Diff line number Diff line
@@ -8405,13 +8405,16 @@ public class ActivityManagerService extends IActivityManager.Stub 
            }

        }













        boolean recoverable = eventType.equals("native_recoverable_crash");



























        EventLogTags.writeAmCrash(Binder.getCallingPid(),















                UserHandle.getUserId(Binder.getCallingUid()), processName,















                r == null ? -1 : r.info.flags,















                crashInfo.exceptionClassName,















                crashInfo.exceptionMessage,















                crashInfo.throwFileName,













                crashInfo.throwLineNumber);













                crashInfo.throwLineNumber,













                recoverable ? 1 : 0);





























        int processClassEnum = processName.equals("system_server") ? ServerProtoEnums.SYSTEM_SERVER















                : (r != null) ? r.getProcessClassEnum()










@@ -8479,8 +8482,14 @@ public class ActivityManagerService extends IActivityManager.Stub













                eventType, r, processName, null, null, null, null, null, null, crashInfo,















                new Float(loadingProgress), incrementalMetrics, null);



























        // For GWP-ASan recoverable crashes, don't make the app crash (the whole point of













        // 'recoverable' is that the app doesn't crash). Normally, for nonrecoreable native crashes,













        // debuggerd will terminate the process, but there's a backup where ActivityManager will













        // also kill it. Avoid that.













        if (!recoverable) {















            mAppErrors.crashApplication(r, crashInfo);















        }













    }





























    public void handleApplicationStrictModeViolation(















            IBinder app,

































services/core/java/com/android/server/am/EventLogTags.logtags



+1
−1




























Original line number
Diff line number
Diff line








@@ -53,7 +53,7 @@ option java_package com.android.server.am













30037 am_process_start_timeout (User|1|5),(PID|1|5),(UID|1|5),(Process Name|3)































# Unhandled exception













30039 am_crash (User|1|5),(PID|1|5),(Process Name|3),(Flags|1|5),(Exception|3),(Message|3),(File|3),(Line|1|5)













30039 am_crash (User|1|5),(PID|1|5),(Process Name|3),(Flags|1|5),(Exception|3),(Message|3),(File|3),(Line|1|5),(Recoverable|1|5)















# Log.wtf() called















30040 am_wtf (User|1|5),(PID|1|5),(Process Name|3),(Flags|1|5),(Tag|3),(Message|3)

















































services/core/java/com/android/server/am/NativeCrashListener.java



+76
−59




























Original line number
Diff line number
Diff line








@@ -64,12 +64,15 @@ final class NativeCrashListener extends Thread {













    class NativeCrashReporter extends Thread {















        ProcessRecord mApp;















        int mSignal;













        boolean mGwpAsanRecoverableCrash;















        String mCrashReport;





























        NativeCrashReporter(ProcessRecord app, int signal, String report) {













        NativeCrashReporter(ProcessRecord app, int signal, boolean gwpAsanRecoverableCrash,













                            String report) {















            super("NativeCrashReport");















            mApp = app;















            mSignal = signal;













            mGwpAsanRecoverableCrash = gwpAsanRecoverableCrash;















            mCrashReport = report;















        }

























@@ -85,7 +88,9 @@ final class NativeCrashListener extends Thread {













                ci.stackTrace = mCrashReport;































                if (DEBUG) Slog.v(TAG, "Calling handleApplicationCrash()");













                mAm.handleApplicationCrashInner("native_crash", mApp, mApp.processName, ci);













                mAm.handleApplicationCrashInner(













                        mGwpAsanRecoverableCrash ? "native_recoverable_crash" : "native_crash",













                        mApp, mApp.processName, ci);















                if (DEBUG) Slog.v(TAG, "<-- handleApplicationCrash() returned");















            } catch (Exception e) {















                Slog.e(TAG, "Unable to report native crash", e);










@@ -207,9 +212,14 @@ final class NativeCrashListener extends Thread {













            // permits crash_dump to connect to it. This allows us to trust the















            // received values.





























            // first, the pid and signal number













            int headerBytes = readExactly(fd, buf, 0, 8);













            if (headerBytes != 8) {













            // Activity Manager protocol:













            //  - 32-bit network-byte-order: pid













            //  - 32-bit network-byte-order: signal number













            //  - byte: gwpAsanRecoverableCrash













            //  - bytes: raw text of the dump













            //  - null terminator













            int headerBytes = readExactly(fd, buf, 0, 9);













            if (headerBytes != 9) {















                // protocol failure; give up















                Slog.e(TAG, "Unable to read from debuggerd");















                return;









@@ -217,17 +227,26 @@ final class NativeCrashListener extends Thread {





























            int pid = unpackInt(buf, 0);















            int signal = unpackInt(buf, 4);













            boolean gwpAsanRecoverableCrash = buf[8] != 0;















            if (DEBUG) {













                Slog.v(TAG, "Read pid=" + pid + " signal=" + signal);













                Slog.v(TAG, "Read pid=" + pid + " signal=" + signal













                        + " recoverable=" + gwpAsanRecoverableCrash);













            }













            if (pid < 0) {













                Slog.e(TAG, "Bogus pid!");













                return;















            }































            // now the text of the dump













            if (pid > 0) {















            final ProcessRecord pr;















            synchronized (mAm.mPidsSelfLocked) {















                pr = mAm.mPidsSelfLocked.get(pid);















            }













                if (pr != null) {













            if (pr == null) {













                Slog.w(TAG, "Couldn't find ProcessRecord for pid " + pid);













                return;













            }





























            // Don't attempt crash reporting for persistent apps















            if (pr.isPersistent()) {















                if (DEBUG) {










@@ -259,27 +278,25 @@ final class NativeCrashListener extends Thread {













            if (DEBUG) Slog.v(TAG, "processing");































            // Mark the process record as being a native crash so that the













                    // cleanup mechanism knows we're still submitting the report













                    // even though the process will vanish as soon as we let













                    // debuggerd proceed.













            // cleanup mechanism knows we're still submitting the report even













            // though the process will vanish as soon as we let debuggerd













            // proceed. This isn't relevant for recoverable crashes, as we don't













            // show the user an "app crashed" dialogue because the app (by













            // design) didn't crash.













            if (!gwpAsanRecoverableCrash) {















                synchronized (mAm) {















                    synchronized (mAm.mProcLock) {















                        pr.mErrorState.setCrashing(true);















                        pr.mErrorState.setForceCrashReport(true);















                    }















                }













            }































            // Crash reporting is synchronous but we want to let debuggerd















            // go about it business right away, so we spin off the actual















            // reporting logic on a thread and let it take it's time.















            final String reportString = new String(os.toByteArray(), "UTF-8");













                    (new NativeCrashReporter(pr, signal, reportString)).start();













                } else {













                    Slog.w(TAG, "Couldn't find ProcessRecord for pid " + pid);













                }













            } else {













                Slog.e(TAG, "Bogus pid!");













            }













            (new NativeCrashReporter(pr, signal, gwpAsanRecoverableCrash, reportString)).start();















        } catch (Exception e) {















            Slog.e(TAG, "Exception dealing with report", e);















            // ugh, fail.