Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(163)

Issue 282523004: Update mach_override to 919148f9, stop mach_host_self port leak (Closed)

Created:
6 years, 7 months ago by Mark Mentovai
Modified:
6 years, 7 months ago
Reviewers:
Robert Sesek
CC:
chromium-reviews
Visibility:
Public.

Description

Update mach_override to 919148f9, picking up: commit 919148f94db54fc04d287eb6a42c0c36b166bbfa Merge: e2ca3c3 84a5bd9 Author: Jonathan 'Wolf' Rentzsch <jwr.git@redshed.net>; Date: Sun May 11 21:51:20 2014 -0500 Merge pull request #16 from mark-chromium/patch-1 [FIX] Stop using mach_host_self and host_page_size, fixing a port right leak. (Mark Mentovai) commit 84a5bd929213b9e0f059d3bc8c5b738e9fe4e620 Author: Mark Mentovai <mark@chromium.org>; Date: Fri May 9 16:40:10 2014 -0400 Stop using mach_host_self and host_page_size, fixing a port right leak It is incorrect to use mach_host_self without disposing of the send send right to the host port with mach_port_deallocate when done with it. http://crbug.com/105513 shows the sorts of problems that can arise when send rights aren’t properly deallocated. mach_host_self was only used by mach_override to be able to call host_page_size. host_page_size is unnecessary, because it always returns a constant value, PAGE_SIZE, which is also known at user-land compile time. See libsyscall/mach/mach_init.c. User code is better off just using this macro directly, and not fumbling with the system calls to obtain and properly dispose of a send right to the host port. (You need to mach_port_deallocate the ports you get from mach_host_self and mach_thread_self, but you must not normally deallocate the one from mach_task_self, because mach_task_self is actually just a macro that references a global variable. It doesn’t add any port rights at all. See <mach/mach_init.h>. If you bypass the macro and call the real mach_task_self system call, you do need to call mach_port_deallocate, but this situation is incredibly rare.) R=rsesek@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269785

Patch Set 1 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -58 lines) Patch
M third_party/mach_override/README.chromium View 2 chunks +3 lines, -8 lines 0 comments Download
M third_party/mach_override/mach_override.c View 5 chunks +34 lines, -50 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Mark Mentovai
6 years, 7 months ago (2014-05-12 14:07:22 UTC) #1
Robert Sesek
LGTM
6 years, 7 months ago (2014-05-12 14:10:46 UTC) #2
Mark Mentovai
6 years, 7 months ago (2014-05-12 15:34:25 UTC) #3
Message was sent while issue was closed.
Committed patchset #1 manually as r269785 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698