Potential bug causing GetPerTcpConnectionEStats to free user allocated buffer when called?

Freedom Sy 26

I just registered here to ask/report this. This is my code:

for (DWORD i = 0; i < ptTable->dwNumEntries; i++) 
{ MIB_TCPROW row;
row.dwLocalAddr = ptTable->table[i].dwLocalAddr;
row.dwLocalPort = ptTable->table[i].dwLocalPort;
row.dwRemoteAddr = ptTable->table[i].dwRemoteAddr;
row.dwRemotePort = ptTable->table[i].dwRemotePort;
row.dwState = ptTable->table[i].dwState;
if (row.dwRemoteAddr)
{ 
std::unique_ptr<UCHAR> rod = std::unique_ptr<UCHAR>(new UCHAR[rodSize]());
ret = GetPerTcpConnectionEStats
} ((PMIB_TCPROW)&row, TcpConnectionEstatsData, NULL, 0, 0, 0, 0, 0, (PUCHAR)rod.get(), 0, rodSize);
}

Whenever GetPerTcpConnectionEStats is called, the buffer gets freed (the bytes turn into 0xdd 0xdd...).

Note: The problem does not always occur. Within the same debuging session, it happens about 20 times out 100 "for loop" iterations given that every iteration calls GetPerTcpConnectionEStats twice, and only the first call (the one above) causes it.

Also the return from GetPerTcpConnectionEStats is NO_ERROR in all cases. Is it my code that's causing the problem or GetPerTcpConnectionEStats itself is broken?

Cheers

Viorel 112.1K Reputation points

2021-02-22T13:04:14.9+00:00

I think that it should be 'std::unique_ptr<UCHAR[]>', but this probably does not explain the issue. Check if it works better in this case.

Do you have some parallel threads, which maybe affects the memory?

2 answers

Freedom Sy 26 Reputation points

2021-02-22T11:54:48.79+00:00

Not sure why the format is messed up! Sorry, it wasn't like this when I clicked submit.
Please sign in to rate this answer.
Viorel 112.1K Reputation points

2021-02-22T12:48:12.803+00:00

Try editing the question, use the “101” button to insert the code.

Freedom Sy 26 Reputation points

2021-02-22T12:56:14.107+00:00

I updated the format and the problem, because I just noticed it doesn't have to do with the way the buffer is allocated (the buffer is getting freed regardless whether malloc, new or a smart pointer was used).
Sign in to comment
Freedom Sy 26 Reputation points

2021-02-22T13:19:34.047+00:00
This post is now more like a report, because it seems to work just fine if you don't heap allocate at all. The following code works for me, might not work for everyone though:

TCP_ESTATS_DATA_ROD_v0 rod = { 0 }; ret= GetPerTcpConnectionEStats((PMIB_TCPROW)&row, TcpConnectionEstatsData, NULL, 0, 0, 0, 0, 0, (PUCHAR)&rod, 0, sizeof(TCP_ESTATS_DATA_ROD_v0));
Please sign in to rate this answer.
Strive Sun-MSFT 426 Reputation points

2021-02-24T07:11:20.657+00:00

Hello, @Freedom Sy

What is the definition of ptTable pointer? Please provide a minimal code example to reproduce the problem.

Freedom Sy 26 Reputation points

2021-02-24T07:36:11.687+00:00

std::unique_ptr<MIB_TCPTABLE_OWNER_PID> buffer; do { buffer = std::unique_ptr<MIB_TCPTABLE_OWNER_PID>(new MIB_TCPTABLE_OWNER_PID[dwSize]()); retValue = GetExtendedTcpTable(buffer.get(), &dwSize, TRUE, AF_INET, TCP_TABLE_OWNER_PID_ALL, 0); } while (ret == ERROR_INSUFFICIENT_BUFFER); PMIB_TCPTABLE_OWNER_PID ptTable = (PMIB_TCPTABLE_OWNER_PID)buffer.get();

Followed by the "for loop" from above.

Strive Sun-MSFT 426 Reputation points

2021-02-24T08:23:28.377+00:00

I am trying to stitch the code, but GetPerTcpConnectionEStats will return an ERROR_INVALID_USER_BUFFER error. I will appreciate you if you can provide a complete code sample :)

Freedom Sy 26 Reputation points

