On 1/25/22 19:47, Dmitry Timoshkov wrote:
Hi Jacek,
Jacek Caban jacek@codeweavers.com wrote:
On 1/24/22 15:04, Dmitry Timoshkov wrote:
update_travellog() in order to clear forward history calls free_travellog_entry() to invalidate forward history entries, and when later an entry gets reused entry->stream contains a no longer valid pointer.
How does it "get reused"? Note that log buffer is also initially not zero-initialized and generally depends on proper bounds checks. update_travellog() decrements length when it clears forward history, which should prevent us from treating those entries as valid.
Probably "gets reused" is a wrong term. What I observe here is that once update_travellog() truncates the log, and position in the history is equal to the length, next call to go_forward() will crash because bounds check 'if (position >= length) return E_FAIL;' doesn't prevent referencing a no longer valid history entry. Does that explain what is going on?
Okay, so it's navigate_history() where the problem happens. It uses NULL check and the situation can only happen if forward history was cleared. Bound checks prevent from using an entry that was never initialized, so that case is fine. The patch looks good then.
Thanks,
Jacek