Use-after-free under icaltimezone_convert_time()

Tue Oct 3 06:17:33 PDT 2017

	Hello,
I've git master of libical at commit
c452816ee23341d92ba9e1dbb43bada6502b69dd.

Opening Evolution in a Month View today results in a crash in 3 of 5
tries (subjective measure). Below are two threads accessing the same
timezone structure, both reading the zone->changes, when one thread
frees it, while the other is still using it. Both threads are using
different component, they only reuse the timezone structure (evolution-
data-server does that to minimize memory usage, while it prefers
built-in timezones provided by libical).

I do not want to blame libical for this use-after-free due to certain
thread interleaving, I'm rather asking whether there's anything to
avoid it. Maybe Evolution(-data-server) should not reuse the timezone
structure between components? Would it be ever possible with built-in
timezones? Am I able to call icaltimezone_expand_changes() with far-
enough year, thus the free of the zone->changes array is avoided? What
would be the far-enough year then?

I can try to propose some fixes to libical, ideally by adding thread
safety to icaltimezone_convert_time(), or the best to
icaltimezone::changes itself, though it could be many mutexes (one per
structure instance), which is probably not the best thing to do, even
it might allow higher concurrency, when there are used different
timezone instances in different threads. Or to change
icaltimezone_expand_changes() to not free the array, but rather merge
it with values (though some might be there multiple times, when the
expand would be called from both (or more) threads at the same time).

Does anyone have any advice, please? Can be also for
evolution(-data-server), though changing API to always return new
timezone, even for those built-in, doesn't sound like a good step, not
talking of the implications for clients of evolution-data-server.

	Thanks and bye,
	Milan

P.S.: This crash is not new, but it was hard to reproduce for me
      in the past; one GNOME bug report is:
      https://bugzilla.gnome.org/show_bug.cgi?id=780894

Here are the two threads in question (line numbers in icalarray.c do
not match, due to added custom code for debugging purposes):

Thread 12 (Thread 0x7f3d8bfff700 (LWP 27954)):
#3  0x00007f3dee4f13b0 in <signal handler called> () at /lib64/libpthread.so.0
#4  0x00007f3dedf83de4 in icalarray_element_at (array=0x7f3d84074a60, position=58) at ..../libical/src/libical/icalarray.c:137
#5  0x00007f3dedf9ab3f in icaltimezone_find_nearby_change (zone=0x7f3d84033c20, change=0x7f3d8bffd660) at ..../libical/src/libical/icaltimezone.c:1020
#6  0x00007f3dedf9a5ab in icaltimezone_get_utc_offset (zone=0x7f3d84033c20, tt=0x7f3d8bffd720, is_daylight=0x0) at ..../libical/src/libical/icaltimezone.c:800
#7  0x00007f3dedf9a3fd in icaltimezone_convert_time (tt=0x7f3d8bffd720, from_zone=0x7f3d84033c20, to_zone=0x7f3dee1ca760 <utc_timezone>) at ..../libical/src/libical/icaltimezone.c:747
#8  0x00007f3dedf96de9 in icaltime_convert_to_zone (tt=..., zone=0x7f3dee1ca760 <utc_timezone>) at ..../libical/src/libical/icaltime.c:989
#9  0x00007f3dedf96658 in icaltime_compare (a_in=..., b_in=...) at ..../libical/src/libical/icaltime.c:744
#10 0x00007f3ddbfafa9c in e_timetype_compare_without_date (a=0x7f3d8bffd900, b=0x7f3d8bffe740) at ..../evolution-data-server/src/calendar/libecal/e-cal-recur.c:68
#11 0x00007f3ddbfb02e0 in intersects_interval (tt=0x7f3d8bffe5c0, duration=0x0, default_duration_days=0, default_duration_seconds=3600, interval_start=0x7f3d8bffe710, interval_end=0x7f3d8bffe740) at ..../evolution-data-server/src/calendar/libecal/e-cal-recur.c:292
#12 0x00007f3ddbfb0dff in e_cal_recur_generate_instances_sync (comp=0x7f3d84080580, interval_start=..., interval_end=..., callback=0x7f3ddbfb1eb5 <backward_compatibility_instance_cb>, callback_user_data=0x7f3d8bffe7b0, get_tz_callback=0x7f3ddbfb1e5c <backward_compatibility_resolve_timezone_cb>, get_tz_callback_user_data=0x7f3d8bffe7b0, default_timezone=0x7feacd0, cancellable=0x0, error=0x0) at ..../evolution-data-server/src/calendar/libecal/e-cal-recur.c:454
#13 0x00007f3ddbfb20d8 in e_cal_recur_generate_instances (comp=0x7f3d84080530 [ECalComponent], start=1506981600, end=1507586399, cb=0x7f3ddbf96338 <add_instance>, cb_data=0x7f3d840738a0, tz_cb=0x7f3ddbf96147 <e_cal_client_resolve_tzid_cb>, tz_cb_data=0x7f3ce4001140, default_timezone=0x7feacd0) at ..../evolution-data-server/src/calendar/libecal/e-cal-recur.c:1334
#14 0x00007f3ddbf9726d in generate_instances (client=0x7f3ce4001140 [ECalClient], start=1506981600, end=1507586399, objects=0x7f3d84087330 = {...}, cancellable=0x0, cb=0x7f3ddbf96338 <add_instance>, cb_data=0x7f3d84052320) at ..../evolution-data-server/src/calendar/libecal/e-cal-client.c:2696
#15 0x00007f3ddbf98ac5 in e_cal_client_generate_instances_for_object_sync (client=0x7f3ce4001140 [ECalClient], icalcomp=0x7e49ae0, start=1506981600, end=1507586399, cb=0x7f3daaca8b87 <cal_data_model_instance_generated>, cb_data=0x7f3d8bffea80) at ..../evolution-data-server/src/calendar/libecal/e-cal-client.c:3353
#16 0x00007f3daaca90ec in cal_data_model_expand_recurrences_thread (data_model=0x79ed590 [ECalDataModel], user_data=0x7f3ce4001140) at ..../evolution/src/calendar/gui/e-cal-data-model.c:1112
#17 0x00007f3daaca74ad in cal_data_model_internal_thread_job_func (data=0x7d6df60, user_data=0x79ed590) at ..../evolution/src/calendar/gui/e-cal-data-model.c:391
#18 0x00007f3dee23df00 in g_thread_pool_thread_proxy (data=<optimized out>) at gthreadpool.c:307
#19 0x00007f3dee23d536 in g_thread_proxy (data=0x7f3d90003840) at gthread.c:784
#20 0x00007f3dee4e636d in start_thread () at /lib64/libpthread.so.0
#21 0x00007f3de9e2ebbf in clone () at /lib64/libc.so.6

