194750 2019-02-16 14:51:39 -0800 Mach exception handler could see uninitialized handler 2022-02-10 16:40:38 -0800 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED INVALID InRadar P2 Normal --- 1 keith_miller keith_miller benjamin cdumez cmarcelo dbates ews-watchlist ggaren mark.lam saam webkit-bug-importer oldest_to_newest 1507044 0 keith_miller 2019-02-16 14:51:39 -0800 Mach exception handler could see uninitialized handler 1507045 1 362221 keith_miller 2019-02-16 14:56:22 -0800 Created attachment 362221 Patch 1507046 2 keith_miller 2019-02-16 14:56:24 -0800 <rdar://problem/47629892> 1507047 3 362221 ggaren 2019-02-16 15:15:31 -0800 Comment on attachment 362221 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=362221&action=review Needs a regression test. Should be easy enough to send yourself a signal in an API test. I'm curious: Why eagerly initialize instead of just checking for null? > Source/WTF/ChangeLog:12 > + type, say bad access, we know how to handler but did not register handler -> handle 1507048 4 keith_miller 2019-02-16 15:51:12 -0800 (In reply to Geoffrey Garen from comment #3) > Comment on attachment 362221 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=362221&action=review > > Needs a regression test. Should be easy enough to send yourself a signal in > an API test. > > I'm curious: Why eagerly initialize instead of just checking for null? How would we test for this other than to crash the API test? This only happens if the process was about to crash anyway. This bug just steals someone else's thunder. LazyNeverDestroyed doesn't have a ! operator. I suppose we could add one though. It also just seemed like we would be less likely to have a bug like this in the future for the cost of 2 extra words. > > > Source/WTF/ChangeLog:12 > > + type, say bad access, we know how to handler but did not register > > handler -> handle Fixed. 1507049 5 362221 saam 2019-02-16 16:26:30 -0800 Comment on attachment 362221 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=362221&action=review > Source/WTF/ChangeLog:16 > + This patch makes it so that we intialize all know handler bags Know => Known > Source/WTF/wtf/threads/Signals.cpp:260 > std::call_once(initializeOnceFlags[static_cast<size_t>(signal)], [&] { Shouldn’t we technically do this before starting the Mach exception handler thread? 1507050 6 362221 saam 2019-02-16 16:27:38 -0800 Comment on attachment 362221 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=362221&action=review >>> Source/WTF/ChangeLog:12 >>> + type, say bad access, we know how to handler but did not register >> >> handler -> handle > > Fixed. “handle” => “handle it, but” 1507051 7 362221 saam 2019-02-16 16:28:55 -0800 Comment on attachment 362221 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=362221&action=review > Source/WTF/ChangeLog:9 > + If we register a mach exception handler for some exception type, I now understand the bug and agree with your fix. I agree with Geoff that we should add a test for this. 1507060 8 ggaren 2019-02-16 20:29:31 -0800 > > Needs a regression test. Should be easy enough to send yourself a signal in > > an API test. > > How would we test for this other than to crash the API test? Can the API test install a signal handler and then send a user signal? Can the API test install a mach exception handler and then do an invalid memory read? Can catch_mach_exception_raise_state support a global override for testing that causes it to return KERN_SUCCESS even if didHandle is false? 1507566 9 keith_miller 2019-02-18 18:16:06 -0800 Sorry, this is totally wrong. I forgot that there is a mask that controls which exceptions are derived to your dispatch queue. We would never see a signal for an exception we did not already initialize. That said, I'm ok making a code quality change here if people want that. 362221 2019-02-16 14:56:22 -0800 2022-02-10 16:40:38 -0800 Patch bug-194750-20190216145620.patch text/plain 4963 keith_miller U3VidmVyc2lvbiBSZXZpc2lvbjogMjQxNjMyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IGFmOTkzMjI3NzU2YTAyOGEyMDA0NDc2 MGFkZTliNDIxYTU1NWU1NWUuLjM5MTViYWQ5ZWU5ZGI0MGRmYWNkOTcyNjAyZjg5NWEzNmE3ZDAw YzAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMjggQEAKKzIwMTktMDItMTYgIEtlaXRoIE1pbGxlciAgPGtlaXRo X21pbGxlckBhcHBsZS5jb20+CisKKyAgICAgICAgTWFjaCBleGNlcHRpb24gaGFuZGxlciBjb3Vs ZCBzZWUgdW5pbml0aWFsaXplZCBoYW5kbGVyCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0xOTQ3NTAKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzQ3NjI5 ODkyPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIElm IHdlIHJlZ2lzdGVyIGEgbWFjaCBleGNlcHRpb24gaGFuZGxlciBmb3Igc29tZSBleGNlcHRpb24g dHlwZSwKKyAgICAgICAgc2F5IGlsbGVnYWwgaW5zdHJ1Y3Rpb24sIHdlIHdpbGwgaW5pdGlhbGl6 ZSB0aGUgQmFnIG9mIGhhbmRsZXJzCisgICAgICAgIG9ubHkgZm9yIHRoYXQgZXhjZXB0aW9uIHR5 cGUuIElmIHdlIHNlZSBhIGRpZmZlcmVudCBleGNlcHRpb24KKyAgICAgICAgdHlwZSwgc2F5IGJh ZCBhY2Nlc3MsIHdlIGtub3cgaG93IHRvIGhhbmRsZXIgYnV0IGRpZCBub3QgcmVnaXN0ZXIKKyAg ICAgICAgYSBoYW5kbGVyIGZvciwgdGhlbiB3ZSB3aWxsIGRlcmVmZXJlbmNlIHRoZSB1bmluaXRp YWxpemVkIEJhZyBmb3IKKyAgICAgICAgYmFkIGFjY2VzcyBhbmQgZ2V0IGEgbnVsbHB0ciBkZXJl ZmVyZW5jZS4KKworICAgICAgICBUaGlzIHBhdGNoIG1ha2VzIGl0IHNvIHRoYXQgd2UgaW50aWFs aXplIGFsbCBrbm93IGhhbmRsZXIgYmFncworICAgICAgICB3aGVuIHJlZ2lzdGVyaW5nIHRoZSBm aXJzdCBleGNlcHRpb24gdHlwZS4KKworICAgICAgICAqIHd0Zi9OZXZlckRlc3Ryb3llZC5oOgor ICAgICAgICAoV1RGOjpMYXp5TmV2ZXJEZXN0cm95ZWQ6Om9wZXJhdG9yW10pOgorICAgICAgICAo V1RGOjpMYXp5TmV2ZXJEZXN0cm95ZWQ6Om9wZXJhdG9yW10gY29uc3QpOgorICAgICAgICAqIHd0 Zi90aHJlYWRzL1NpZ25hbHMuY3BwOgorICAgICAgICAoV1RGOjppbnN0YWxsU2lnbmFsSGFuZGxl cik6CisgICAgICAgIChXVEY6OmpzY1NpZ25hbEhhbmRsZXIpOgorCiAyMDE5LTAyLTE1ICBSb3Nz IEtpcnNsaW5nICA8cm9zcy5raXJzbGluZ0Bzb255LmNvbT4KIAogICAgICAgICBbV1RGXSBBZGQg ZW52aXJvbm1lbnQgdmFyaWFibGUgaGVscGVycwpkaWZmIC0tZ2l0IGEvU291cmNlL1dURi93dGYv TmV2ZXJEZXN0cm95ZWQuaCBiL1NvdXJjZS9XVEYvd3RmL05ldmVyRGVzdHJveWVkLmgKaW5kZXgg YzMxMWYxMzZmZjVlNTg0MGVlNWY0OWVlNWY3MTg2ODYxODdkNjRjNy4uYjNmNDc1MjJlZTU2ODg0 NTUxZmIxZWMwMTkxZGI5MzA3OWFhYjQ1ZiAxMDA2NDQKLS0tIGEvU291cmNlL1dURi93dGYvTmV2 ZXJEZXN0cm95ZWQuaAorKysgYi9Tb3VyY2UvV1RGL3d0Zi9OZXZlckRlc3Ryb3llZC5oCkBAIC0x MTUsNiArMTE1LDEyIEBAIHB1YmxpYzoKICAgICBib29sIGlzQ29uc3RydWN0ZWQoKSBjb25zdCB7 IHJldHVybiBtX2lzQ29uc3RydWN0ZWQ7IH0KICNlbmRpZgogCisgICAgdGVtcGxhdGU8dHlwZW5h bWUgSW5kZXg+CisgICAgYXV0byYgb3BlcmF0b3JbXShJbmRleCB0KSB7IHJldHVybiBnZXQoKVt0 XTsgfQorCisgICAgdGVtcGxhdGU8dHlwZW5hbWUgSW5kZXg+CisgICAgY29uc3QgYXV0byYgb3Bl cmF0b3JbXShJbmRleCB0KSBjb25zdCB7IHJldHVybiBnZXQoKVt0XTsgfQorCiBwcml2YXRlOgog ICAgIHVzaW5nIFBvaW50ZXJUeXBlID0gdHlwZW5hbWUgc3RkOjpyZW1vdmVfY29uc3Q8VD46OnR5 cGUqOwogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL3d0Zi90aHJlYWRzL1NpZ25hbHMuY3BwIGIv U291cmNlL1dURi93dGYvdGhyZWFkcy9TaWduYWxzLmNwcAppbmRleCBmY2QwZjFkNjhmYWVjZTgz NDBjNzliNGU2YzVkYjA3MWIwYTExMjM0Li5jZTQwNmIwMGM1MmZhNmQyNzk5NGZjOGZlMjJiMDY1 NWRlMWM1YWQ5IDEwMDY0NAotLS0gYS9Tb3VyY2UvV1RGL3d0Zi90aHJlYWRzL1NpZ25hbHMuY3Bw CisrKyBiL1NvdXJjZS9XVEYvd3RmL3RocmVhZHMvU2lnbmFscy5jcHAKQEAgLTM0LDYgKzM0LDcg QEAgZXh0ZXJuICJDIiB7CiB9OwogI2VuZGlmCiAKKyNpbmNsdWRlIDxhcnJheT4KICNpbmNsdWRl IDxjc3RkaW8+CiAjaW5jbHVkZSA8bXV0ZXg+CiAjaW5jbHVkZSA8c2lnbmFsLmg+CkBAIC01Niw4 ICs1Nyw3IEBAIGV4dGVybiAiQyIgewogbmFtZXNwYWNlIFdURiB7CiAKICAgICAKLXN0YXRpYyBM YXp5TmV2ZXJEZXN0cm95ZWQ8TG9ja2xlc3NCYWc8U2lnbmFsSGFuZGxlcj4+IGhhbmRsZXJzW3N0 YXRpY19jYXN0PHNpemVfdD4oU2lnbmFsOjpOdW1iZXJPZlNpZ25hbHMpXSA9IHsgfTsKLXN0YXRp YyBzdGQ6Om9uY2VfZmxhZyBpbml0aWFsaXplT25jZUZsYWdzW3N0YXRpY19jYXN0PHNpemVfdD4o U2lnbmFsOjpOdW1iZXJPZlNpZ25hbHMpXTsKK3N0YXRpYyBMYXp5TmV2ZXJEZXN0cm95ZWQ8c3Rk OjphcnJheTxMb2NrbGVzc0JhZzxTaWduYWxIYW5kbGVyPiwgc3RhdGljX2Nhc3Q8c2l6ZV90PihT aWduYWw6Ok51bWJlck9mU2lnbmFscyk+PiBoYW5kbGVycyA9IHsgfTsKIHN0YXRpYyBzdHJ1Y3Qg c2lnYWN0aW9uIG9sZEFjdGlvbnNbc3RhdGljX2Nhc3Q8c2l6ZV90PihTaWduYWw6Ok51bWJlck9m U2lnbmFscyldOwogCiAjaWYgSEFWRShNQUNIX0VYQ0VQVElPTlMpCkBAIC0xNzgsNyArMTc4LDcg QEAga2Vybl9yZXR1cm5fdCBjYXRjaF9tYWNoX2V4Y2VwdGlvbl9yYWlzZV9zdGF0ZSgKICAgICB9 CiAKICAgICBib29sIGRpZEhhbmRsZSA9IGZhbHNlOwotICAgIGhhbmRsZXJzW3N0YXRpY19jYXN0 PHNpemVfdD4oc2lnbmFsKV0tPml0ZXJhdGUoWyZdIChjb25zdCBTaWduYWxIYW5kbGVyJiBoYW5k bGVyKSB7CisgICAgaGFuZGxlcnNbc3RhdGljX2Nhc3Q8c2l6ZV90PihzaWduYWwpXS5pdGVyYXRl KFsmXSAoY29uc3QgU2lnbmFsSGFuZGxlciYgaGFuZGxlcikgewogICAgICAgICBTaWduYWxBY3Rp b24gaGFuZGxlclJlc3VsdCA9IGhhbmRsZXIoc2lnbmFsLCBpbmZvLCByZWdpc3RlcnMpOwogICAg ICAgICBkaWRIYW5kbGUgfD0gaGFuZGxlclJlc3VsdCA9PSBTaWduYWxBY3Rpb246OkhhbmRsZWQ7 CiAgICAgfSk7CkBAIC0yNDMsNiArMjQzLDEyIEBAIHN0YXRpYyB2b2lkIGpzY1NpZ25hbEhhbmRs ZXIoaW50LCBzaWdpbmZvX3QqLCB2b2lkKik7CiB2b2lkIGluc3RhbGxTaWduYWxIYW5kbGVyKFNp Z25hbCBzaWduYWwsIFNpZ25hbEhhbmRsZXImJiBoYW5kbGVyKQogewogICAgIEFTU0VSVChzaWdu YWwgPCBTaWduYWw6OlVua25vd24pOworCisgICAgc3RhdGljIHN0ZDo6b25jZV9mbGFnIGhhbmRs ZXJzRmxhZzsKKyAgICBzdGQ6OmNhbGxfb25jZShoYW5kbGVyc0ZsYWcsIFtdIHsKKyAgICAgICAg aGFuZGxlcnMuY29uc3RydWN0KCk7CisgICAgfSk7CisKICNpZiBIQVZFKE1BQ0hfRVhDRVBUSU9O UykKICAgICBBU1NFUlQoIXVzZU1hY2ggfHwgc2lnbmFsICE9IFNpZ25hbDo6VXNyKTsKIApAQCAt MjUwLDkgKzI1Niw4IEBAIHZvaWQgaW5zdGFsbFNpZ25hbEhhbmRsZXIoU2lnbmFsIHNpZ25hbCwg U2lnbmFsSGFuZGxlciYmIGhhbmRsZXIpCiAgICAgICAgIHN0YXJ0TWFjaEV4Y2VwdGlvbkhhbmRs ZXJUaHJlYWQoKTsKICNlbmRpZgogCisgICAgc3RhdGljIHN0ZDo6b25jZV9mbGFnIGluaXRpYWxp emVPbmNlRmxhZ3Nbc3RhdGljX2Nhc3Q8c2l6ZV90PihTaWduYWw6Ok51bWJlck9mU2lnbmFscyld OwogICAgIHN0ZDo6Y2FsbF9vbmNlKGluaXRpYWxpemVPbmNlRmxhZ3Nbc3RhdGljX2Nhc3Q8c2l6 ZV90PihzaWduYWwpXSwgWyZdIHsKLSAgICAgICAgaGFuZGxlcnNbc3RhdGljX2Nhc3Q8c2l6ZV90 PihzaWduYWwpXS5jb25zdHJ1Y3QoKTsKLQogICAgICAgICBpZiAoIXVzZU1hY2gpIHsKICAgICAg ICAgICAgIHN0cnVjdCBzaWdhY3Rpb24gYWN0aW9uOwogICAgICAgICAgICAgYWN0aW9uLnNhX3Np Z2FjdGlvbiA9IGpzY1NpZ25hbEhhbmRsZXI7CkBAIC0yNzEsNyArMjc2LDcgQEAgdm9pZCBpbnN0 YWxsU2lnbmFsSGFuZGxlcihTaWduYWwgc2lnbmFsLCBTaWduYWxIYW5kbGVyJiYgaGFuZGxlcikK IAogICAgIH0pOwogCi0gICAgaGFuZGxlcnNbc3RhdGljX2Nhc3Q8c2l6ZV90PihzaWduYWwpXS0+ YWRkKFdURk1vdmUoaGFuZGxlcikpOworICAgIGhhbmRsZXJzW3N0YXRpY19jYXN0PHNpemVfdD4o c2lnbmFsKV0uYWRkKFdURk1vdmUoaGFuZGxlcikpOwogCiAjaWYgSEFWRShNQUNIX0VYQ0VQVElP TlMpCiAgICAgYXV0byBsb2NrZXIgPSBob2xkTG9jayhhY3RpdmVUaHJlYWRzKCkuZ2V0TG9jaygp KTsKQEAgLTMxMiw3ICszMTcsNyBAQCB2b2lkIGpzY1NpZ25hbEhhbmRsZXIoaW50IHNpZywgc2ln aW5mb190KiBpbmZvLCB2b2lkKiB1Y29udGV4dCkKIAogICAgIGJvb2wgZGlkSGFuZGxlID0gZmFs c2U7CiAgICAgYm9vbCByZXN0b3JlRGVmYXVsdEhhbmRsZXIgPSBmYWxzZTsKLSAgICBoYW5kbGVy c1tzdGF0aWNfY2FzdDxzaXplX3Q+KHNpZ25hbCldLT5pdGVyYXRlKFsmXSAoY29uc3QgU2lnbmFs SGFuZGxlciYgaGFuZGxlcikgeworICAgIGhhbmRsZXJzW3N0YXRpY19jYXN0PHNpemVfdD4oc2ln bmFsKV0uaXRlcmF0ZShbJl0gKGNvbnN0IFNpZ25hbEhhbmRsZXImIGhhbmRsZXIpIHsKICAgICAg ICAgc3dpdGNoIChoYW5kbGVyKHNpZ25hbCwgc2lnSW5mbywgcmVnaXN0ZXJzKSkgewogICAgICAg ICBjYXNlIFNpZ25hbEFjdGlvbjo6SGFuZGxlZDoKICAgICAgICAgICAgIGRpZEhhbmRsZSA9IHRy dWU7Cg==