62304 2011-06-08 12:10:12 -0700 PingLoader dtor has NULL dereference 2011-06-09 10:51:43 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Other Other RESOLVED FIXED InRadar P2 Normal --- 0 dave+webkit webkit-unassigned ap dbates fishd staikos webkit.review.bot oldest_to_newest 417394 0 dave+webkit 2011-06-08 12:10:12 -0700 PingLoader::~PingLoader has a potential null dereference if the ResourceHandle failed during creation. 417402 1 96455 dave+webkit 2011-06-08 12:15:50 -0700 Created attachment 96455 Check for valid ref before calling cancel 417408 2 96459 dave+webkit 2011-06-08 12:20:43 -0700 Created attachment 96459 Check for valid ref before calling cancel Sorry PrepareChangeLog munged the ChangeLog with the git commit incorrectly. 417630 3 ap 2011-06-08 15:50:20 -0700 Can a regression test e made for this? 417971 4 dave+webkit 2011-06-09 06:24:17 -0700 (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. 418035 5 ap 2011-06-09 08:46:55 -0700 In that case, how did you decide that this is a bug in PingLoader destructor, and not in ResourceHandle::start() on your platform? 418039 6 dave+webkit 2011-06-09 08:52:16 -0700 (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. 418047 7 ap 2011-06-09 09:05:56 -0700 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? 418060 8 dave+webkit 2011-06-09 09:32:21 -0700 (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. 418071 9 ap 2011-06-09 09:47:15 -0700 ok 418081 10 96459 dbates 2011-06-09 10:12:33 -0700 Comment on attachment 96459 Check for valid ref before calling cancel r=me 418103 11 96459 webkit.review.bot 2011-06-09 10:49:46 -0700 Comment on attachment 96459 Check for valid ref before calling cancel Clearing flags on attachment: 96459 Committed r88458: <http://trac.webkit.org/changeset/88458> 418104 12 webkit.review.bot 2011-06-09 10:49:51 -0700 All reviewed patches have been landed. Closing bug. 418106 13 darin 2011-06-09 10:51:43 -0700 <rdar://problem/9581084> 96455 2011-06-08 12:15:50 -0700 2011-06-08 12:20:43 -0700 Check for valid ref before calling cancel PingLoader.patch.txt text/plain 1795 dave+webkit Y29tbWl0IDk1ZThhNWQzNzMxMGQ4ZjgzMDE1ZjM3Y2ZjNzFiOTYxMTlhZmEwYWIKQXV0aG9yOiBE YXZlIFRhcHVza2EgPGR0YXB1c2thQHJpbS5jb20+CkRhdGU6ICAgV2VkIEp1biA4IDE1OjA5OjA0 IDIwMTEgLTA0MDAKCiAgICAyMDExLTA2LTA4ICBEYXZlIFRhcHVza2EgIDxkdGFwdXNrYUByaW0u Y29tPgogICAgCiAgICAgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgogICAgCiAg ICAgICAgICAgIFBpbmdMb2FkZXIgZGVzdHJ1Y3RvciBjb3VsZCBkZXJlZmVyZW5jZSAwIGlmIHRo ZSBSZXNvdXJjZQogICAgICAgICAgICBIYW5kbGUgY3JlYXRpb24gZmFpbGVkLgogICAgCiAgICAg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02MjMwNAogICAg CiAgICAgICAgICAgICogbG9hZGVyL1BpbmdMb2FkZXIuY3BwOgogICAgICAgICAgICAoV2ViQ29y ZTo6UGluZ0xvYWRlcjo6flBpbmdMb2FkZXIpOgoKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3Jl L0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCA2ZDcwMDVjLi5mMTBk YmZlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dl YkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjYgQEAKKzIwMTEtMDYtMDggIERhdmUgVGFwdXNr YSAgPGR0YXB1c2thQHJpbS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgMjAxMS0wNi0wOCAgRGF2ZSBUYXB1c2thICA8ZHRhcHVza2FAcmltLmNv bT4KKworICAgICAgICAgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAg ICAgICAgICAgICAgUGluZ0xvYWRlciBkZXN0cnVjdG9yIGNvdWxkIGRlcmVmZXJlbmNlIDAgaWYg dGhlIFJlc291cmNlCisgICAgICAgICAgICAgICAgSGFuZGxlIGNyZWF0aW9uIGZhaWxlZC4KKwor ICAgICAgICAgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02 MjMwNAorCisgICAgICAgICAgICAgICAgKiBsb2FkZXIvUGluZ0xvYWRlci5jcHA6CisgICAgICAg ICAgICAgICAgKFdlYkNvcmU6OlBpbmdMb2FkZXI6On5QaW5nTG9hZGVyKToKKworICAgICAgICBO ZWVkIGEgc2hvcnQgZGVzY3JpcHRpb24gYW5kIGJ1ZyBVUkwgKE9PUFMhKQorCisgICAgICAgIE5v IG5ldyB0ZXN0cy4gKE9PUFMhKQorCisgICAgICAgICogbG9hZGVyL1BpbmdMb2FkZXIuY3BwOgor ICAgICAgICAoV2ViQ29yZTo6UGluZ0xvYWRlcjo6flBpbmdMb2FkZXIpOgorCiAyMDExLTA2LTA4 ICBNaWtvxYJhaiBNYcWCZWNraSAgPG0ubWFsZWNraUBzYW1zdW5nLmNvbT4KIAogICAgICAgICBS ZXZpZXdlZCBieSBQYXZlbCBGZWxkbWFuLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvbG9h ZGVyL1BpbmdMb2FkZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvbG9hZGVyL1BpbmdMb2FkZXIuY3Bw CmluZGV4IDk3NDUyNzMuLjAyN2FkZTkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2xvYWRl ci9QaW5nTG9hZGVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvUGluZ0xvYWRlci5j cHAKQEAgLTEyNyw3ICsxMjcsOCBAQCBQaW5nTG9hZGVyOjpQaW5nTG9hZGVyKEZyYW1lKiBmcmFt ZSwgUmVzb3VyY2VSZXF1ZXN0JiByZXF1ZXN0KQogCiBQaW5nTG9hZGVyOjp+UGluZ0xvYWRlcigp CiB7Ci0gICAgbV9oYW5kbGUtPmNhbmNlbCgpOworICAgIGlmIChtX2hhbmRsZSkKKyAgICAgICAg bV9oYW5kbGUtPmNhbmNlbCgpOwogfQogCiB9Cg== 96459 2011-06-08 12:20:43 -0700 2011-06-09 10:49:46 -0700 Check for valid ref before calling cancel PingLoader.patch.txt text/plain 1482 dave+webkit Y29tbWl0IDFiNzNiOWM2ZGRmMDZjMWMwMzUwYTI0MzJkNDJjNmQ5ZjE0ZGFhNmEKQXV0aG9yOiBE YXZlIFRhcHVza2EgPGR0YXB1c2thQHJpbS5jb20+CkRhdGU6ICAgV2VkIEp1biA4IDE1OjA5OjA0 IDIwMTEgLTA0MDAKCiAgICAyMDExLTA2LTA4ICBEYXZlIFRhcHVza2EgIDxkdGFwdXNrYUByaW0u Y29tPgogICAgCiAgICAgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgogICAgCiAg ICAgICAgICAgIFBpbmdMb2FkZXIgZGVzdHJ1Y3RvciBjb3VsZCBkZXJlZmVyZW5jZSAwIGlmIHRo ZSBSZXNvdXJjZQogICAgICAgICAgICBIYW5kbGUgY3JlYXRpb24gZmFpbGVkLgogICAgCiAgICAg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02MjMwNAogICAg CiAgICAgICAgICAgICogbG9hZGVyL1BpbmdMb2FkZXIuY3BwOgogICAgICAgICAgICAoV2ViQ29y ZTo6UGluZ0xvYWRlcjo6flBpbmdMb2FkZXIpOgoKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3Jl L0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCA2ZDcwMDVjLi43OGU4 NjViIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dl YkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTUgQEAKKzIwMTEtMDYtMDggIERhdmUgVGFwdXNr YSAgPGR0YXB1c2thQHJpbS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgUGluZ0xvYWRlciBkZXN0cnVjdG9yIGNvdWxkIGRlcmVmZXJlbmNlIDAg aWYgdGhlIFJlc291cmNlCisgICAgICAgIEhhbmRsZSBjcmVhdGlvbiBmYWlsZWQuCisKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTYyMzA0CisKKyAgICAg ICAgKiBsb2FkZXIvUGluZ0xvYWRlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpQaW5nTG9hZGVy Ojp+UGluZ0xvYWRlcik6CisKIDIwMTEtMDYtMDggIE1pa2/FgmFqIE1hxYJlY2tpICA8bS5tYWxl Y2tpQHNhbXN1bmcuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFBhdmVsIEZlbGRtYW4uCmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvUGluZ0xvYWRlci5jcHAgYi9Tb3VyY2Uv V2ViQ29yZS9sb2FkZXIvUGluZ0xvYWRlci5jcHAKaW5kZXggOTc0NTI3My4uMDI3YWRlOSAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvbG9hZGVyL1BpbmdMb2FkZXIuY3BwCisrKyBiL1NvdXJj ZS9XZWJDb3JlL2xvYWRlci9QaW5nTG9hZGVyLmNwcApAQCAtMTI3LDcgKzEyNyw4IEBAIFBpbmdM b2FkZXI6OlBpbmdMb2FkZXIoRnJhbWUqIGZyYW1lLCBSZXNvdXJjZVJlcXVlc3QmIHJlcXVlc3Qp CiAKIFBpbmdMb2FkZXI6On5QaW5nTG9hZGVyKCkKIHsKLSAgICBtX2hhbmRsZS0+Y2FuY2VsKCk7 CisgICAgaWYgKG1faGFuZGxlKQorICAgICAgICBtX2hhbmRsZS0+Y2FuY2VsKCk7CiB9CiAKIH0K