20 Commits

Author SHA1 Message Date
Lei Zhang
1675ce7c58 Add missing base/check.h includes
Do not rely on base/logging.h to provide it.

Change-Id: I8b7d733bcf66abe9b46eabd3703b7ed549d02db7
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/5007844
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2023-11-06 22:00:17 +00:00
Mark Mentovai
6278690abe Update copyright boilerplate, 2022 edition (Crashpad)
sed -i '' -E -e 's/Copyright (.+) The Crashpad Authors\. All rights reserved\.$/Copyright \1 The Crashpad Authors/' $(git grep -El 'Copyright (.+) The Crashpad Authors\. All rights reserved\.$')

Bug: chromium:1098010
Change-Id: I8d6138469ddbe3d281a5d83f64cf918ec2491611
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3878262
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2022-09-06 23:54:07 +00:00
Daniel Cheng
0affe61689 Migrate base::{size,empty,data} to STL equivalents in crashpad.
Bug: chromium:1299695
Change-Id: I95187a425b08c96430c659f843c379d506972f0f
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3496462
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2022-03-01 12:06:49 +00:00
Lei Zhang
d8f60c8112 Swap from base/stl_util.h to cxx17_backports.h.
Chromium moved base::size() to base/cxx17_backports.h, so do the same in
mini_chromium and update the users in Crashpad.

Roll mini_chromium to 2f06f83f to make the new base header available.

Bug: chromium:1210983
Change-Id: Ie3dc4c189dcdfcac030b95fe285f94abb29a27bf
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2917779
Commit-Queue: Lei Zhang <thestig@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2021-05-25 21:44:52 +00:00
Jan Wilken Dörrie
a98ee20e57 [crashpad] Prepare crashpad for base::string16 switch
This change prepares crashpad for the upcoming switch of base::string16
to std::u16string on all platforms. It does so by replacing Windows-only
instances of base::string16 with std::wstring, and using appropriate
string utility functions.

Bug: chromium:911896
Change-Id: Ibb0b8a4e4dc7fae1d24d18823f8dbb6da31f8239
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2332402
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2020-09-12 07:36:53 +00:00
James Forshaw
c6153f0b6e [Windows] Add AppContainer SID to Named Pipe DACL.
This CL modifies the creation of the Named Pipe Security Descriptor to
allow access from AppContainer processes. The DACL only allows access for
the current user and SYSTEM which matches up with the auto-assigned DACL
used previously (the read-only logon SID ACE has been removed). As this
new code uses APIs from ADVAPI32 a check is made to ensure it's not being
called while the loader lock is held to avoid hitting previous similar
issues.

Bug: crashpad: 318
Change-Id: I3f9cf5c788dbadacad21c8a2d57a0188f690ac32
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1955982
Commit-Queue: James Forshaw <forshaw@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-12-10 17:16:24 +00:00
Mark Mentovai
cc166d71f4 Use base::size where appropriate, and ArraySize elsewhere
This is a follow-up to c8a016b99d97, following the post-landing
discussion at
https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1393921/5#message-2058541d8c4505d20a990ab7734cd758e437a5f7

base::size, and std::size that will eventually replace it when C++17 is
assured, does not allow the size of non-static data members to be taken
in constant expression context. The remaining uses of ArraySize are in:

minidump/minidump_exception_writer.cc (×1)
minidump/minidump_system_info_writer.cc (×2, also uses base::size)
snapshot/cpu_context.cc (×4, also uses base::size)
util/misc/arraysize_test.cc (×10, of course)

The first of these occurs when initializing a constexpr variable. All
others are in expressions used with static_assert.

Includes:
Update mini_chromium to 737433ebade4d446643c6c07daae02a67e8deccao

f701716d9546 Add Windows ARM64 build target to mini_chromium
87a95a3d6ac2 Remove the arraysize macro
1f7255ead1f7 Placate MSVC in areas of base::size usage
737433ebade4 Add cast

Bug: chromium:837308
Change-Id: I6a5162654461b1bdd9b7b6864d0d71a734bcde19
Reviewed-on: https://chromium-review.googlesource.com/c/1396108
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
2019-01-04 22:42:57 +00:00
Avi Drissman
c8a016b99d Remove base's arraysize from Crashpad.
BUG=837308
R=mark@chromium.org

Change-Id: Ibecbfc7bc2d61ee54bc1114e4b20978adbc77db2
Reviewed-on: https://chromium-review.googlesource.com/c/1393921
Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
2019-01-03 19:44:15 +00:00
Mark Mentovai
20e5aba1af URL cleanups: switch to HTTPS, fix dead ones, use canonical ones
Change-Id: I4b247d7fae1a212350f8ffcf2bf5ba1fa730f5c1
Reviewed-on: https://chromium-review.googlesource.com/780339
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Mark Mentovai <mark@chromium.org>
2017-11-20 22:23:39 +00:00
Mark Mentovai
281be63d00 Standardize on static constexpr for arrays when possible
This uses “static” at function scope to avoid making local copies, even
in cases where the compiler can’t see that the local copy is
unnecessary. “constexpr” adds additional safety in that it prevents
global state from being initialized from any runtime dependencies, which
would be undesirable.

