eden-emu/eden - eden - InfoTonic

xbzk b6ee847947 [hle/am] make Service::Process move-only to fix #3908 KProcess use-after-free (#4137 ) #3908 changed process_creation., CreateProcess/CreateApplicationProcess, to return std::optional<Process> instead of std::unique_ptr<Process>, so the AM sites now transfer a Process by value via make_unique<Service::Process>(std::move(opt)). The consequence: Process owns a refcounted KProcess* but its user-declared dtor suppressed the implicit move ctor, so that "move" silently shallow-copied and the temporary's dtor Close()/RemoveProcess()'d the shared handle -> use-after-free. It's seems to be user end based, so whether it crashes may depend on machine, compiler, allocator reuse, refcount slack, and the AM event-observer thread race, idk. It reliably crashed my MSVC build at launching games (cstack: ProcessHolder -> MultiWait -> KSynchronizationObject::Wait -> null) multiple times. Fix: give Process a move ctor that steals the handle (nulling the source so the moved-from dtor is a no-op) and delete copy/move-assign, making the optional<->unique_ptr transfer safe. Bonus: explicited delete for the 3 kinds of assignment: copy ctor (the one used in eden), copy assign and move assign (currently unused) to force compile error if they ever come to use. Reviewed-on: https://git.eden-emu.dev/eden-emu/eden/pulls/4137 Reviewed-by: Lizzie <lizzie@eden-emu.dev> Reviewed-by: MaranBr <maranbr@eden-emu.dev>	1 month ago
..
0001-linkerns-cpm.patch	[cmake] refactor: Use CPM over submodules (#143)	1 year ago

[hle/am] make Service::Process move-only to fix #3908 KProcess use-after-free (#4137 )

#3908 changed process_creation.*, CreateProcess/CreateApplicationProcess, to return std::optional<Process> instead of std::unique_ptr<Process>, so the AM sites now transfer a Process by value via make_unique<Service::Process>(*std::move(opt)).

The consequence: Process owns a refcounted KProcess* but its user-declared dtor suppressed the implicit move ctor, so that "move" silently shallow-copied and the temporary's dtor Close()/RemoveProcess()'d the shared handle -> use-after-free.

It's seems to be user end based, so whether it crashes may depend on machine, compiler, allocator reuse, refcount slack, and the AM event-observer thread race, idk. It reliably crashed my MSVC build at launching games (cstack: ProcessHolder -> MultiWait -> KSynchronizationObject::Wait -> null) multiple times.

Fix: give Process a move ctor that steals the handle (nulling the source so the moved-from dtor is a no-op) and delete copy/move-assign, making the optional<->unique_ptr transfer safe.

Bonus: explicited delete for the 3 kinds of assignment: copy ctor (the one used in eden), copy assign and move assign (currently unused) to force compile error if they ever come to use.

Reviewed-on: https://git.eden-emu.dev/eden-emu/eden/pulls/4137
Reviewed-by: Lizzie <lizzie@eden-emu.dev>
Reviewed-by: MaranBr <maranbr@eden-emu.dev>

1 month ago

0001-linkerns-cpm.patch

[cmake] refactor: Use CPM over submodules (#143)

1 year ago