Bug 222954 – Regression(r273875): Potential over-release in WKRemoteObjectCoder's decodeObjCObject()

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

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

[Chris Dumez]

<rdar://75163359>

[Chris Dumez]

Created attachment 422636 [details]
Patch

[Darin Adler]

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?

[Chris Dumez]

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.

[Chris Dumez]

Created attachment 422645 [details]
Patch

[EWS]

Committed r274129: <https://commits.webkit.org/r274129>

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