Closed Bug 1343752 Opened 7 years ago Closed 7 years ago

Guard against modules list shrinking between EnumProcessModules calls

Categories

(Toolkit :: Telemetry, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: away, Assigned: away)

References

Details

Attachments

(2 files, 2 obsolete files)

Attached patch patch (obsolete) — Splinter Review
This seems very unlikely, but from my understanding of the docs, it's not guaranteed impossible.
Attachment #8842719 - Flags: review?(mcastelluccio)
Attached patch Same patch for media code (obsolete) — Splinter Review
Assignee: nobody → dmajor
Attachment #8842723 - Flags: review?(cpearce)
Isn't this going to cause a crash if the list grows between the calls?
To elaborate, if the list of modules were to grow, EnumProcessModules would fill up hMods safely, but set modulesSize to a value larger than before. The current code safely ignores that, but with the changes there might be an out-of-bounds access in the for loop.

What about resetting modulesNum only if the list shrinked?
> if (modulesSize / sizeof(HMODULE) < modulesNum) {
>   modulesNum = modulesSize / sizeof(HMODULE);
> }
Good point!
Attachment #8842719 - Flags: review?(mcastelluccio)
Attachment #8842723 - Flags: review?(cpearce)
Thanks for the catch.
Attachment #8842719 - Attachment is obsolete: true
Attachment #8842723 - Attachment is obsolete: true
Attachment #8843083 - Flags: review?(mcastelluccio)
Attached patch patch for mediaSplinter Review
Attachment #8843123 - Flags: review?(cpearce)
Attachment #8843123 - Flags: review?(cpearce) → review+
Attachment #8843083 - Flags: review?(mcastelluccio) → review+
Pushed by dmajor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4087fb7a5fb5
Guard against modules list shrinking between EnumProcessModules calls (media/) r=cpearce
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c3ed29c6ffb
Guard against modules list shrinking between EnumProcessModules calls. (telemetry/) r=marco
https://hg.mozilla.org/mozilla-central/rev/4087fb7a5fb5
https://hg.mozilla.org/mozilla-central/rev/7c3ed29c6ffb
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.