Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Crash loading binary components bigger than 4KB

RESOLVED FIXED in Firefox 7

Status

()

Core
Widget: Android
--
critical
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Trunk
mozilla8
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox6-, firefox7+ fixed, fennec7+)

Details

(Whiteboard: QA?)

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
The code path used to load libraries that are already mapped in memory leads to a crash if a library is bigger than a page, because load_library passes only a page worth of the library to load_segments as 'header' argument, vs. the full library when coming from load_mapped_library.
But load_segments assumes the 'header' argument to contain the whole library here:
http://hg.mozilla.org/mozilla-central/file/84034146670c/other-licenses/android/linker.c#l943
(Assignee)

Updated

6 years ago
Blocks: 668210
(Assignee)

Comment 1

6 years ago
Created attachment 544224 [details] [diff] [review]
Make our Android linker properly load libraries outside the .apk
Attachment #544224 - Flags: review?(mwu)

Comment 2

6 years ago
Comment on attachment 544224 [details] [diff] [review]
Make our Android linker properly load libraries outside the .apk

Well, looks fine. There's some additional improvements that could be made to in the case of fd's point to real files but I don't know the changes to APKOpen well enough to comment on how to do it.
Attachment #544224 - Flags: review?(mwu) → review+
(Assignee)

Comment 3

6 years ago
(In reply to comment #2)
> Comment on attachment 544224 [details] [diff] [review] [review]
> Make our Android linker properly load libraries outside the .apk
> 
> Well, looks fine. There's some additional improvements that could be made to
> in the case of fd's point to real files but I don't know the changes to
> APKOpen well enough to comment on how to do it.

Actually, this takes care of the case of real files, which is what was broken.
(Assignee)

Comment 4

6 years ago
http://hg.mozilla.org/mozilla-central/rev/337618a7bc79
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8

Comment 5

6 years ago
(In reply to comment #3)
> Actually, this takes care of the case of real files, which is what was
> broken.

I'm talking about the case where a real file FD gets passed to load_mapped_library. In that case, you would be able to not set FLAG_MMAPPED. This is not a particularly important case since it's only relevant when there are uncompressed libraries in the apk.
(Assignee)

Comment 6

6 years ago
(In reply to comment #5)
> I'm talking about the case where a real file FD gets passed to
> load_mapped_library. In that case, you would be able to not set
> FLAG_MMAPPED. This is not a particularly important case since it's only
> relevant when there are uncompressed libraries in the apk.

... which doesn't work anyway, for other reasons.
(Assignee)

Comment 7

6 years ago
Without the patch, Firefox crashes on Android when using extensions with binary components or using js-ctypes.
tracking-firefox6: --- → ?
tracking-firefox7: --- → ?
(Assignee)

Comment 8

6 years ago
(In reply to comment #7)
> Without the patch, Firefox crashes on Android when using extensions with
> binary components or using js-ctypes.

(for anything that is not in the Firefox apk ; using js-ctypes on e.g. nss works)

Comment 9

6 years ago
we're interested in this for aurora after it's baked on the trunk for a bit. not sure about beta yet.
tracking-firefox7: ? → +
It's unfortunate, but I'd guess there aren't any current extensions being impacted, because this has never worked in a shipping release, I think. It does block off one avenue of extension development, which kind of sucks, but I don't know that it's bad enough to bother branch-landing.

Comment 11

6 years ago
This is Android specific.  Release drivers want to know if this affects Firefox 6 (beta) and if we should be tracking.  We didn't have mobile quorum to decide that.
(Assignee)

Comment 12

6 years ago
I affects all versions of Firefox mobile >= 4.

Comment 13

6 years ago
the mobile team will track this but not the larger release team.
tracking-firefox6: ? → -
tracking-fennec: --- → 7+
(Assignee)

Updated

6 years ago
Attachment #544224 - Flags: approval-mozilla-aurora?

Comment 14

6 years ago
We didn't have a mobile rep at today's triage but desktop has no concerns here so I'm delegating the yes/no to which ever of the Mobile drivers gets here first.
Attachment #544224 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 15

6 years ago
http://hg.mozilla.org/releases/mozilla-aurora/rev/7a76cd285475
status-firefox7: --- → fixed

Comment 16

6 years ago
Using Android/Fennec and the above patch, I am still crashing when attempting to use dlopen() and friends from any thread using the specialized linker. This seems to happen when trying a specific library or dlopen(NULL, ...) attempting to fetch the currently loaded table.

I have created a new bug: #673937
(Assignee)

Updated

6 years ago
Duplicate of this bug: 674570
(Assignee)

Updated

6 years ago
Duplicate of this bug: 612544
(Assignee)

Comment 19

6 years ago
Note that as bug 612544 witnesses, this does affect plugin loading, too...

Updated

6 years ago
Whiteboard: QA?
You need to log in before you can comment on or make changes to this bug.