1047549 - Copy libraries out of the APK

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Richard Newman [:rnewman]]

Unfortunately, we don't have write access to our library directories, so we can't fix what Android broke. But we can -- if there's enough space -- try copying them into our files directory, which will at least get the browser running.

This is an awful fallback, and should be accompanied by a notification saying "Firefox is in screwed-up mode; please reinstall".

We shouldn't attempt this when we're in /mismatched_uid. That should be a notification and quit.

Comment 1

[Richard Newman [:rnewman]]

Attachment: Copy libraries out of the APK when they're missing. v1

I haven't thoroughly tested this yet (only the error states that led to its particular design), but it should work.

I would like to do more before we ship this, though: telemetry, a notification begging forgiveness, etc.

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 8466361 [details] [diff] [review]
Copy libraries out of the APK when they're missing. v1

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

::: mobile/android/base/mozglue/GeckoLoader.java.in
@@ +304,5 @@
> +
> +                final InputStream in = zipFile.getInputStream(entry);
> +                try {
> +                    final String outPath = outDir + "/lib" + lib + ".so";
> +                    final FileOutputStream out = new FileOutputStream(outPath);

presumably we have to make this executable to be useful. Have you tried testing loading this library directly?

Comment 3

[Richard Newman [:rnewman]]

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #2)

> presumably we have to make this executable to be useful. Have you tried
> testing loading this library directly?

Not yet. (That should really have been f?, but bzexport is flaky when it comes to feedback flags.)

Thoughts on additional work mentioned in Comment 1?

Comment 4

[Richard Newman [:rnewman]]

Testing suggests that the copied library loads correctly when `new File(path).canExecute()` returns false. So apparently the linker doesn't care.

Testing hardware: Kindle Fire HD, ARMv7, internal memory.

(From what I've read it only cares that it can mmap the file and is not on a noexec filesystem, so that means this won't work on the raw SD card. This is level 6 of our fallbacks, so I don't care too much.)

I'm open to the possibility that I'm testing this wrong!

Comment 5

[Richard Newman [:rnewman]]

Note that the Android-extracted libraries in a correct install *are* executable by this measurement.

08-01 18:40:38.050 I/GeckoLoader(25741): XXX: Loading /data/data/org.mozilla.fennec_rnewman/lib/libmozglue.so
08-01 18:40:38.050 I/GeckoLoader(25741): XXX: Executable true

Comment 6

[Richard Newman [:rnewman]]

Attachment: Copy libraries out of the APK when they're missing. v2

Tested. It works.

Additional changes:

* If we fail in the copy for any exceptional reason, we delete the output file.
* Despite testing showing no need for it, we mark the output as world-executable.

Comment 7

[Richard Newman [:rnewman]]

Attachment: Copy libraries out of the APK when they're missing. v2

Tested. It works.

Additional changes:

* If we fail in the copy for any exceptional reason, we delete the output file.
* Despite testing showing no need for it, we mark the output as world-executable.

Comment 8

[Richard Newman [:rnewman]]

Comment on attachment 8466565 [details] [diff] [review]
Copy libraries out of the APK when they're missing. v2

Oh, Bugzilla API.

Comment 9

[Kevin Brosnan [Ex-Mozilla]]

Is there a way to do this without being world executable? We have seen security researchers make use of that property.

Comment 10

[Richard Newman [:rnewman]]

(In reply to Kevin Brosnan [:kbrosnan] from comment #9)
> Is there a way to do this without being world executable? We have seen
> security researchers make use of that property.

We can make it user-executable, but I don't know what the implications are of that.

The copied lib is in the app's private files area, so in theory this choice is academic.

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 8466566 [details] [diff] [review]
Copy libraries out of the APK when they're missing. v2

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

Note that this is chatty on the logs. I think that's ok since this only kicks in when the standard code paths are failing.

::: mobile/android/base/mozglue/GeckoLoader.java.in
@@ +342,5 @@
> +            } finally {
> +                zipFile.close();
> +            }
> +        } catch (Exception e) {
> +            Log.e(LOGTAG, "Failed to extract lib from APK.", e);

note for future consideration. My guess is there will be a set of reasons why the extraction fails which we can boil down into sensible error messages for the user such as "Firefox couldn't start because your device is low on memory." It is fine to eat the exceptions for now, but I suspect we'll want them (or perhaps a reasons enum) to send back to the caller to do something intelligent with

Comment 12

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/a6d41807dcb8

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a6d41807dcb8

Comment 14

[Richard Newman [:rnewman]]

Comment on attachment 8466566 [details] [diff] [review]
Copy libraries out of the APK when they're missing. v2

One step further in our Google Play-only misinstalled recovery attempts. Requesting uplift for that.

Comment 15

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8466566 [details] [diff] [review]
Copy libraries out of the APK when they're missing. v2

rnewman and kbrosnan explained that this is a fix for when the Fennec installation is corrupted. In that case we try to recover the best that we can so that the user can backup their data and reinstall. The new code paths introduced here should only be executed when we detect that Fennec is in this busted state.

Beta+
Aurora+

Comment 16

[Aaron Train [:aaronmt]]

F/GeckoLoader( 7064): Couldn't load mozglue. Trying native library dir.
F/GeckoLoader( 7064): Library doesn't exist when it should.
F/GeckoLoader( 7064): Couldn't load /data/data/org.mozilla.fennec/lib/libmozglue.so: java.lang.UnsatisfiedLinkError: dlopen failed: library "/data/data/org.mozilla.fennec/lib/libmozglue.so" not found
D/GeckoLoader( 7064): Copying lib/armeabi-v7a/libmozglue.so to /data/data/org.mozilla.fennec/files/lib/libmozglue.so
D/GeckoLoader( 7064): Marking /data/data/org.mozilla.fennec/files/lib/libmozglue.so as executable.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2633e370a93b
https://hg.mozilla.org/releases/mozilla-beta/rev/9d8f79b400bf