Bug 83590 - ** WARNING **: Unknown action_id '<random string>'
Summary: ** WARNING **: Unknown action_id '<random string>'
Status: RESOLVED FIXED
Alias: None
Product: PolicyKit
Classification: Unclassified
Component: daemon (show other bugs)
Version: unspecified
Hardware: Other All
: medium critical
Assignee: David Zeuthen (not reading bugmail)
QA Contact: David Zeuthen (not reading bugmail)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-07 17:39 UTC by Laurent Bigonville
Modified: 2015-07-02 19:40 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Valgrind log (297.92 KB, text/plain)
2014-09-07 17:39 UTC, Laurent Bigonville
Details
0001-Duplicate-the-action_id-before-inserting-in-the-hash.patch (1.86 KB, patch)
2014-09-07 19:36 UTC, Laurent Bigonville
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Laurent Bigonville 2014-09-07 17:39:55 UTC
Created attachment 105862 [details]
Valgrind log

Hello,

With polkit 0.112, I'm getting a lot of these errors when starting the daemon:

** (polkitd:21926): WARNING **: Unknown action_id '<random string>'

The action_id is almost always different.

When running polkitd in valgrind, I'm getting a lot of these "invalid read":

==21968== Invalid read of size 1
==21968==    at 0x585997D: g_str_hash (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
==21968==    by 0x5858E97: g_hash_table_lookup (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
==21968==    by 0x416E4C: polkit_backend_action_pool_get_action (polkitbackendactionpool.c:388)
==21968==    by 0x41703B: polkit_backend_action_pool_get_all_actions (polkitbackendactionpool.c:452)
==21968==    by 0x4119D4: check_authorization_sync (polkitbackendinteractiveauthority.c:1171)
==21968==    by 0x411341: polkit_backend_interactive_authority_check_authorization (polkitbackendinteractiveauthority.c:952)
==21968==    by 0x40963F: polkit_backend_authority_check_authorization (polkitbackendauthority.c:227)
==21968==    by 0x40A208: server_handle_check_authorization (polkitbackendauthority.c:787)
==21968==    by 0x40AE50: server_handle_method_call (polkitbackendauthority.c:1214)
==21968==    by 0x5321BFB: call_in_idle_cb (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4000.0)
==21968==    by 0x5869DDC: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
==21968==    by 0x586A1B7: g_main_context_iterate.isra.29 (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
==21968==    by 0x586A4E1: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
==21968==    by 0x409209: main (polkitd.c:236)
==21968==  Address 0xcf24a91 is 1 bytes inside a block of size 38 free'd
==21968==    at 0x4C29730: free (vg_replace_malloc.c:468)
==21968==    by 0x416586: parsed_action_free (polkitbackendactionpool.c:69)
==21968==    by 0x58587E7: g_hash_table_insert_node (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
==21968==    by 0x58589F9: g_hash_table_insert_internal (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
==21968==    by 0x418304: _end (polkitbackendactionpool.c:1040)
==21968==    by 0x5B35CEF: ??? (in /lib/x86_64-linux-gnu/libexpat.so.1.6.0)
==21968==    by 0x5B3664D: ??? (in /lib/x86_64-linux-gnu/libexpat.so.1.6.0)
==21968==    by 0x5B349E0: ??? (in /lib/x86_64-linux-gnu/libexpat.so.1.6.0)
==21968==    by 0x5B3516C: ??? (in /lib/x86_64-linux-gnu/libexpat.so.1.6.0)
==21968==    by 0x5B385DE: XML_ParseBuffer (in /lib/x86_64-linux-gnu/libexpat.so.1.6.0)
==21968==    by 0x4184C8: process_policy_file (polkitbackendactionpool.c:1097)
==21968==    by 0x41718A: ensure_file (polkitbackendactionpool.c:495)
==21968==    by 0x417326: ensure_all_files (polkitbackendactionpool.c:553)
==21968==    by 0x416E2D: polkit_backend_action_pool_get_action (polkitbackendactionpool.c:384)
==21968==    by 0x411631: check_authorization_sync (polkitbackendinteractiveauthority.c:1073)
==21968==    by 0x411341: polkit_backend_interactive_authority_check_authorization (polkitbackendinteractiveauthority.c:952)
==21968==    by 0x40963F: polkit_backend_authority_check_authorization (polkitbackendauthority.c:227)
==21968==    by 0x40A208: server_handle_check_authorization (polkitbackendauthority.c:787)
==21968==    by 0x40AE50: server_handle_method_call (polkitbackendauthority.c:1214)
==21968==    by 0x5321BFB: call_in_idle_cb (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4000.0)

The full log is attached to this bug
Comment 1 Laurent Bigonville 2014-09-07 19:01:33 UTC
OK I believe I know what's happening here.

FirewallD is for some reasons, shipping a symlink:

lrwxrwxrwx. 1 root root   42 aoû 21 15:26 org.fedoraproject.FirewallD1.policy -> org.fedoraproject.FirewallD1.server.policy
-rw-r--r--. 1 root root 4033 aoû 21 15:26 org.fedoraproject.FirewallD1.server.policy

The code is trying to insert twice the same action_id in the hash table and parsed_action_free() is called on the initial one and then something goes wrong, the documentation says that the value should be replace though:

If the key already exists in the GHashTable its current value is replaced with the new value. If you supplied a value_destroy_func when creating the GHashTable, the old value is freed using that function. If you supplied a key_destroy_func when creating the GHashTable, the passed key is freed using that function.
Comment 2 Laurent Bigonville 2014-09-07 19:36:37 UTC
Created attachment 105867 [details] [review]
0001-Duplicate-the-action_id-before-inserting-in-the-hash.patch

This patch seems to do the trick.
Comment 3 Christophe Fergeau 2014-09-08 07:35:58 UTC
I believe
diff --git a/src/polkitbackend/polkitbackendactionpool.c b/src/polkitbackend/pol
index bc14381..9712a1d 100644
--- a/src/polkitbackend/polkitbackendactionpool.c
+++ b/src/polkitbackend/polkitbackendactionpool.c
@@ -1003,7 +1003,7 @@ _end (void *data, const char *el)
         action->implicit_authorization_inactive = pd->implicit_authorization_in
         action->implicit_authorization_active = pd->implicit_authorization_acti
 
-        g_hash_table_insert (priv->parsed_actions, action->action_id, action);
+        g_hash_table_replace (priv->parsed_actions, action->action_id, action);
 
         /* we steal these hash tables */
         pd->annotations = NULL;

would do the trick too

g_hash_table_replace documentation states that « Inserts a new key and value into a GHashTable similar to g_hash_table_insert(). The difference is that if the key already exists in the GHashTable, it gets replaced by the new key. »

This is what is wanted here as the key gets freed when the value is replaced. An alternative would be to error out/ignore on already present action ids.
Comment 4 Laurent Bigonville 2014-09-20 12:10:58 UTC
Any news about this? You want me to propose a new patch?
Comment 5 Laurent Bigonville 2015-06-07 23:17:23 UTC
Ping?
Comment 6 Miloslav Trmac 2015-06-16 23:29:30 UTC
Sorry for not replying earlier; you are correct in the diagnosis, and a similar patch (with a little more cleanup) will be committed soon.
Comment 7 Miloslav Trmac 2015-07-02 19:40:52 UTC
Fixed in polkit-0.113. Thanks again for the patch!


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.