Summary: | Crash shortly after loading certain web pages | ||
---|---|---|---|
Product: | WebKit | Reporter: | Aaron Golden <aegolden> |
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | andersca, commit-queue |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Attachments: |
Description
Aaron Golden
2014-03-06 22:13:38 PST
Created attachment 226086 [details]
Patch prevents shimSecItem{CopyMatching,Add,Update,Delete} from trying to use NULL implementations
Patch prevents the crash by guarding various methods against calling implementations from the secItemShimCallbacks struct when those implementations are NULL. I'm actually not sure what the most appropriate OSStatus return value is when the callback is NULL. errSecUnimplemented seemed pretty good, but maybe there's a better/more standard one.
Created attachment 226088 [details]
Same as the other patch, but remembered to include a new ChangeLog entry.
I don't think this is correct. A null shim function indicates a serious problem (like the initialization function never being called). Is it weird that shimSecItemCopyMatching is being called from the SafariForWebKitDevelopment process itself? I see WebKitSecItemShimInitialize called from within the child WebProcess, but not from within the SafariForWebKitDevelopment process. The bug disappeared when I rolled back my local patch. I wonder if when I got past the crash the first time (with the r- patch) some state was saved that prevents the troublesome code path from executing on subsequent launches. I was able to reproduce the bug again by running Tools/Scripts/debug-safari --target-web-process instead of Tools/Scripts/run-safari --debug. The WebProcess is hitting the shim initializer (WebKitSecItemShimInitialize) with non-NULL callbacks, but the SafariForWebkitDevelopment process *itself* is calling shimSecItemCopyMatching, and the shim struct used by the SafariForWebkitDevelopment process is empty (all callbacks NULL). Created attachment 226189 [details]
Prevents processes spawned by WebProcess from trying to insert SecItem shim.
The trouble here is that SafariForWebkitDevelopment (spawned by WebProcess) is *itself* trying to call functions in SecItemShimLibrary.mm, but the secItemShimCallbacks struct is initialized only in the WebProcess. The spawned processes shouldn't be in this code at all.
Comment on attachment 226189 [details] Prevents processes spawned by WebProcess from trying to insert SecItem shim. Clearing flags on attachment: 226189 Committed r165329: <http://trac.webkit.org/changeset/165329> All reviewed patches have been landed. Closing bug. |