Initialization of buffer in hashing.h is wrong, causing crash when extracting TrackIR firmware

[Vantskruv]

In the constructor of FashHash in hashing.h, buffer is reserved, but not allocated. Hence: buffer.reserve(length); should be replaced by: buffer.resize(length);

This fixed my problem when extracting firmware for the TrackIR, causing the application to crash and fail the extraction. With the above fix, it works.

[Vantskruv]

Created a pull-request to fix this problem. https://github.com/uglyDwarf/linuxtrack/pull/201

I am not sure if should close this topic as of yet ...

[exuvo]

Nice fix.

[uglyDwarf]

In the constructor of FashHash in hashing.h, buffer is reserved, but not allocated. Hence: buffer.reserve(length); should be replaced by: buffer.resize(length);

This fixed my problem when extracting firmware for the TrackIR, causing the application to crash and fail the extraction. With the above fix, it works.

Hello, I was going through the code and I don't see how the original code with reserve would cause a crash (as it does allocate enough memory, that is initialized immediately after the reserve). Do you by any chance remember where exactly that crash occur?

Kind regards,

Michal

[exuvo]

I agree code looks like it should work either way but i think this fixed the firmware extraction crash for me too. Maybe the problem is elsewhere and this just happens to move things around.

[Vantskruv]

I do not see any allocation of the buffer, therefore it crashes. Hence, reserve does not allocate the array.

[Vantskruv]

Sorry, I was wrong about this. It seems reserve does allocate memory!

Though, I think the difference is that when doing reserve, it only allocates memory, but does not size the array. The vector is still 0 size, and accessing the array out of bounds causes the crash. So you need call push_back or insert to add elements to the array.

Using resize(), allocates the array, and sets the size of the array. So you can access all the elements in the array. Point me if I am wrong.

[exuvo]

Well then i am "pointing you" as you are wrong. Vector [] does not do bounds checking. Only if you use .at. Both reserve and resize allocate memory but resize also initializes the elements and updates the size variable. Elements can be accessed with both as long as you handle that with reserve they are not initialized to a known value and vectors size variable is unmodified.

[Vantskruv]

Then I am as confused as you are. As the change I made fixed the problem. :stuck_out_tongue_closed_eyes:

[exuvo]

A code change in unrelated sections of a program can move around memory placement, so there probably is a use after free, reference to old stack variable or a out of bounds bug somewhere that became non-crashing with this change. Valgrind should be able catch it if you want to give it a try.

[Vantskruv]

According to documentation, calling reserve only allocates memory (increasing its capacity size), it does not increase the vector size. In this case, you are accessing the vector at indexes higher than its vector size. This is called undefined behaviour according to the documentation: Source: https://cplusplus.com/reference/vector/vector/operator[]/

[uglyDwarf]

According to documentation, calling reserve only allocates memory (increasing its capacity size), it does not increase the vector size. In this case, you are accessing the vector at indexes higher than its vector size. This is called undefined behaviour according to the documentation: Source: https://cplusplus.com/reference/vector/vector/operator[]/

Good thinking! It didn't occur to me... Anyway I changed the code to be more C++ like (avoiding these cludges altogether), so this shouldn't cause issues anymore... Kind regards,

Michal