At namespace scope, “constexpr” is also used where appropriate.

For the most part, this was a mechanical transformation for things
matching '(^| )const [^=]*\['.

Similar transformations could be applied to non-arrays in some cases,
but there’s limited practical impact in most non-array cases relative to
arrays, there are far more use sites, and much more manual intervention
would be required.

Change-Id: I3513b739ee8b0be026f8285475cddc5f9cc81152
Reviewed-on: https://chromium-review.googlesource.com/583997
Commit-Queue: Mark Mentovai <mark@chromium.org>
Reviewed-by: Leonard Mosescu <mosescu@chromium.org>
2017-07-25 17:40:51 +00:00
Mark Mentovai
013d5e14a3 #include <stddef.h> where offsetof() is used
Bug: crashpad:30
Change-Id: If23ca9ea3141d3d34dc494aa29a1bd1dc8f83130
Reviewed-on: https://chromium-review.googlesource.com/458079
Reviewed-by: Scott Graham <scottmg@chromium.org>
2017-03-23 02:15:32 +00:00
Scott Graham
32981a3ee9 win: Fix clang warning in SECURITY_DESCRIPTOR construction
c:\src\cr\src\third_party\crashpad\crashpad\util\win\registration_protocol_win.cc(193,23):  error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
                      SECURITY_MANDATORY_LABEL_AUTHORITY,
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
c:\src\depot_tools\win_toolchain\vs_files\d5dc33b15d1b2c086f2f6632e2fd15882f80dbd3\win_sdk\Include\10.0.10586.0\um\winnt.h(9068,54):  note: expanded from macro 'SECURITY_MANDATORY_LABEL_AUTHORITY'
                                                     ^~~~~~~~~~~~
1 error generated.

R=mark@chromium.org
BUG=chromium:656800

Change-Id: I1121a42ca98d8a7432e247d4b44a9ad1214d4b39
Reviewed-on: https://chromium-review.googlesource.com/418010
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-12-08 18:12:04 +00:00
Scott Graham
f94dd14c45 win: fix SECURITY_DESCRIPTOR builder on vs2013
After https://chromium.googlesource.com/crashpad/crashpad/+/5b83e587.

R=mark@chromium.org
BUG=chromium:655788,chromium:656800

Change-Id: Ic33b9daedc340bfce3cc4ddde4eb4c93f68e7ad0
Reviewed-on: https://chromium-review.googlesource.com/417412
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-12-07 21:51:57 +00:00
Scott Graham
5b83e58771 win: Remove use of rpcrt4 and advapi32 from some util code
ConvertStringSecurityDescriptorToSecurityDescriptor() is used when
creating the initial connection pipe. Because this is done from inside
DllMain(), we cannot use advapi32 (where this function is). Instead,
save the binary representation of the self-relative SECURITY_DESCRIPTOR.
It is conceivable that this could change, but unlikely as this is the
same blob that would be stored on a file in NTFS.

Another potential approach would be to not make the pipe available to
all integrity levels here, and instead modify the Chromium sandbox code
to allow a specific pipe name prefix that would have to correspond with
the pipe name that Crashpad creates.

Similarly, UuidCreate() (used when initializing the database) is in a
DLL that can't be loaded early, so use the Linux/Android implementation
on Windows too.

R=mark@chromium.org
BUG=chromium:655788,chromium:656800

Change-Id: I434f8e96fc275fc30d0a31208b025bfc08595ff9
Reviewed-on: https://chromium-review.googlesource.com/417223
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-12-07 20:03:45 +00:00
Scott Graham
76ef9b5c2b win: Address failure-to-start-handler case for async startup
Second follow up to https://chromium-review.googlesource.com/c/400015/

The ideal would be that if we fail to start the handler, then we don't
end up passing through our unhandled exception filter at all.

In the case of the non-initial client (i.e. renderers) we can do this by
not setting our UnhandledExceptionFilter until after we know we've
connected successfully (because those connections are synchronous from
its point of view). We also change WaitForNamedPipe in the connection
message to block forever, so as long as the precreated pipe exists,
they'll wait to connect. After the initial client has passed the server
side of that pipe to the handler, the handler has the only handle to it.
So, if the handler has disappeared for whatever reason, pipe-connecting
clients will fail with FILE_NOT_FOUND, and will not stick around in the
connection loop. This means non-initial clients do not need additional
logic to avoid getting stuck in our UnhandledExceptionFilter.

For the initial client, it would be ideal to avoid passing through our
UEF too, but none of the 3 options are great:
1. Block until we find out if we started, and then install the filter.
   We don't want to do that, because we don't want to wait.
