[PATCH] fix xfwm4 crash on OpenBSD; time_t size problem

[Matthew Brush]

On 14-02-08 02:49 AM, Stefan Sperling wrote:
> Cc'ing xfce4-dev -- I'm subscribed here, too, though usually just lurking.
> I'm leaving the context intact for reference.
>
> [snip]
>
> The crash happens because this code is walking a corrupt list.
> The list is corrupt because xfwm4 uses pointers of the wrong size
> (long instead of time_t) while constructing elements of the list.
> time_t is always 64bit on OpenBSD. But 'long' is 32bit on OpenBSD/i386.
> This explains why it doesn't crash on OpenBSD/amd64 where long' is 64bit.
>
> (code from libstartup-notification)
> [[[
> void
> sn_startup_sequence_get_last_active_time (SnStartupSequence *sequence,
>                                            time_t            *tv_sec,
>                                            suseconds_t       *tv_usec)
> {
>    /* for now the same as get_initiated_time */
>    if (tv_sec)
>      *tv_sec = sequence->initiation_time.tv_sec;
>    if (tv_usec)
>      *tv_usec = sequence->initiation_time.tv_usec;
> }
> ]]]
>

http://cgit.freedesktop.org/startup-notification/tree/libsn/sn-monitor.c?id=bc7c6d2d285509c7fcd1899c866838bd278fab58#n388

It seems to be using long ints as well unlike above paste, but it sets 
those values from a struct timeval which does use time_t and 
suseconds_t. Maybe the pasted code is from a (correctly) patched libsn 
where the patch hasn't been upstreamed yet?

> (code from xfwm4)
> [[[
> static void
> sn_collect_timed_out_foreach (void *element, void *data)
> {
>      CollectTimedOutData *ctod;
>      SnStartupSequence *sequence;
>      long tv_sec, tv_usec;
>      double elapsed;
>
>      g_return_if_fail (data != NULL);
>      g_return_if_fail (element != NULL);
>
>      sequence = element;
>      ctod = (CollectTimedOutData *) data;
>      sn_startup_sequence_get_last_active_time (sequence, &tv_sec, &tv_usec);
>
>      elapsed =
>          ((((double) ctod->now.tv_sec - tv_sec) * G_USEC_PER_SEC +
>              (ctod->now.tv_usec - tv_usec))) / 1000.0;
>
>      if (elapsed > STARTUP_TIMEOUT)
>      {
>          ctod->list = g_slist_prepend (ctod->list, sequence);
>      }
> }
> ]]]
>
> The above call to sn_startup_sequence_get_last_active_time() probably
> corrupts the value of the 'sequence' pointer on the stack, which is
> then copied into the list.
>
> This patch fixes the problem for me.
>
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/x11/xfce4/xfwm4/Makefile,v
> retrieving revision 1.42
> diff -u -p -r1.42 Makefile
> --- Makefile	31 May 2013 19:21:10 -0000	1.42
> +++ Makefile	8 Feb 2014 10:25:45 -0000
> @@ -3,6 +3,7 @@
>   COMMENT=	Xfce4 window manager
>
>   XFCE_PROJECT=	xfwm4
> +REVISION=	0
>
>   # GPLv2
>   PERMIT_PACKAGE_CDROM=	Yes
> Index: patches/patch-src_startup_notification_c
> ===================================================================
> RCS file: patches/patch-src_startup_notification_c
> diff -N patches/patch-src_startup_notification_c
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ patches/patch-src_startup_notification_c	8 Feb 2014 10:09:17 -0000
> @@ -0,0 +1,13 @@
> +$OpenBSD$
> +--- src/startup_notification.c.orig	Sat Feb  8 11:08:22 2014
> ++++ src/startup_notification.c	Sat Feb  8 11:08:13 2014
> +@@ -126,7 +126,8 @@ sn_collect_timed_out_foreach (void *element, void *dat
> + {
> +     CollectTimedOutData *ctod;
> +     SnStartupSequence *sequence;
> +-    long tv_sec, tv_usec;
> ++    time_t tv_sec;
> ++    suseconds_t tv_usec;

Should it explicitly include <sys/types.h> (or whatever header) to make 
sure those types are still available if whoever currently includes it 
stops doing so?

Cheers,
Matthew Brush