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 626684 - Segfault when invoking methods with "out" GValues with wrong arguments
Segfault when invoking methods with "out" GValues with wrong arguments
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: introspection
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
: 625583 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-08-11 21:42 UTC by Simon Pena
Modified: 2010-09-23 22:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Testcase showing the explained behavior (87 bytes, application/python)
2010-08-11 21:42 UTC, Simon Pena
  Details
Partial fix (1.12 KB, patch)
2010-08-11 21:47 UTC, Simon Pena
needs-work Details | Review
Testcase showing the free crash when the GValue hasn't been set (146 bytes, application/python)
2010-08-11 21:54 UTC, Simon Pena
  Details
Remove useless checks. (1.60 KB, patch)
2010-08-12 09:17 UTC, Simon van der Linden
none Details | Review
Fix caller-allocates emergency free. (1.72 KB, patch)
2010-08-12 09:21 UTC, Simon van der Linden
none Details | Review
Fix caller-allocates emergency free. (1.72 KB, patch)
2010-08-12 09:34 UTC, Simon van der Linden
committed Details | Review
Remove useless checks. (1.60 KB, patch)
2010-08-12 09:34 UTC, Simon van der Linden
committed Details | Review

Description Simon Pena 2010-08-11 21:42:43 UTC
Created attachment 167670 [details]
Testcase showing the explained behavior

Invoking a method with a GValue annotated as "out" produces a crash, if the
arguments provided aren't right. 

Invoking the method without arguments, or providing the wrong type or amount
produces a segmentation fault. Calling the method with the right arguments works
as expected.

The testcase provided lets you reproduce it: calling the Gtk.ListStore.get_value()
method with two wrong args.

The segmentation fault is produced at pygi-invoke.c:958 (_free_invocation_state), 
when the GValue is first unset and then freed: state->args is null.
Comment 1 Simon Pena 2010-08-11 21:47:22 UTC
Created attachment 167672 [details] [review]
Partial fix

This patch provides a partial fix for the bug, checking if the state arguments are null before trying to unset them. It works as expected: when calling the method with the wrong type/number of arguments it fails with a TypeError, and works when doing it right.

It's based on the assumption that it is fine if the state arguments are null, as a result of them being assigned in a lazy way or something like that.

However, when the arguments meet the method signature but the GValue didn't get stored (due to the method's logic), it segfaults. I'll attach another testcase to display this behaviour.
Comment 2 Simon Pena 2010-08-11 21:54:24 UTC
Created attachment 167673 [details]
Testcase showing the free crash when the GValue hasn't been set

When the method signature is met but, due to the application logic, the GValue doesn't get to store a value, a crash is produced when the GValue gets freed.

The testcase attached passes an invalid iterator to the model, which then refuses to store a value in the GValue.

This behaviour was already present, and hasn't been altered with the previous patch.
Comment 3 Simon van der Linden 2010-08-12 09:08:29 UTC
Review of attachment 167672 [details] [review]:

Indeed, state->args and state->args[i] can be NULL at that point, if _prepare_invocation_state returns before it allocated them.

::: gi/pygi-invoke.c
@@ +959,3 @@
+                        g_value_unset ( (GValue *) state->args[i]);
+                        g_free (state->args[i]);
+                    }

This can be checked earlier.
Comment 4 Simon van der Linden 2010-08-12 09:17:30 UTC
Created attachment 167706 [details] [review]
Remove useless checks.

No need to check for state->arg_infos, state->arg_type_infos, and
state->args_is_auxiliary to be NULL, they are always allocated.
Comment 5 Simon van der Linden 2010-08-12 09:21:37 UTC
Created attachment 167708 [details] [review]
Fix caller-allocates emergency free.

In the state, args, args[i], arg_infos[i], and arg_type_infos[i] must not be
NULL in order to be able caller-allocates. This patch adds those conditions.

Moreover, the interface info needs to be freed afterwards.
Comment 6 Simon van der Linden 2010-08-12 09:34:27 UTC
The following fixes have been pushed:
e4c4ccc Fix caller-allocates emergency free.
0ab967c Remove useless checks.
Comment 7 Simon van der Linden 2010-08-12 09:34:31 UTC
Created attachment 167710 [details] [review]
Fix caller-allocates emergency free.

In the state, args, args[i], arg_infos[i], and arg_type_infos[i] must not be
NULL in order to be able caller-allocates. This patch adds those conditions.

Moreover, the interface info needs to be freed afterwards.
Comment 8 Simon van der Linden 2010-08-12 09:34:35 UTC
Created attachment 167711 [details] [review]
Remove useless checks.

No need to check for state->arg_infos, state->arg_type_infos, and
state->args_is_auxiliary to be NULL, they are always allocated.
Comment 9 Simon van der Linden 2010-08-12 10:04:49 UTC
For the other test case, see bug 620912.
Comment 10 Philip Withnall 2010-09-23 22:53:21 UTC
*** Bug 625583 has been marked as a duplicate of this bug. ***