Use-after-free under icaltimezone_convert_time()

Milan Crha mcrha at redhat.com
Mon Oct 9 23:58:22 PDT 2017


On Mon, 2017-10-09 at 18:58 -0400, Allen Winter wrote:
> Anyway, regarding the threading issue... I don't really have advice.
> I wonder if running in valgrind's helgrind will show any hints.

	Hi,
I tried with valgrind first, but then I realized it slows it all that
much that it avoids the proper thread interleaving.

> The zone->changes uses icalarray stuff .. I wonder if icalarray isn't
> thread safe.

Even if icalarray was thread safe, which it isn't (no sign of pthread
calls in it), the backtrace I posted shows that one thread frees the
array, while another is still reading from it.

I think the best would be to add some thread safety around
icaltimezone::changes, but it seems one cannot do it by adding a
pthread mutex into icaltimezone structure, thus what about having a
global mutex, similar to builtin_mutex, in icaltimezone.c? That could
eventually slow down (not only multithreaded) applications, but I
suppose not for much, because the operations on the
icaltimezone::changes might be lightning fast. And due to avoiding the
crash, it surely worth it.

I made a proposed patch and attached it here. I tested it with
'make test' and it passes. It also slows it down by approximately half
a second here (sometime a bit more, sometimes a bit less). I'm afraid
it cannot run any better and being thread safe at the same time.

What do you think?

	Thanks and bye,
	Milan

P.S.: I'm sorry for the inconvenience, I've still no github account
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ical.patch
Type: text/x-patch
Size: 8916 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/libical-devel/attachments/20171010/5ea9d92b/attachment.bin>


More information about the libical-devel mailing list