fix(runtime): a refused late load closes the checked file's handle twice

Open Beginner friendly
#2,730 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
72/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
cpp

Research direction

Start in src/runtime/ext/plugin_loader.cpp at PluginLoadFilter, where entry->handle = h is set, and find the if (!enforcing) refusal branch that calls CloseHandle(h) before returning. The FilterState::Verified destructor also closes handle, so the double close is there. Write a test that counts closes on the file's handle for a refused late load and expects exactly one. Done when that test passes with the second close removed or the handle cleared after closing.

Written by the indexing model from the issue text.

Description

Evidence at 689ef6896

  • src/runtime/ext/plugin_loader.cpp, PluginLoadFilter: entry->handle = h; (the Verified entry owns the handle h of the file being checked).
  • FilterState::Verified::~Verified closes handle when it is not INVALID_HANDLE_VALUE.
  • On the path where the check refused the file and the filter goes on to the if (!enforcing) branch, the function calls CloseHandle(h); and returns; entry is still alive and still holds h, so its destructor closes the same handle value a second time.
  • On the accepted path the entry is moved into f.verified and the handle is not closed by the function (return false; comes first), so only the refusal path is affected.

Not shown

  • That another thread can be handed the same handle value between the two closes in a real run. In a process where a handle value is reused, the second close would shut an unrelated handle; in a debugger or with Application Verifier it raises an invalid-handle exception.
  • Whether Wine reports the second close.

Falsifier

A test (or a run under a handle-checking tool) that makes a late load refused by the plugin gate and sees only one CloseHandle on the file's handle would show this wrong.

Work

  1. Test first: a refused late load closes the file's handle exactly once (a seam that counts closes, or a duplicated handle the test checks is still valid afterwards).
  2. Fix: close it in one place. Either drop the CloseHandle(h) on the refusal path and let the entry close it, or clear entry->handle after the close.
Dominant language
C++
Stars
2
Forks
5
Avg merge
4h 12m
Merged PRs (30d)
788

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from EchoTools/nevr-runtime

All issues in EchoTools/nevr-runtime

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.