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
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.
Created attachment 105867 [details] [review] 0001-Duplicate-the-action_id-before-inserting-in-the-hash.patch This patch seems to do the trick.
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.
Any news about this? You want me to propose a new patch?
Ping?
Sorry for not replying earlier; you are correct in the diagnosis, and a similar patch (with a little more cleanup) will be committed soon.
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.