Bug 1797175 - Assert that base::GetProcId isn't given an invalid handle. r=nika

This is an adaptation of an improvement that landed upstream in Chromium
(https://crbug.com/869154), to assert that `GetProcessId` doesn't fail
with an invalid handle error; it apparently helped them track down some
use-after-close bugs, so it might be useful.  There are two changes:

1. The assertion is enabled only if `MOZ_DIAGNOSTIC_ASSERT` would be.

2. This patch allows both `kInvalidProcessHandle` as well as null
   handles to simply fail, rather than passing them to the OS and
   setting off the assertion.  Upstream uses only nullptr for optional
   handles (and we should too, but that's beyond the scope of this
   patch).

Differential Revision: https://phabricator.services.mozilla.com/D160297
This commit is contained in:
Jed Davis 2022-10-28 20:45:45 +00:00
parent dcbf0c91ef
commit 2d803a76e0

View File

@ -93,8 +93,17 @@ void CloseProcessHandle(ProcessHandle process) {
}
ProcessId GetProcId(ProcessHandle process) {
if (process == base::kInvalidProcessHandle || process == nullptr) {
return 0;
}
// This returns 0 if we have insufficient rights to query the process handle.
return GetProcessId(process);
// Invalid handles or non-process handles will cause a diagnostic assert.
ProcessId result = GetProcessId(process);
#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
CHECK(result != 0 || GetLastError() != ERROR_INVALID_HANDLE)
<< "process handle = " << process;
#endif
return result;
}
// from sandbox_policy_base.cc in a later version of the chromium ipc code...