2021-02-24T09:02:16.33+00:00

God, it took me a while to reproduce. I also recorded it if you need.

I can't paste the code here because of the characters limit.

Also, I might have overseen something really dumb that's causing the problem, so go easy on me.

Here's the full (not) working code https://pastebin.com/EVHvD3TL. The code is missing a lot of error checks and safety measures, but it should reproduce the bug, at least it does for me.

And here's a screenshot of the step by step debug session: https://ibb.co/pRDWk0H

(Links expire in one week)

Also, parts of the code are from Malcolm Swaine's response at https://stackoverflow.com/questions/44689482/how-to-monitor-process-network-usage-under-windows

I would really appreciate a short response/feedback of what's causing the problem to occur.

Cheers

Strive Sun-MSFT 426 Reputation points

2021-02-24T10:04:23.437+00:00

Thanks for your code, I still can't reproduce your problem. Look at the gif below , winStatus will not always return 0, it will return some other error codes. And when returning 0, ptr will not be cleared.

Have you tested it on other machines?

My machine: Windows 10 20H2 VS2019

Freedom Sy 26 Reputation points

2021-02-24T10:50:47.493+00:00

Well, now I understand why it took me a while to reproduce the first time, because this time I also went ahead and tried again and couldn't get it to (not) work. The thing I did the first time that I didn't mention because I though it wasn't relevant is that I restarted my PC, and tested it immediately after restart.

Before restart, I also got return values like 50 and 1168 sometimes. (btw, you guys need to add the respective values of the return values (ERROR_INSUFFICIENT_BUFFER = whatever, etc...) at win32/api/iphlpapi/nf-iphlpapi-getpertcpconnectionestats I'm tired of "peeking definition" :))

Either way, I made a short video of it: streamable.com/wuqdkb

At this point, I'm kind of sure it's a bug.

I'm also on a Windows 10 20H2, using VS2019

Cheers

Strive Sun-MSFT 426 Reputation points

2021-02-25T07:23:37.143+00:00

After testing, I was able to reproduce your problem. This problem occurs after the first call to GetPerTcpConnectionEStats. I would agree with Viorel-1's point of view, which may be related to the automatic release of std::unique_ptr<UCHAR[]>. When I used malloc for testing, the same problem did not occur.

Freedom Sy 26 Reputation points

2021-02-25T07:45:06.037+00:00

What does the automatic release of a smart pointer have to do with GetPerTcpConnectionEStats?

I'm 100% sure I was able to reproduce it with malloc and raw pointers. I even edited the post as soon as I realized it doesn't have to do with smart pointers.

I've never had this happen with a smart pointer except when I used GetPerTcpConnectionEStats.

If you have the source code, you should be able to have look inside and tell at which point the memory is being freed.

That has nothing to do with my post though, thank you for helping and taking the time.

Cheers

Strive Sun-MSFT 426 Reputation points

2021-02-25T07:48:22.717+00:00

I will confirm this matter with the relevant engineers and update it here. please wait patiently.

Strive Sun-MSFT 426 Reputation points

2021-03-01T06:38:03.463+00:00

Can you send me sample that use malloc for testing?

Freedom Sy 26 Reputation points

2021-03-01T07:26:33.78+00:00

Unfortunately, I'm unable to reproduce it using malloc or raw pointers.

Strive Sun-MSFT 426 Reputation points

2021-03-02T09:44:36.947+00:00

The buffer is not released, GetPerTcpConnectionEStats gives an invalid value to the buffer.

Look at these two images,

After "YYYYYYYYYY" appeared, I manually released the buffer without error.

Freedom Sy 26 Reputation points

2021-03-02T09:53:58.857+00:00

Three questions here:

1- You seem to have been able to reproduce the thing using malloc right? or is that a smart pointer in the screenshots?

2- What's causing GetPerTcpConnectionEStats to return an invalid value to the buffer when it's return value is NO_ERROR as per screenshots?

3- Why and since when does YYYYYYYYYY represent an invalid value when that's not mentioned in the documentation? 0xdd 0xdd had always been the representation of a freed memory region from my experience at least, seems kind of unusual to see my buffer pointing to that same representation.

Either way, and again, my code works just fine using stack allocation so... this is more like me being curious no more.

Cheers
Sign in to comment