_recalloc in mozjemalloc/jemalloc.c uses system realloc

RESOLVED FIXED in Firefox 40

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Blocks 1 bug)

unspecified
mozilla42
All
Windows 7
Points:
---

Firefox Tracking Flags

(firefox39 unaffected, firefox40 fixed, firefox41 fixed, firefox42 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Fortunately, nothing uses it in gecko, so it's not a problem.

I'm note that:
- it's not exposed by replace-malloc
- it's not supported by jemalloc3

We might as well remove it, as well as _expand and maybe _msize. And ensure we don't break later with bug 1134914.

Kind of related: bug 558163
(Assignee)

Comment 1

4 years ago
I think this is the cause of all Symantec's problems with the Norton Toolbar with Firefox 40 (bug 1187862 and bug 1191190) so let's fix this now, and we'll see in a followup if we want to kill it entirely (which, arguably, would lead to the same problem) or what.
Assignee: nobody → mh+mozilla
Attachment #8644821 - Flags: review?(n.nethercote)
Attachment #8644821 - Flags: review?(n.nethercote) → review+
(Assignee)

Comment 3

4 years ago
Comment on attachment 8644821 [details] [diff] [review]
Do not use system realloc in mozjemalloc-exposed _recalloc

Approval Request Comment
[Feature/regressing bug #]: It's been a long standing issue, but bug 868814 made it a problem with third party components that were previously using the system allocator and are now more or less forced to use jemalloc.
[User impact if declined]: Norton Toolbar and possibly other addons, if not modified to not use the _recalloc API, trigger random crashes.
[Describe test coverage new/current, TreeHerder]: Nothing is actually using the modified function except for third party code. I did test this fixes the crashes I was getting with a reduced test-case addon from the people at Symantec.
[Risks and why]: The patch is trivial, and the function was simply not doing the right thing before. The worst thing that could happen is that the function has other bugs, which, in fact, it has two of them:
- it will do bad things if passed a pointer that wasn't allocated by jemalloc, but so will almost every other jemalloc function
- it can overflow on the multiplication between count and size, which would require the caller of the function to be trying to allocate more than 4GB of memory in the first place.
[String/UUID change made/needed]: None
Attachment #8644821 - Flags: approval-mozilla-release?
Attachment #8644821 - Flags: approval-mozilla-beta?
Attachment #8644821 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1187862
(Assignee)

Updated

4 years ago
Duplicate of this bug: 1191190
(Assignee)

Comment 6

4 years ago
39 is not affected in any interesting way although the bug exists there, essentially invisible because bug 868814 is not in 39. I flagged approval-mozilla-release because the -release repo is already 40 as of writing.
Comment on attachment 8644821 [details] [diff] [review]
Do not use system realloc in mozjemalloc-exposed _recalloc

This is a potential driver for a dot release (impact on an important addon). Taking the patch in case we do a third RC (which seems for now likely)
Attachment #8644821 - Flags: approval-mozilla-release?
Attachment #8644821 - Flags: approval-mozilla-release+
Attachment #8644821 - Flags: approval-mozilla-beta?
Attachment #8644821 - Flags: approval-mozilla-beta-
Attachment #8644821 - Flags: approval-mozilla-aurora?
Attachment #8644821 - Flags: approval-mozilla-aurora+

Comment 9

4 years ago
Hi,

Could you let us know when this fix is getting released? Will there any new RC for FF 40 which will have this fix?

Thanks
Shan'
Except a last minute change, I think we will ship 40.0 with the fix. We will start a build5 for 40.0 today.
https://hg.mozilla.org/mozilla-central/rev/3d180b8dcf42
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
We have tried to verify this by verifying the duplicates (bug 1191190, and bug 1187862), however we did not manage to install the Norton Toolbar as it gets automatically disabled after install, and the extension from https://bugzilla.mozilla.org/show_bug.cgi?id=1191190#c3 is reported as corrupted. This issue is similar to what we had in https://bugzilla.mozilla.org/show_bug.cgi?id=1151506#c117.

We have however tested various add-ons and themes in Firefox 40 RC build 5, and no issues were found related to this fix.
You need to log in before you can comment on or make changes to this bug.