Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update FLT_FILE_NAME_INFORMATION usage to match docs #1371

Merged
merged 1 commit into from
Nov 14, 2021

Conversation

Therzok
Copy link
Contributor

@Therzok Therzok commented Nov 14, 2021

This is possible a performance issue I've noticed on my system, where the FMfn tag pool is using increasingly more memory.

It all seems be pointing to fltMgr.sys, which I guess is this FltGetFileNameInformation call.

The code-path seems to be querying FltGetFileNameInformation, and on the success path, it only released the filename when the spool folder work is allowed.

That meant this was effectively leaking a reference to FLT_FILE_NAME_INFORMATION for non-spooler work folders, because as the docs state:

After a successful call to FltGetFileNameInformation, the caller is responsible for releasing the pointer returned in the FileNameInformation parameter when the pointer is no longer needed. The caller does this by calling FltReleaseFileNameInformation.

This is possible a performance issue I've noticed on my system, where the `FMfn tag` pool is using increasingly more memory.

It all seems be pointing to fltMgr.sys, which I guess is this `FltGetFileNameInformation` call.

The code-path seems to be querying `FltGetFileNameInformation`, and on the success path, it only released the filename when the spool folder work is allowed.

That meant this was effectively leaking a reference to `FLT_FILE_NAME_INFORMATION` for non-spooler work folders, because as the docs state:
> After a successful call to FltGetFileNameInformation, the caller is responsible for releasing the pointer returned in the FileNameInformation parameter when the pointer is no longer needed. The caller does this by calling FltReleaseFileNameInformation.
@DavidXanatos DavidXanatos merged commit 43d7cba into sandboxie-plus:master Nov 14, 2021
@DavidXanatos
Copy link
Member

great find will check it out on my system asap

@Therzok Therzok deleted the patch-1 branch November 14, 2021 17:33
@Therzok
Copy link
Contributor Author

Therzok commented Nov 14, 2021

Yeah, over 2 days of usage, I ended up with a lot of pools leaked - validated via poolmon. There were also a lot of mapped memory file handle leaks of files that have only been open temporarily, so I suspect this is what caused it.

@DavidXanatos
Copy link
Member

Yea there are many old bugs in the sandboxie code base.

@isaak654
Copy link
Collaborator

isaak654 commented Dec 1, 2021

It looks like this pull request may have stopped the ability to print inside sandboxes for some Chrome/Firefox users: #1376 (comment)

@DavidXanatos
Copy link
Member

hmm... not good because we should avoid memory leaks still

@isaak654 isaak654 added recently broken Type: Regression A Sandboxie build broke compatibility, it was working before and removed Type: Regression A Sandboxie build broke compatibility, it was working before recently broken labels Dec 7, 2021
@isaak654
Copy link
Collaborator

isaak654 commented Dec 7, 2021

My bad, fortunately this PR isn't involved in the printing issue.

@Therzok
Copy link
Contributor Author

Therzok commented Dec 7, 2021

I was really confused how it would cause that, but did not know enough about the Sandboxie internals or FLT API to respond on that :D

@DavidXanatos
Copy link
Member

I really dont think its reaponsible, also when in roll back the change printing remains broken, so its somethign different

@isaak654
Copy link
Collaborator

isaak654 commented Dec 9, 2021

I can confirm version 1.0.2 fixed the printing issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants