After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 749993 - Totem crashes at startup on 32bit systems
Totem crashes at startup on 32bit systems
Status: RESOLVED FIXED
Product: totem
Classification: Core
Component: GStreamer backend
3.16.x
Other Linux
: Normal normal
: ---
Assigned To: Maintainer alias for GStreamer component of Totem
Maintainer alias for GStreamer component of Totem
Depends on:
Blocks:
 
 
Reported: 2015-05-27 18:10 UTC by Mario Sánchez Prada
Modified: 2016-01-08 20:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch proposal (951 bytes, patch)
2015-05-27 18:15 UTC, Mario Sánchez Prada
none Details | Review
Patch proposal (1.34 KB, patch)
2015-05-28 09:52 UTC, Mario Sánchez Prada
accepted-commit_now Details | Review

Description Mario Sánchez Prada 2015-05-27 18:10:33 UTC
On 32bit systems, I've witnessed totem crashing due when calling g_object_set (bvw->priv->play, "connection-speed", MAX_NETWORK_SPEED, NULL) in bacon_video_widget_initable_init(), which seems to be happening due to the MAX_NETWORK_SPEED macro being declared like this:

  #define MAX_NETWORK_SPEED 10752

...instead of this:

  #define MAX_NETWORK_SPEED G_GUINT64_CONSTANT(10752)

The problem is that g_object_set() uses a var_args list internally, which will work fine in 64bit systems without using G_GUINT64_CONSTANT because connection-speed is an uint64 property. However, in 32bit systems the var_args list will read both the MAX_NETWORK_SPEED value and the terminating NULL passed as one element, and then will keep reading after the end of the arguments list, causing the crash.
 
See the relevant excerpt of the crashing backtrace below:

  • #0 __strchr_sse2_bsf
    at ../sysdeps/i386/i686/multiarch/strchr-sse2-bsf.S line 60
  • #1 g_param_spec_pool_lookup
    at /usr/src/packages/BUILD/glib2.0-2.44.0+dev29.e9741eb/./gobject/gparam.c line 1070
  • #2 g_object_set_valist
    at /usr/src/packages/BUILD/glib2.0-2.44.0+dev29.e9741eb/./gobject/gobject.c line 2120
  • #3 g_object_set
    at /usr/src/packages/BUILD/glib2.0-2.44.0+dev29.e9741eb/./gobject/gobject.c line 2268
  • #4 bacon_video_widget_initable_init
    at bacon-video-widget.c line 6033

Comment 1 Mario Sánchez Prada 2015-05-27 18:15:00 UTC
Created attachment 304097 [details] [review]
Patch proposal

Attached the patch that fixed the issue in my 32-bit system.

Please review, thanks.
Comment 2 Philip Withnall 2015-05-27 19:15:18 UTC
Review of attachment 304097 [details] [review]:

Thanks for the patch!

::: src/backend/bacon-video-widget.c
@@ +96,3 @@
 #define POPUP_HIDING_TIMEOUT 2
 
+#define MAX_NETWORK_SPEED G_GUINT64_CONSTANT(10752)

I think it would be better to do the (guint64) cast at the g_object_set() call which uses MAX_NETWORK_SPEED, rather than in the definition of MAX_NETWORK_SPEED itself, since there’s no real reason for it to be 64-bit otherwise.
Comment 3 Mario Sánchez Prada 2015-05-28 09:52:31 UTC
Created attachment 304139 [details] [review]
Patch proposal

(In reply to Philip Withnall from comment #2)
> [...]
> I think it would be better to do the (guint64) cast at the g_object_set()
> call which uses MAX_NETWORK_SPEED, rather than in the definition of
> MAX_NETWORK_SPEED itself, since there’s no real reason for it to be 64-bit
> otherwise.

No problem, I did that in this new patch as well as added a small comment explaining why we need to do that.
Comment 4 Philip Withnall 2015-05-28 09:54:20 UTC
Review of attachment 304139 [details] [review]:

Great, please commit to master! Thanks.
Comment 5 Mario Sánchez Prada 2015-05-28 09:56:44 UTC
Committed, thanks
Comment 6 Emmanuele Bassi (:ebassi) 2016-01-08 14:29:33 UTC
*** Bug 739034 has been marked as a duplicate of this bug. ***
Comment 7 Bastien Nocera 2016-01-08 20:08:18 UTC
I've pushed the patch to gnome-3-14 as well.