815684 - Send some logcat info in crash reports

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

As of Jelly Bean, Android apps can read back the logcat info associated with their uid [1] without requiring any extra permissions. We should do this on crashes so that we have some idea of why crashes like bug 780831 (i.e. dalvik aborts with 0xdeadd00d) happen. We can restrict this to sending back the last 50 lines or the last 3 seconds of logs or something to avoid sending too much data. As long as we stop printing out sensitive user information in our logs (which we shouldn't be doing anyway) there shouldn't be any privacy concerns here.

[1] https://groups.google.com/group/android-developers/msg/22205f37ad6cf602?hl=en-GB

Comment 1

[Chris Peterson [:cpeterson]]

This is an awesome idea!

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Send 200 lines of logcat in the crash report

I originally looked at doing this in nsExceptionHandler.cpp but realized there's no point doing it there since all the crash report annotations just get parsed out in the CrashReporter.java.in anyway.

I tested it by building with this patch and the crash reporter enabled, and sending out a crash report. about:crashes lists the report ID as bp-0d7e6321-4a51-437a-a406-c8cde2121128.

:kairo, is it possible to check if the above crash report ID got an extra field called Android_Logcat with approx 29kb of logcat lines?

Comment 3

[Robert Kaiser]

(In reply to Kartikaya Gupta (:kats) from comment #2)
> :kairo, is it possible to check if the above crash report ID got an extra
> field called Android_Logcat with approx 29kb of logcat lines?

It has. If you mean with this that we'll add this HUGE field to *every* crash report from now on, I think we'll need to ramp up storage requirements on the server side (and we're pretty tight there), and it also will definitely have an impact on getting reports from phones with low-rate data connections as submission will take longer and therefore fail more often.

Comment 4

[Robert Kaiser]

So, in terms of server storage, I just heard the team is working on improving the situation anyhow - and our new Flash hang report format probably has a much higher impact than this anyhow, but we need to be aware of things like this here as well to not be surprised by larger space requirements.

On the bandwidth side, if someone implements bug 816164 in the Android crash submission code, that would probably more than mitigate that hit. :)

Comment 5

[Chris Peterson [:cpeterson]]

Comment on attachment 686147 [details] [diff] [review]
Send 200 lines of logcat in the crash report

Review of attachment 686147 [details] [diff] [review]:
-----------------------------------------------------------------

Do you know if Breakpad or Socorro has a limit on a crash report's part length? The Android_Logcat field value will be ~20 KB.

::: mobile/android/base/CrashReporter.java.in
@@ +219,5 @@
> +        BufferedReader br = null;
> +        try {
> +            // get the last 200 lines of logcat
> +            Process proc = Runtime.getRuntime().exec(new String[] {
> +                "logcat", "-v", "threadtime", "-t", "200", "-d"

adb logcat -? says "If no filterspec is found, filter defaults to '*:I'". I think we should specify '-B "*:D"' or even '-B "*:V"'.

@@ +232,5 @@
> +            return "Unable to get logcat: " + e.toString();
> +        } finally {
> +            try {
> +                br.close();
> +            } catch (Exception e) {

Do you want to check for (br != null)? I know the try/catch will catch a NullPointerException, but it seems sloppy since our dying process is already in a fragile state.

Comment 6

[Robert Kaiser]

(In reply to Chris Peterson (:cpeterson) from comment #5)
> Do you know if Breakpad or Socorro has a limit on a crash report's part
> length? The Android_Logcat field value will be ~20 KB.

I don't think we have a length limit - I'm pretty sure the new Windows Flash hang reports containing 4 minidumps are way larger than what we have here. What is a concern is the time it takes to submit the report, which esp. over slow phone connections can be problematic. AFAIK we have set a quite high timeout on Apcache on the server side for that incoming POST request, but the longer it takes, the higher the risk it's interrupted and therefore doesn't complete correctly. I think we'll need bug 816164 to improve there (this is in Android code, so just needs an Android dev to work on it!) and cut down size and therefore submission time.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

(In reply to Robert Kaiser (:[e-mail]) from comment #4)
> On the bandwidth side, if someone implements bug 816164 in the Android crash
> submission code, that would probably more than mitigate that hit. :)

I can work on that bug; marked it as a blocker of this so that we end up net-neutral or net-win in terms of bandwidth.

(In reply to Chris Peterson (:cpeterson) from comment #5)
> adb logcat -? says "If no filterspec is found, filter defaults to '*:I'". I
> think we should specify '-B "*:D"' or even '-B "*:V"'.

That's weird; when I tested it it did include D, I, W, and E logcat messages. I can add -B *:D explicitly though. I feel like we shouldn't include *:V because (a) that might flood the log and (b) it's good to have a level that won't get picked up in case we need to use it for anything. I think all of our logging is done using debug or higher level anyway right now.

> Do you want to check for (br != null)? I know the try/catch will catch a
> NullPointerException, but it seems sloppy since our dying process is already
> in a fragile state.

Good catch; I meant to do that but forgot.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Also adding privacy-review-needed. I think I do want at least a cursory examination of the privacy implications. Even though we don't (or shouldn't be) logging URLs from Java code, there might still be URLs logged to logcat from Javascript/CSS warnings and the like (stuff that usually goes to the gecko console which shows up in logcat). This data could be privacy sensitive and could impact this bug even if the logcat data isn't displayed on crash-stats by default.

If that is a problem, one possible option is to only send the logcat data when the user explicitly selects "send URL with crash report".

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Attachment: Send 200 lines of logcat in the crash report (v2)

Patch updated with cpeterson's comments (i.e. I added "*:D" and the null check).

Comment 10

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Isn't there a possibility of some of the logcat information holding private information such as telephone number or website address or something similar?  Should this also have privacy team/legal check as well?

Comment 11

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

oops.  ignore comment 10.  saw comment 8 and couldn't stop the submit.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/629f2a524de1

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/629f2a524de1

Comment 14

[chris hofmann]

re: comment 9 and privacy concerns.  

one way to help get some more data here would be to have several people look at the the logcat output on their phones to see if there is anything that turns up that they might consider private data.

can someone post instructions on how to get the logcat output by hand?

Comment 15

[chris hofmann]

so here is a reference doc

  http://developer.android.com/tools/debugging/debugging-log.html#outputFormat

and here is what we intend to run from the patch:

  "logcat", "-v", "threadtime", "-t", "200", "-d", "*:D"

I'm hunting for some kind of spec that specifies a list of the types of information that might get logged and show up in the -verbose or other logcat options but don't see anything yet.

One privacy concern here is that if andriod or the logcat tool changes to collect different kinds of more private information, or there are different types of information already being logged and sent by different versions of android, then we would just start passing that more sensistive information along given the way this is implemented.

Comment 16

[Kevin Brosnan [Ex-Mozilla]]

We can only get access to Logcat data that is created by the Firefox apk. System or other app Logcat data requires an additional permission 'read sensitive log data'.

Comment 17

[Kartikaya Gupta (email:[e-mail])]

(In reply to Kevin Brosnan [:kbrosnan] from comment #16)
> We can only get access to Logcat data that is created by the Firefox apk.

.. or the dalvikvm running firefox. But yeah in general we won't be reading data written by other processes.

(In reply to chris hofmann from comment #15)
> I'm hunting for some kind of spec that specifies a list of the types of
> information that might get logged and show up in the -verbose or other
> logcat options but don't see anything yet.
> 
> One privacy concern here is that if andriod or the logcat tool changes to
> collect different kinds of more private information, or there are different
> types of information already being logged and sent by different versions of
> android, then we would just start passing that more sensistive information
> along given the way this is implemented.

The best explanation I have found so far is the link I posted in comment 0, which is from an actual Android developer [1]. That thread is worth skimming through, in particular the message at [2].

[1] https://groups.google.com/group/android-developers/msg/22205f37ad6cf602
[2] https://groups.google.com/group/android-developers/msg/493e72478b2a4b74

Comment 18

[Kartikaya Gupta (email:[e-mail])]

(In reply to chris hofmann from comment #14)
> can someone post instructions on how to get the logcat output by hand?

Install the android SDK, plug in the device via USB and run "adb logcat" (with any options you want). Note that when you do this you will be getting the full log rather than just the stuff from the Firefox process, which is all we will be able to read programatically.

Comment 19

[Kartikaya Gupta (email:[e-mail])]

Also there is an example of the kind of data this will be gathering at https://bug780831.bugzilla.mozilla.org/attachment.cgi?id=687858 - there's nothing sensitive in this one so I attached it to the bug that it was needed for.

Comment 20

[chris hofmann]

> One privacy concern here is that if andriod or the logcat tool changes to
> collect different kinds of more private information, or there are different
> types of information already being logged and sent by different versions of
> android, then we would just start passing that more sensistive information
> along given the way this is implemented.


This article doesn't have specifics on the options needed to set up the debugging of other apps, but we should make sure that we don't leak data from other apps out though these debugging logs on gingerbread or earlier.   http://blog.trendmicro.com/trendlabs-security-intelligence/the-issues-surrounding-android-debugging/

does anyone know of that external app debugging feature involves logcat options that we are not using, or what the options are to make that happen?

Comment 21

[Kevin Brosnan [Ex-Mozilla]]

"read sensitive log data"

https://play.google.com/store/apps/details?id=org.jtb.alogcat