Bug 62304 – PingLoader dtor has NULL dereference

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[David Tapuska]

PingLoader::~PingLoader has a potential null dereference if the ResourceHandle failed during creation.

[David Tapuska]

Created attachment 96455 [details]
Check for valid ref before calling cancel

[David Tapuska]

Created attachment 96459 [details]
Check for valid ref before calling cancel

Sorry PrepareChangeLog munged the ChangeLog with the git commit incorrectly.

[Alexey Proskuryakov]

Can a regression test e made for this?

[David Tapuska]

(In reply to comment #3)
> Can a regression test e made for this?

I don't have a reproduction step. The fix for this crash was mined from coredumps on our platform. For a regression test we'd need to somehow fudge that the ResourceHandle failed to create a resource; and the ResourceHandle::create is platform specific.

[Alexey Proskuryakov]

In that case, how did you decide that this is a bug in PingLoader destructor, and not in ResourceHandle::start() on your platform?

[David Tapuska]

(In reply to comment #5)
> In that case, how did you decide that this is a bug in PingLoader destructor, and not in ResourceHandle::start() on your platform?

Checkout the ResourceLoader.cpp and MainResourceLoader.cpp and you'll find that anyone that uses ResourceHandle::create will check that the value returned is a valid ref before accessing it.

ResourceHandle::create has a NULL return path depending on the return code from newHandle->start(...) the start implementation is the platform specific code I referred to.

[Alexey Proskuryakov]

ResourceHandle::start() is expected to return false when invoked on a detached page. This shouldn't happen with PingLoader - it's created in very specific and controlled circumstances. So, it's different from any other caller of ResourceHandle::create().

Does PingLoader actually work on your platform?

[David Tapuska]

(In reply to comment #7)
> ResourceHandle::start() is expected to return false when invoked on a detached page. This shouldn't happen with PingLoader - it's created in very specific and controlled circumstances. So, it's different from any other caller of ResourceHandle::create().
> 
> Does PingLoader actually work on your platform?

ResoruceHandle::start() can return false for other reasons as well. Not just on a detached page.

For example in the ResourceHandleWin.cpp if InternetConnectW fails; start will return false and then the ResourceHandle::create will return 0.

[Alexey Proskuryakov]

ok

[Daniel Bates]

Comment on attachment 96459 [details]
Check for valid ref before calling cancel

r=me

[WebKit Review Bot]

Comment on attachment 96459 [details]
Check for valid ref before calling cancel

Clearing flags on attachment: 96459

Committed r88458: <http://trac.webkit.org/changeset/88458>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

<rdar://problem/9581084>