Re: [PATCH] Fix heap-use-after-free in index-cached with --disable-threading

[Hannes Domani <ssbssa@yahoo.de>]

 Am Freitag, 10. Mai 2024 um 07:57:58 MESZ hat Bernd Edlinger <bernd.edlinger@hotmail.de> Folgendes geschrieben:

> On 5/4/24 18:56, Hannes Domani wrote:
>
> >  Am Samstag, 4. Mai 2024 um 17:45:06 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:
> >
> >>>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
> >>
> >> Hannes> Fixed by making cooked_index_worker::wait only return true if desired_state
> >> Hannes> is CACHE_DONE, same as if threading was enabled, so m_state will not be
> >> Hannes> prematurely deleted.
> >>
> >> Hannes> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31694
> >>
> >> Thank you.  This is ok.
> >> Approved-By: Tom Tromey <tom@tromey.com>
> >
> > Pushed, thanks.
> >
> >
> > Hannes
>
> >
> Hi,
>
> due to this incident you fixed here, I did some testing with tsan,
> and found a couple issues that I think are important, but I have no
> good idea how to solve them.
> https://sourceware.org/bugzilla/show_bug.cgi?id=31713
> https://sourceware.org/bugzilla/show_bug.cgi?id=31715
> https://sourceware.org/bugzilla/show_bug.cgi?id=31716
>
> I have found an issue (bug#31715) with the function
> cooked_index_worker::wait that was changed here.
> In one of the tsan reports I see something interesting here:
> https://sourceware.org/bugzilla/attachment.cgi?id=15506
> The cooked_index_worker::wait apparently proceeds and reads
> the "canonical" using cooked_index_entry::full_name
> without lock, and later a worker thread changes this value
> also without lock.
> Do you have any idea what is going on here?

Looks to me they are because while the background DWARF reading is happening,
gdb is processing some command (break/load/set), and both are accessing the
same memory.


Hannes