Closed Bug 1536697 Opened 7 months ago Closed 2 months ago

POSIX SharedMemory::Map leaves memory_ set to MAP_FAILED on error (and probably passes it to munmap)

Categories

(Core :: IPC, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: jld, Assigned: jld)

Details

Attachments

(1 file)

See also https://codereview.chromium.org/10174007, found while browing the history for the apparently nonstandard use of (void*)-1 instead of MAP_FAILED. The error case leaves memory_ set to MAP_FAILED, but other parts of the code check against null. This should be harmless in practice (we're not checking for errors from munmap, which should fail without side effects in this case) but we ought to fix it.

I fixed this while writing unit tests for bug 1479960.

Assignee: nobody → jld
Type: enhancement → defect
Priority: -- → P2

If mmap failed, we'd leave the memory_ member variable set to MAP_FAILED,
but everything else in this file checks for nullptr (and only nullptr) to
test if the pointer is valid.

Also, this removes the debug assertion that the mmap succeeded, to allow
writing unit tests where we expect it to fail (e.g., for insufficient
permissions).

Depends on D26747

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:jld, could you have a look please?

Flags: needinfo?(jld)

This is part of a series of patches that's being held until the next merge cycle.

Flags: needinfo?(jld)
Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a7c09894c6a0
Fix error handling in base::SharedMemory::Map. r=froydnj
Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7172762c4b87
Fix error handling in base::SharedMemory::Map. r=froydnj
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla69 → ---
Flags: needinfo?(jld)
Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/86cb672b7000
Fix error handling in base::SharedMemory::Map. r=froydnj
Status: REOPENED → RESOLVED
Closed: 4 months ago2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
You need to log in before you can comment on or make changes to this bug.