Copy libraries out of the APK

VERIFIED FIXED in Firefox 32

Status

()

defect
VERIFIED FIXED
5 years ago
3 years ago

People

(Reporter: rnewman, Assigned: rnewman)

Tracking

(Blocks 2 bugs)

Trunk
Firefox 34
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox32 fixed, firefox33 fixed, firefox34 verified)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
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.
Attachment #8466361 - Flags: review?(blassey.bugs)
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?
(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?
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!
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
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.
Attachment #8466565 - Flags: review?(blassey.bugs)
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.
Attachment #8466566 - Flags: review?(blassey.bugs)
Comment on attachment 8466565 [details] [diff] [review]
Copy libraries out of the APK when they're missing. v2

Oh, Bugzilla API.
Attachment #8466565 - Attachment is obsolete: true
Attachment #8466565 - Flags: review?(blassey.bugs)
Attachment #8466361 - Attachment is obsolete: true
Attachment #8466361 - Flags: review?(blassey.bugs)
Is there a way to do this without being world executable? We have seen security researchers make use of that property.
(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 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
Attachment #8466566 - Flags: review?(blassey.bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/a6d41807dcb8
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
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.
Attachment #8466566 - Flags: approval-mozilla-beta?
Attachment #8466566 - Flags: approval-mozilla-aurora?
Blocks: 1049911
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+
Attachment #8466566 - Flags: approval-mozilla-beta?
Attachment #8466566 - Flags: approval-mozilla-beta+
Attachment #8466566 - Flags: approval-mozilla-aurora?
Attachment #8466566 - Flags: approval-mozilla-aurora+
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.