Browse Source

#51

meta/gitblit/tickets
rarified 10 months ago
parent
commit
ca21750e3a
  1. 2
      id/51/51/journal.json

2
id/51/51/journal.json

@ -1 +1 @@
[{"date":"2023-06-25T15:49:44Z","author":"dgelessus","fields":{"title":"Fix possible use after free in plRegistryPageNode::UnloadKeys (\"link sound crash on exit\")","body":"Attempted fix for the well-known \"link sound crash on exit\". Works on my machine :)\n\nTo reproduce: link two avvies to the Ferry Terminal, then close the client that linked in first, wait a second or two, then close the second client. After a few seconds, the second client will crash with this stack trace:\n\n```\nUruLive.Dev.1.918 - Internal.Release\nException type: Access violation\nCall stack (12 levels, truncated):\n __alloca_probe (0x00742C20 + 0x000001C6)\n plResManager::IShutdown (0x0070E780 + 0x0000007E)\n plRegistryPageNode::UnloadKeys (0x007121A0 + 0x0000001E)\n plRegistryKeyList (0x00713D10 + 0x00000061)\n _EplKeyImp (0x00712DA0 + 0x0000000B)\n plKeyImp (0x005AE120 + 0x00000077)\n hsRefCnt::UnRef (0x0049BD70 + 0x00000010)\n _GplWin32LinkSound (0x005DF0C0 + 0x00000011)\n plWin32StaticSound (0x005DEBE0 + 0x00000050)\n plKey::IDecRef (0x005ABA10 + 0x00000026)\n plResManager::IKeyUnreffed (0x0070D870 + 0x0000001E)\n plResManager::FindPage (0x0070C2D0 + 0x00000053)\n```\n\nFix ported from [H-uru/Plasma#1400](https://github.com/H-uru/Plasma/pull/1400).\n\nCo-authored-by: Adam Johnson \u003cAdamJohnso@gmail.com\u003e","type":"Bug","severity":"Unrated","priority":"Normal","mergeTo":"master","watchers":"+dgelessus","status":"New"}},{"date":"2023-06-25T15:50:27Z","author":"dgelessus","patchset":{"number":1,"rev":1,"tip":"5d5ba00f7d057072ebc607b0e693b89540e81401","base":"4b0ab75a7917acd8abee82276da404ad40796319","insertions":4,"deletions":1,"commits":2,"added":0,"type":"Proposal"}},{"date":"2023-06-25T21:00:23Z","author":"rarified","comment":{"text":"In plRegistryKeylist.cpp with the combined effect of both commits, shouldn’t new line #64 be “fStaticKeys[i] \u003d nullptr;” instead of nulling out local variable keyImp (which is essentially a no-op since it is local to the for(){} block)? The committed code simply moves the corruption into a different state (fStaticKeys[i] pointing to a now deallocated memory block).","id":"838ed3790ae43e2955f01647d55420fbc44cf360"},"fields":{"watchers":"+rarified"}},{"date":"2023-06-25T21:19:02Z","author":"dgelessus","comment":{"text":"Ah, you\u0027re right. It\u0027s a reference on the H\u0027uru side and I didn\u0027t pay enough attention when adjusting that \u003e.\u003e Will fix, thanks.","id":"f2a7f438ad9466b1b43763ddb78002bcc46ab710"}},{"date":"2023-06-25T22:16:39Z","author":"dgelessus","patchset":{"number":1,"rev":2,"tip":"ea7e4b2ab54872f59234fe1370aa83b84e711c40","parent":"5d5ba00f7d057072ebc607b0e693b89540e81401","base":"4b0ab75a7917acd8abee82276da404ad40796319","insertions":1,"deletions":1,"commits":3,"added":1,"type":"FastForward"}},{"date":"2023-06-25T22:17:09Z","author":"dgelessus","comment":{"text":"OK, I fixed the code to null out the correct thing. Unfortunately, with this change, it doesn\u0027t actually fix the crash anymore :( I was actually expecting this and was surprised that my original commits worked...\n\nI think I know what really causes the crash in the OpenUru code (in short: the same thing for `plResManager::fAllPages`), but I still need to figure out how to fix that without breaking anything.","id":"fc6eb25ab8c36ca5d4ce0528d5ac28afceaf4506"}},{"date":"2023-06-29T20:13:39Z","author":"dgelessus","patchset":{"number":1,"rev":3,"tip":"7d3774a732a0cecd6655499eec529e86567f3c27","parent":"ea7e4b2ab54872f59234fe1370aa83b84e711c40","base":"4b0ab75a7917acd8abee82276da404ad40796319","insertions":11,"deletions":5,"commits":4,"added":1,"type":"FastForward"}},{"date":"2023-06-29T20:18:30Z","author":"dgelessus","comment":{"text":"OK, added a proper fix for the crash now. On my local testing setup, it works like it should.","id":"d5c3f80c5faa0543bd399e9b1dd222a755ec0a6d"}}]
[{"date":"2023-06-25T15:49:44Z","author":"dgelessus","fields":{"title":"Fix possible use after free in plRegistryPageNode::UnloadKeys (\"link sound crash on exit\")","body":"Attempted fix for the well-known \"link sound crash on exit\". Works on my machine :)\n\nTo reproduce: link two avvies to the Ferry Terminal, then close the client that linked in first, wait a second or two, then close the second client. After a few seconds, the second client will crash with this stack trace:\n\n```\nUruLive.Dev.1.918 - Internal.Release\nException type: Access violation\nCall stack (12 levels, truncated):\n __alloca_probe (0x00742C20 + 0x000001C6)\n plResManager::IShutdown (0x0070E780 + 0x0000007E)\n plRegistryPageNode::UnloadKeys (0x007121A0 + 0x0000001E)\n plRegistryKeyList (0x00713D10 + 0x00000061)\n _EplKeyImp (0x00712DA0 + 0x0000000B)\n plKeyImp (0x005AE120 + 0x00000077)\n hsRefCnt::UnRef (0x0049BD70 + 0x00000010)\n _GplWin32LinkSound (0x005DF0C0 + 0x00000011)\n plWin32StaticSound (0x005DEBE0 + 0x00000050)\n plKey::IDecRef (0x005ABA10 + 0x00000026)\n plResManager::IKeyUnreffed (0x0070D870 + 0x0000001E)\n plResManager::FindPage (0x0070C2D0 + 0x00000053)\n```\n\nFix ported from [H-uru/Plasma#1400](https://github.com/H-uru/Plasma/pull/1400).\n\nCo-authored-by: Adam Johnson \u003cAdamJohnso@gmail.com\u003e","type":"Bug","severity":"Unrated","priority":"Normal","mergeTo":"master","watchers":"+dgelessus","status":"New"}},{"date":"2023-06-25T15:50:27Z","author":"dgelessus","patchset":{"number":1,"rev":1,"tip":"5d5ba00f7d057072ebc607b0e693b89540e81401","base":"4b0ab75a7917acd8abee82276da404ad40796319","insertions":4,"deletions":1,"commits":2,"added":0,"type":"Proposal"}},{"date":"2023-06-25T21:00:23Z","author":"rarified","comment":{"text":"In plRegistryKeylist.cpp with the combined effect of both commits, shouldn’t new line #64 be “fStaticKeys[i] \u003d nullptr;” instead of nulling out local variable keyImp (which is essentially a no-op since it is local to the for(){} block)? The committed code simply moves the corruption into a different state (fStaticKeys[i] pointing to a now deallocated memory block).","id":"838ed3790ae43e2955f01647d55420fbc44cf360"},"fields":{"watchers":"+rarified"}},{"date":"2023-06-25T21:19:02Z","author":"dgelessus","comment":{"text":"Ah, you\u0027re right. It\u0027s a reference on the H\u0027uru side and I didn\u0027t pay enough attention when adjusting that \u003e.\u003e Will fix, thanks.","id":"f2a7f438ad9466b1b43763ddb78002bcc46ab710"}},{"date":"2023-06-25T22:16:39Z","author":"dgelessus","patchset":{"number":1,"rev":2,"tip":"ea7e4b2ab54872f59234fe1370aa83b84e711c40","parent":"5d5ba00f7d057072ebc607b0e693b89540e81401","base":"4b0ab75a7917acd8abee82276da404ad40796319","insertions":1,"deletions":1,"commits":3,"added":1,"type":"FastForward"}},{"date":"2023-06-25T22:17:09Z","author":"dgelessus","comment":{"text":"OK, I fixed the code to null out the correct thing. Unfortunately, with this change, it doesn\u0027t actually fix the crash anymore :( I was actually expecting this and was surprised that my original commits worked...\n\nI think I know what really causes the crash in the OpenUru code (in short: the same thing for `plResManager::fAllPages`), but I still need to figure out how to fix that without breaking anything.","id":"fc6eb25ab8c36ca5d4ce0528d5ac28afceaf4506"}},{"date":"2023-06-29T20:13:39Z","author":"dgelessus","patchset":{"number":1,"rev":3,"tip":"7d3774a732a0cecd6655499eec529e86567f3c27","parent":"ea7e4b2ab54872f59234fe1370aa83b84e711c40","base":"4b0ab75a7917acd8abee82276da404ad40796319","insertions":11,"deletions":5,"commits":4,"added":1,"type":"FastForward"}},{"date":"2023-06-29T20:18:30Z","author":"dgelessus","comment":{"text":"OK, added a proper fix for the crash now. On my local testing setup, it works like it should.","id":"d5c3f80c5faa0543bd399e9b1dd222a755ec0a6d"}},{"date":"2023-07-02T16:10:06Z","author":"rarified","fields":{"reviewers":"+rarified"},"review":{"patchset":1,"rev":3,"score":1}}]
Loading…
Cancel
Save