Use-after-free under icaltimezone_convert_time()

Mon Oct 9 15:58:09 PDT 2017

Milan,

I spent some time testing the builtin-timezone stuff today and it doesn't even work.
I need to restore the #if 0 code in icaltimezone_get_builtin_timezone_from_tzid for one thing.

Anyway, regarding the threading issue... I don't really have advice.
I wonder if running in valgrind's helgrind will show any hints.

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

On Tuesday, October 3, 2017 9:17:33 AM EDT Milan Crha wrote:
> 	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
> 
> 
> _______________________________________________
> libical-devel mailing list
> libical-devel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/libical-devel
> 