Correctly handle kernel addresses in an x86 stack trace

[mihai12p]

On an x86 application, ULONG_PTR is 4 bytes long, which means it cannot handle 8-byte long kernel addresses used by a 64-bit system.

[mihai12p]

@microsoft-github-policy-service agree

[kylereedmsft]

Hi there,
Can you elaborate a little on this scenario? What's the krabs host architecture (x86?) and system architecture?

I'm looking at the code in schema.hpp and your change does not work correctly in the case that the frames are 32bit.
The current design is that we send two values in the case of the 64 bit frames and only one value in the case of a 32bit frame.

The following function also needs to be updated so that in the 64bit case only one value is pushed and in the 32bit case, the address is widened before it is pushed.

    inline std::vector<ULONG_PTR> schema::stack_trace() const
    {
        std::vector<ULONG_PTR> call_stack;
        if (record_.ExtendedDataCount != 0) {
            for (USHORT i = 0; i < record_.ExtendedDataCount; i++)
            {
                auto item = record_.ExtendedData[i];
                if (item.ExtType == EVENT_HEADER_EXT_TYPE_STACK_TRACE64) {
                    auto stacktrace = reinterpret_cast<PEVENT_EXTENDED_ITEM_STACK_TRACE64>(item.DataPtr);
                    auto stack_length = (item.DataSize - sizeof(ULONG64)) / sizeof(ULONG64);
                    for (size_t j = 0; j < stack_length; j++)
                    {
                        call_stack.push_back(stacktrace->Address[j]);
                    }
                }
                else if (item.ExtType == EVENT_HEADER_EXT_TYPE_STACK_TRACE32) {
                    auto stacktrace = reinterpret_cast<PEVENT_EXTENDED_ITEM_STACK_TRACE32>(item.DataPtr);
                    auto stack_length = (item.DataSize - sizeof(ULONG64)) / sizeof(ULONG);
                    for (size_t j = 0; j < stack_length; j++)
                    {
                        call_stack.push_back(stacktrace->Address[j]);
                    }
                }
            }
        }

        return call_stack;
    }

[mihai12p]

Hi there,

I was testing on an x64 system with an x86 (WoW64) application. The issue I encountered is that 8-byte long addresses (particularly kernel addresses due to the x64 system) are being truncated because of the ULONG_PTR vector type, which is 4 bytes long in an x86 application.

When the header type is x64, the stack_length corresponds to the number of stack addresses, each being ULONG64 (8 bytes). In the loop, call_stack.push_back(stacktrace->Address[j]); only pushes the lower DWORD of the 8-byte address because call_stack values are of type ULONG_PTR. This is where the truncation occurs.

Regarding the function you mentioned, I believe it does not need to be updated, as the upcast to ULONG64 will happen automatically in the case of 4-byte addresses.

My proposed solution is to change the vector to contain only 8-byte long values. This way, it can accommodate both 4-byte and 8-byte addresses without issues.

Please let me know if I’m overlooking something. I’ve tested this approach, and it behaves as expected.

[kylereedmsft]

Ok, I looked closer at the stack track function and the two types EVENT_EXTENDED_ITEM_STACK_TRACE32 and EVENT_EXTENDED_ITEM_STACK_TRACE64. You're right, the function is correct.

Widening the vector should work. I'm not sure why we used ULONG_PTR here instead of ULONG or ULONG64.

Looks good. Thanks for the PR!

[kylereedmsft]

Looks good! Thanks for the PR.

[swannman]

@mihai12p would you mind updating the version numbers and release notes in the .nuspec files so we can publish an updated version after merging your change? Thanks!

[swannman]

Also, looks like we have a build failure - have you taken a look at the compilation error?

[mihai12p]

I’ve fixed the build issues and updated the release notes as requested.

I’m looking forward to more improvements as I’ll be using krabsetw extensively. Thank you!