GNOME Bugzilla – Bug 686015
vinagre crashes when connecting
Last modified: 2012-10-15 12:30:53 UTC
libsecret from git, vinagre 3.6.0 Program received signal SIGSEGV, Segmentation fault. 0x00007ffff5fe444e in fast_validate (str=0x0) at gutf8.c:1461 1461 for (p = str; *p; p++) (gdb) bt
+ Trace 231021
463 g_return_val_if_fail (schema != NULL, NULL); 464 g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), NULL); 465 g_return_val_if_fail (error == NULL || *error == NULL, NULL); 466 467 va_start (va, error); 468 attributes = secret_attributes_buildv (schema, va); 469 va_end (va); 470 471 password = secret_password_lookupv_sync (schema, attributes, 472 cancellable, error); (gdb) p error $2 = (GError **) 0x0 (gdb) p schema $3 = (const SecretSchema *) 0x7ffff7dcd9e0 <network_schema>
It looks like vinagre is passing a NULL pointer as a value to secret_password_lookup_sync(). Currently this is undefined. I thought about this a bit recently.... Perhaps we should define this, as 'don't include the attribute in the search when vararg value is NULL'. That makes it much easier to do searches using varargs without first checking all the values, and using multiple vararg calls when one or more are NULL.
Created attachment 226317 [details] [review] Better critical preconditions for invalid attributes * When passing NULL for a string vararg attribute print a critical warning. * Make all attribute warnings critical * Add tests for the various critical warnings Turns out the above logic about using NULL doesn't really work API wise. It's inconsistent with the integer and boolean attribute varargs. In reality the caller should just check for NULL values, and substitute something appropriate, or not include it in the search. The above patch cleans up the behavior for libsecret. Does the patch produce the expected warning for you Bastien? That way we'll know if I've diagnosed this correctly.
A little bit too warny. One of those is a proper warning as well (the GHashTable one). ** (vinagre:20898): CRITICAL **: The value for attribute 'user' was NULL Program received signal SIGTRAP, Trace/breakpoint trap. 0x00007ffff5fb78e0 in g_logv (log_domain=0x0, log_level=G_LOG_LEVEL_CRITICAL, format=0x7ffff7bbcc00 "The value for attribute '%s' was NULL", args=0x7fffffffc518) at gmessages.c:967 967 G_BREAKPOINT (); (gdb) bt
+ Trace 231025
Created attachment 226340 [details] [review] Better critical preconditions for invalid attributes * When passing NULL for a string vararg attribute print a critical warning. * Make all attribute warnings critical * Add tests for the various critical warnings Bastien, how does this look?
Looks good: ** (vinagre:14173): CRITICAL **: The value for attribute 'user' was NULL
Attachment 226340 [details] pushed as 7e02a59 - Better critical preconditions for invalid attributes
*** Bug 685394 has been marked as a duplicate of this bug. ***