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

Avoid squirrelly memcpy() call in filesystem.cpp #4933

Conversation

StephanTLavavej
Copy link
Member

We have an extremely squirrelly line of code that's memcpying two consecutive DWORDs into the beginning of a buffer. There's no reason for this weirdness - it isn't perf-critical, and the optimizer should understand memcpy. Now, code analysis tools (specifically CodeQL) are noticing that this code is a 🐿️ read overrun. Let's avoid this by splitting it up into two separate reads.

For the destination, _Id points to FILE_ID_INFO. Its FileId is FILE_ID_128, which contains BYTE Identifier[16];.

For the source, _Info is BY_HANDLE_FILE_INFORMATION:

typedef struct _BY_HANDLE_FILE_INFORMATION {
  DWORD    dwFileAttributes;
  FILETIME ftCreationTime;
  FILETIME ftLastAccessTime;
  FILETIME ftLastWriteTime;
  DWORD    dwVolumeSerialNumber;
  DWORD    nFileSizeHigh;
  DWORD    nFileSizeLow;
  DWORD    nNumberOfLinks;
  DWORD    nFileIndexHigh;
  DWORD    nFileIndexLow;
} BY_HANDLE_FILE_INFORMATION, *PBY_HANDLE_FILE_INFORMATION, *LPBY_HANDLE_FILE_INFORMATION;

@StephanTLavavej StephanTLavavej added enhancement Something can be improved filesystem C++17 filesystem labels Sep 4, 2024
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner September 4, 2024 05:21
@StephanTLavavej StephanTLavavej self-assigned this Sep 9, 2024
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 479dcb9 into microsoft:main Sep 9, 2024
39 checks passed
@StephanTLavavej StephanTLavavej deleted the the-unbeatable-squirrel-filesystem branch September 9, 2024 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved filesystem C++17 filesystem
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants