[ASan] XPCConnectionTerminationWatchdog may kill an unrelated process after its pid is reused by ddkilzer · Pull Request #77405 · WebKit/WebKit

ddkilzer · 2026-10-17T20:20:19Z

`a497ee1`

[ASan] XPCConnectionTerminationWatchdog may kill an unrelated process after its pid is reused
<https://bugs.webkit.org/show_bug.cgi?id=327960>
<rdar://190319872>

Reviewed by NOBODY (OOPS!).

In ASan builds, `terminateWithReason()` kills the process with
`terminate_with_reason()`, which takes a pid.  The termination watchdog
calls it 30 seconds after the connection was cancelled, so if the
child process already exited and its pid was reused, the watchdog
kills an unrelated process.  `xpc_connection_kill()`, used in other
builds, does not have this problem.

Call `terminate_with_reason()` only if `proc_pidpath_audittoken()`
succeeds for the connection's audit token, which fails once the
process that the token identifies has exited, and fall back to
`xpc_connection_kill()` otherwise.  The iOS Simulator SDKs lack
`<libproc.h>`, so those builds skip the check and keep calling
`terminate_with_reason()` to preserve their crash reports.

No new tests since this change is not directly testable.

* Source/WebKit/Platform/cocoa/XPCUtilities.mm:
(WebKit::terminateWithReason):

a497ee1

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win	⏳ 🛠 ios-apple
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests	⏳ 🛠 mac-apple
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe		⏳ 🛠 vision-apple
	✅ 🧪 ios-wk2-wpt	⏳ 🧪 api-mac-debug
	🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk3-gcc
	✅ 🛠 ios-safer-cpp	🧪 mac-AS-debug-wk2	✅ 🛠 gtk
	✅ 🛠 vision		✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	⏳ 🧪 mac-intel-wk2	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🛠 mac-safer-cpp	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🧪 mac-site-isolation
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2026-10-17T20:20:25Z

EWS run on previous version of this PR (hash c1aac06)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 win	⏳ 🛠 ios-apple
	❌ 🛠 ios-sim	✅ 🛠 mac-AS-debug	⏳ 🧪 wpe-wk2	⏳ 🧪 win-tests	⏳ 🛠 mac-apple
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	⏳ 🧪 api-mac	⏳ 🧪 api-wpe		⏳ 🛠 vision-apple
	❌ 🧪 ios-wk2-wpt	⏳ 🧪 api-mac-debug	⏳ 🧪 jsc-wpe
⏳ 🧪 jsc-x86-64	❌ 🧪 api-ios	⏳ 🧪 mac-wk2	✅ 🛠 gtk3-gcc
⏳ 🛠 🧪 jsc-debug-arm64	~~🛠 ios-safer-cpp~~	⏳ 🧪 mac-AS-debug-wk2	✅ 🛠 gtk
	⏳ 🛠 vision	⏳ 🧪 mac-wk2-stress	~~🧪 gtk-wk2~~
	⏳ 🛠 vision-sim	⏳ 🧪 mac-intel-wk2	✅ 🧪 api-gtk
	⏳ 🧪 vision-wk2	✅ 🛠 mac-safer-cpp	⏳ 🛠 playstation
	⏳ 🛠 tv	⏳ 🧪 mac-site-isolation
	⏳ 🛠 tv-sim
	⏳ 🛠 watch
	~~🛠 watch-sim~~

webkit-early-warning-system · 2026-10-17T20:44:24Z

EWS run on previous version of this PR (hash a743947)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win	⏳ 🛠 ios-apple
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests	⏳ 🛠 mac-apple
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe		⏳ 🛠 vision-apple
	✅ 🧪 ios-wk2-wpt	⏳ 🧪 api-mac-debug
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk3-gcc
	✅ 🛠 ios-safer-cpp	✅ 🧪 mac-AS-debug-wk2	✅ 🛠 gtk
	✅ 🛠 vision		✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	⏳ 🧪 mac-intel-wk2	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🛠 mac-safer-cpp	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🧪 mac-site-isolation
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

… after its pid is reused <https://bugs.webkit.org/show_bug.cgi?id=327960> <rdar://190319872> Reviewed by NOBODY (OOPS!). In ASan builds, `terminateWithReason()` kills the process with `terminate_with_reason()`, which takes a pid. The termination watchdog calls it 30 seconds after the connection was cancelled, so if the child process already exited and its pid was reused, the watchdog kills an unrelated process. `xpc_connection_kill()`, used in other builds, does not have this problem. Call `terminate_with_reason()` only if `proc_pidpath_audittoken()` succeeds for the connection's audit token, which fails once the process that the token identifies has exited, and fall back to `xpc_connection_kill()` otherwise. The iOS Simulator SDKs lack `<libproc.h>`, so those builds skip the check and keep calling `terminate_with_reason()` to preserve their crash reports. No new tests since this change is not directly testable. * Source/WebKit/Platform/cocoa/XPCUtilities.mm: (WebKit::terminateWithReason):

webkit-early-warning-system · 2026-10-18T03:03:00Z