Thread 25 (Thread 0x7f3cabfff700 (LWP 28111)):
#19 0x00007f3dedf83caa in icalarray_free (array=0x7f3d84074a60) at ..../libical/src/libical/icalarray.c:112
#20 0x00007f3dedf99c17 in icaltimezone_expand_changes (zone=0x7f3d84033c20, end_year=2022) at ..../libical/src/libical/icaltimezone.c:480
#21 0x00007f3dedf99b67 in icaltimezone_ensure_coverage (zone=0x7f3d84033c20, end_year=2014) at ..../libical/src/libical/icaltimezone.c:452
#22 0x00007f3dedf9a51a in icaltimezone_get_utc_offset (zone=0x7f3d84033c20, tt=0x7f3cabffd880, is_daylight=0x0) at ..../libical/src/libical/icaltimezone.c:784
#23 0x00007f3dedf9a3fd in icaltimezone_convert_time (tt=0x7f3cabffd880, from_zone=0x7f3d84033c20, to_zone=0x7f3dee1ca760 <utc_timezone>) at ..../libical/src/libical/icaltimezone.c:747
#24 0x00007f3dedf95b26 in icaltime_as_timet_with_zone (tt=..., zone=0x7f3d84033c20) at ..../libical/src/libical/icaltime.c:324
#25 0x00007f3ddbfb0923 in e_cal_recur_generate_instances_sync (comp=0x7f3d1005fbf0, interval_start=..., interval_end=..., callback=0x7f3ddbfb1eb5 <backward_compatibility_instance_cb>, callback_user_data=0x7f3cabffe7b0, get_tz_callback=0x7f3ddbfb1e5c <backward_compatibility_resolve_timezone_cb>, get_tz_callback_user_data=0x7f3cabffe7b0, default_timezone=0x7feacd0, cancellable=0x0, error=0x0) at ..../evolution-data-server/src/calendar/libecal/e-cal-recur.c:401
#26 0x00007f3ddbfb20d8 in e_cal_recur_generate_instances (comp=0x7f3d1005fba0 [ECalComponent], start=1506290400, end=1509922799, cb=0x7f3ddbf96338 <add_instance>, cb_data=0x7f3d10052f00, tz_cb=0x7f3ddbf96147 <e_cal_client_resolve_tzid_cb>, tz_cb_data=0x7f3ce4001140, default_timezone=0x7feacd0) at ..../evolution-data-server/src/calendar/libecal/e-cal-recur.c:1334
#27 0x00007f3ddbf9726d in generate_instances (client=0x7f3ce4001140 [ECalClient], start=1506290400, end=1509922799, objects=0x7f3d10066970 = {...}, cancellable=0x0, cb=0x7f3ddbf96338 <add_instance>, cb_data=0x7f3d100258b0) at ..../evolution-data-server/src/calendar/libecal/e-cal-client.c:2696
#28 0x00007f3ddbf98ac5 in e_cal_client_generate_instances_for_object_sync (client=0x7f3ce4001140 [ECalClient], icalcomp=0x7ffa290, start=1506290400, end=1509922799, cb=0x7f3daaca8b87 <cal_data_model_instance_generated>, cb_data=0x7f3cabffea80) at ..../evolution-data-server/src/calendar/libecal/e-cal-client.c:3353
#29 0x00007f3daaca90ec in cal_data_model_expand_recurrences_thread (data_model=0x7564aa0 [ECalDataModel], user_data=0x7f3ce4001140) at ..../evolution/src/calendar/gui/e-cal-data-model.c:1112
#30 0x00007f3daaca74ad in cal_data_model_internal_thread_job_func (data=0x7aef010, user_data=0x7564aa0) at ..../evolution/src/calendar/gui/e-cal-data-model.c:391
#31 0x00007f3dee23df00 in g_thread_pool_thread_proxy (data=<optimized out>) at gthreadpool.c:307
#32 0x00007f3dee23d536 in g_thread_proxy (data=0x8000a00) at gthread.c:784
#33 0x00007f3dee4e636d in start_thread () at /lib64/libpthread.so.0
#34 0x00007f3de9e2ebbf in clone () at /lib64/libc.so.6