2. Restore the old filter if it turns out we failed to start. We can't
   do that because Chrome disables ::SetUnhandledExceptionFilter()
   immediately after StartHandler/SetHandlerIPCPipe returns.
3. Don't install our filter until we've successfully started. We don't
   want to do that because we'd miss early crashes, negating the benefit
   of deferred startup.

So, we do need to pass through our UnhandledExceptionFilter. I don't
want more Win32 API calls during the vulnerable filter function. So, at
any point during async startup where there's a failure, set a global
atomic that allows the filter function to abort without trying to signal
a handler that's known to not exist.

One further improvement we might want to look at is unexpected
termination of the handler (as opposed to a failure to start) which
would still result in a useless Sleep(60s). This isn't new behaviour,
but now we have a clear thing to do if we detect the handler is gone.

(Also a missing DWORD/size_t cast for the _x64 bots.)

R=mark@chromium.org
BUG=chromium:567850,chromium:656800

Change-Id: I5be831ca39bd8b2e5c962b9647c8bd469e2be878
Reviewed-on: https://chromium-review.googlesource.com/400985
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-11-02 21:39:52 +00:00
Scott Graham
2d87606bb5 win: Start crashpad_handler by inheriting connection data to it
Previously, StartHandler() launched the handler process, then connected
over a pipe to register for crash handling. Instead, the initial client
can create and inherit handles to the handler and pass those handle
values and other data (addresses, etc.) on the command line.

This should improve startup time as there's no need to synchronize with
the process at startup, and allows avoiding a call to CreateProcess()
directly in StartHandler(), which is important for registration for
crash reporting from DllMain().

Incidentally adds new utility functions for string/number conversion and
string splitting.

Note: API change; UseHandler() is removed for all platforms.

BUG=chromium:567850,chromium:656800

Change-Id: I1602724183cb107f805f109674c53e95841b24fd
Reviewed-on: https://chromium-review.googlesource.com/400015
Reviewed-by: Mark Mentovai <mark@chromium.org>
2016-10-21 20:35:58 +00:00
Mark Mentovai
7413569ea6 win: Explain the CreateFile() client-side pipe-opening loop
The bug and linked code review has more of the history, but we’ve been
tempted to remove the loop outright a couple of times already before
realizing that it serves an important purpose. Hopefully this comment
will protect our future selves from going on the same fool’s errand.

BUG=crashpad:75
R=scottmg@chromium.org

Review URL: https://codereview.chromium.org/1427643010 .
2015-11-10 16:43:13 -05:00
Scott Graham
ff274507dc win: Only retry in UseHandler() loop on ERROR_PIPE_BUSY
This is better because now end_to_end_test.py fails immediately with

[1180:9020:20151106,145204.830:ERROR registration_protocol_win.cc:39] CreateFile: The system cannot find the file specified.  (0x2)

R=mark@chromium.org
BUG=crashpad:75

Review URL: https://codereview.chromium.org/1409693011 .
2015-11-06 15:54:48 -08:00
Scott Graham
81269ee676 win: Fix pipe leak on connection
The pipe handle was being leaked on connections (oops!). On XP this
resulted in the next test's CreateNamedPipe to fail, because the
previous one still existed (because all handles were not closed). More
recent OSs are more forgiving so I got away with the buggy code.

R=mark@chromium.org
BUG=crashpad:50

Review URL: https://codereview.chromium.org/1337953003 .
2015-09-11 15:34:35 -07:00
Scott Graham
6978bf7646 win: Crash handler server
This replaces the registration server, and adds dispatch to a delegate
on crash requests.

(As you are already aware) we went around in circles on trying to come
up with a slightly-too-fancy threading design. All of them seemed to
have problems when it comes to out of order events, and orderly
shutdown, so I've gone back to something not-too-fancy.

Two named pipe instances (that clients connect to) are created. These
are used only for registration (which should take <1ms), so 2 should be
sufficient to avoid any waits. When a client registers, we duplicate
an event to it, which is used to signal when it wants a dump taken.

The server registers threadpool waits on that event, and also on the
process handle (which will be signalled when the client process exits).
These requests (in particular the taking of the dump) are serviced
on the threadpool, which avoids us needing to manage those threads,
but still allows parallelism in taking dumps. On process termination,
we use an IO Completion Port to post a message back to the main thread
to request cleanup. This complexity is necessary so that we can
unregister the threadpool waits without being on the threadpool, which
we need to do synchronously so that we can be sure that no further
callbacks will execute (and expect to have the client data around
still).

In a followup, I will readd support for DumpWithoutCrashing -- I don't
think it will be too difficult now that we have an orderly way to
clean up client records in the server.

R=cpu@chromium.org, mark@chromium.org, jschuh@chromium.org
BUG=crashpad:1,crashpad:45

Review URL: https://codereview.chromium.org/1301853002 .
2015-09-03 11:06:17 -07:00