EWS run on current version of this PR (hash a497ee1)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows	Apple Internal
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win	⏳ 🛠 ios-apple
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests	⏳ 🛠 mac-apple
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe		⏳ 🛠 vision-apple
	✅ 🧪 ios-wk2-wpt	⏳ 🧪 api-mac-debug
	🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk3-gcc
	✅ 🛠 ios-safer-cpp	🧪 mac-AS-debug-wk2	✅ 🛠 gtk
	✅ 🛠 vision		✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	⏳ 🧪 mac-intel-wk2	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🛠 mac-safer-cpp	✅ 🛠 playstation
	✅ 🛠 tv	✅ 🧪 mac-site-isolation
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

zakariaridouh · 2026-10-18T03:18:30Z

        if (invalidMessageName)
            code |= static_cast<uint64_t>(std::to_underlying(*invalidMessageName)) << 8;
-        if (!terminate_with_reason(xpc_connection_get_pid(connection), OS_REASON_WEBKIT, code, reason, 0))
+        bool isSameProcess = true;


Nit: maybe we should move the mutable isSameProcess and its inner #if into a small helper. Non blocking.

zakariaridouh · 2026-10-18T03:19:14Z

-        if (!terminate_with_reason(xpc_connection_get_pid(connection), OS_REASON_WEBKIT, code, reason, 0))
+        bool isSameProcess = true;
+#if HAVE(LIBPROC)
+        // terminate_with_reason() signals by pid, and the watchdog fires 30s after the connection


I don't think the '30s after the connection was cancelled' part of the comment is correct. The timer is armed from terminate(), shutDownProcess() or the destructor, so I think its more accurate to say 'after the watchdog was armed'.

zakariaridouh · 2026-10-18T03:19:49Z

            code |= static_cast<uint64_t>(std::to_underlying(*invalidMessageName)) << 8;
-        if (!terminate_with_reason(xpc_connection_get_pid(connection), OS_REASON_WEBKIT, code, reason, 0))
+        bool isSameProcess = true;
+#if HAVE(LIBPROC)


Where HAVE(LIBPROC) is false the hazard remains. That includes iOS Simulator, where processes are host processes. We can add a FIXME and mention it in the commit message. Non blocking.

zakariaridouh · 2026-10-18T03:20:28Z

+        std::array<char, PROC_PIDPATHINFO_MAXSIZE> path;
+        isSameProcess = proc_pidpath_audittoken(&auditToken, path.data(), path.size()) > 0;
+#endif
+        if (isSameProcess && !terminate_with_reason(xpc_connection_get_pid(connection), OS_REASON_WEBKIT, code, reason, 0))


Use audit_token_to_pid(auditToken) for the terminate_with_reason pid instead of a second xpc_connection_get_pid. As written, the check can pass while xpc_connection_get_pid returns 0 on a cancelled connection.

zakariaridouh

r+ with some nits and comments

ddkilzer requested a review from cdumez as a code owner October 17, 2026 20:20

ddkilzer self-assigned this Oct 17, 2026

ddkilzer added the WebKit Process Model Bugs related to WebKit's multi-process architecture label Oct 17, 2026

ddkilzer requested a review from zakariaridouh October 17, 2026 20:21

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 17, 2026

ddkilzer marked this pull request as draft October 17, 2026 20:37

ddkilzer removed the merging-blocked Applied to prevent a change from being merged label Oct 17, 2026

ddkilzer force-pushed the eng/ASan-XPCConnectionTerminationWatchdog-may-kill-an branch from c1aac06 to a743947 Compare October 17, 2026 20:44

ddkilzer marked this pull request as ready for review October 17, 2026 20:53

ddkilzer force-pushed the eng/ASan-XPCConnectionTerminationWatchdog-may-kill-an branch from a743947 to a497ee1 Compare October 18, 2026 03:02

zakariaridouh reviewed Oct 18, 2026

View reviewed changes

zakariaridouh approved these changes Oct 18, 2026

View reviewed changes

Navigation Menu

Repository navigation

[ASan] XPCConnectionTerminationWatchdog may kill an unrelated process after its pid is reused - #77405

[ASan] XPCConnectionTerminationWatchdog may kill an unrelated process after its pid is reused#77405
ddkilzer wants to merge 1 commit into
WebKit:mainfrom
ddkilzer:eng/ASan-XPCConnectionTerminationWatchdog-may-kill-an

ddkilzer commented Oct 17, 2026 •

edited by webkit-early-warning-system

Loading

Uh oh!

webkit-early-warning-system commented Oct 17, 2026 •

edited

Loading

Uh oh!

webkit-early-warning-system commented Oct 17, 2026 •

edited

Loading

Uh oh!

webkit-early-warning-system commented Oct 18, 2026 •

edited

Loading

Uh oh!

zakariaridouh Oct 18, 2026

Uh oh!

zakariaridouh Oct 18, 2026

Uh oh!

zakariaridouh Oct 18, 2026

Uh oh!

zakariaridouh Oct 18, 2026

Uh oh!

zakariaridouh left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ddkilzer commented Oct 17, 2026 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

a497ee1

Uh oh!

webkit-early-warning-system commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zakariaridouh Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

zakariaridouh Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

zakariaridouh Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

zakariaridouh Oct 18, 2026

Choose a reason for hiding this comment

Uh oh!

zakariaridouh left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ddkilzer commented Oct 17, 2026 •

edited by webkit-early-warning-system

Loading

`a497ee1`

webkit-early-warning-system commented Oct 17, 2026 •

edited

Loading

webkit-early-warning-system commented Oct 17, 2026 •

edited

Loading

webkit-early-warning-system commented Oct 18, 2026 •

edited

Loading