Bug 222954 - Regression(r273875): Potential over-release in WKRemoteObjectCoder's decodeObjCObject()
Summary: Regression(r273875): Potential over-release in WKRemoteObjectCoder's decodeOb...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL: https://developer.apple.com/documenta...
Keywords: InRadar
Depends on:
Blocks: 222401
  Show dependency tree / graph
 
Reported: 2021-03-08 16:31 PST by Chris Dumez
Modified: 2021-03-08 19:08 PST (History)
7 users (show)

See Also:

Attachments
Patch (2.72 KB, patch)
2021-03-08 16:36 PST, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (2.30 KB, patch)
2021-03-08 17:44 PST, Chris Dumez 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2021-03-08 16:31:23 PST 
Potential over-release in WKRemoteObjectCoder's decodeObjCObject():
  1 libobjc.A.dylib                0x000084af objc_release + 31 (/System/Volumes/Data/SWE/macOS/BuildRoots/2288acc43c/Library/Caches/com.apple.xbs/Sources/objc4/objc4-824/runtime/objc-runtime-new.h:1589)
>  2 com.apple.WebKit               0x001aee38 decodeObjCObject(WKRemoteObjectDecoder*, objc_class*) + 0
   3 com.apple.WebKit               0x001ae243 decodeObject(WKRemoteObjectDecoder*) + 0
   4 com.apple.WebKit               0x001ad24a decodeObject(WKRemoteObjectDecoder*, API::Dictionary const*, WTF::HashSet<void const*, WTF::DefaultHash<void const*>, WTF::HashTraits<void const*> > const&) + 0
   5 com.apple.WebKit               0x001ad12e -[WKRemoteObjectDecoder decodeObjectOfClasses:forKey:] + 0
   6 com.apple.AppKit               0x000a1120 -[NSColor initWithCoder:] + 197 (/System/Volumes/Data/SWE/macOS/BuildRoots/2288acc43c/Library/Caches/com.apple.xbs/Sources/AppKit/AppKit-2022.44.149/AppKit.subproj/NSColor.m:1652)
   7 com.apple.WebKit               0x001aedc5 decodeObjCObject(WKRemoteObjectDecoder*, objc_class*) + 0
   8 com.apple.WebKit               0x001ae243 decodeObject(WKRemoteObjectDecoder*) + 0
   9 com.apple.WebKit               0x001ad24a decodeObject(WKRemoteObjectDecoder*, API::Dictionary const*, WTF::HashSet<void const*, WTF::DefaultHash<void const*>, WTF::HashTraits<void const*> > const&) + 0
  10 com.apple.WebKit               0x001ad12e -[WKRemoteObjectDecoder decodeObjectOfClasses:forKey:] + 0
  11 com.apple.AppKit               0x002e886a -[NSImage initWithCoder:] + 1768 (/System/Volumes/Data/SWE/macOS/BuildRoots/2288acc43c/Library/Caches/com.apple.xbs/Sources/AppKit/AppKit-2022.44.149/AppKit.subproj/NSImage.m:2013)
  12 com.apple.WebKit               0x001aedc5 decodeObjCObject(WKRemoteObjectDecoder*, objc_class*) + 0
  13 com.apple.WebKit               0x001ae243 decodeObject(WKRemoteObjectDecoder*) + 0
  14 com.apple.WebKit               0x001ad24a decodeObject(WKRemoteObjectDecoder*, API::Dictionary const*, WTF::HashSet<void const*, WTF::DefaultHash<void const*>, WTF::HashTraits<void const*> > const&) + 0
  15 com.apple.WebKit               0x001af1a3 decodeInvocationArguments(WKRemoteObjectDecoder*, NSInvocation*, WTF::Vector<WTF::HashSet<void const*, WTF::DefaultHash<void const*>, WTF::HashTraits<void const*> >, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, unsigned long) + 0
  16 com.apple.WebKit               0x001ae828 decodeObject(WKRemoteObjectDecoder*) + 0
  17 com.apple.WebKit               0x001ad24a decodeObject(WKRemoteObjectDecoder*, API::Dictionary const*, WTF::HashSet<void const*, WTF::DefaultHash<void const*>, WTF::HashTraits<void const*> > const&) + 0
  18 com.apple.WebKit               0x001ad12e -[WKRemoteObjectDecoder decodeObjectOfClasses:forKey:] + 0
  19 com.apple.WebKit               0x001b24b9 -[_WKRemoteObjectRegistry _invokeMethod:] + 0
Comment 1 Chris Dumez 2021-03-08 16:31:39 PST 
<rdar://75163359>
Comment 2 Chris Dumez 2021-03-08 16:36:04 PST 
Created attachment 422636 [details]
Patch
Comment 3 Darin Adler 2021-03-08 17:20:42 PST 
Comment on attachment 422636 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422636&action=review

> Source/WebKit/Shared/API/Cocoa/WKRemoteObjectCoder.mm:314
>      if (!result)

Isn’t the correct fix:

    result = adoptNS([result.leakRef() awakeAfterUsingCoder:decoder])

and no other changes?
Comment 4 Chris Dumez 2021-03-08 17:42:04 PST 
Comment on attachment 422636 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422636&action=review

>> Source/WebKit/Shared/API/Cocoa/WKRemoteObjectCoder.mm:314
>>      if (!result)
> 
> Isn’t the correct fix:
> 
>     result = adoptNS([result.leakRef() awakeAfterUsingCoder:decoder])
> 
> and no other changes?

Yes, it seems like this should work and it is a bit nicer. I'll switch.
Comment 5 Chris Dumez 2021-03-08 17:44:13 PST 
Created attachment 422645 [details]
Patch
Comment 6 EWS 2021-03-08 19:07:57 PST 
Committed r274129: <https://commits.webkit.org/r274129>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422645 [details].