Sending SIGHUP twice to `swhkd` crashes with `Failed to set init groups: EPERM`

[ajanon]

Version Information:

• Linux arch-dktp-alexis 5.19.5-arch1-1 #1 SMP PREEMPT_DYNAMIC Mon, 29 Aug 2022 15:51:05 +0000 x86_64 GNU/Linux
• swhkd 1.2.1 (installed from the swhkd-git AUR package)

Describe the bug: Sending SIGHUP twice in a row crashes swhkd. It then outputs: Failed to set init groups: EPERM

Expected behavior: Sending SIGHUP twice (i.e., reloading swhkd's config) should not cause the daemon to exit.

Actual behavior: The first SIGHUP sent reloads the config as expected, the second one crashes the daemon.

To Reproduce: Start swhkd. Send SIGHUP (sudo pkill -HUP swhkd), wait a bit, send SIGHUP again. I have tried to wait for as long as 1 minute (just in case reloading the config lasted some time).

Additional information: From what I can tell, this error is logged in the set_initgroups function: https://github.com/waycrate/swhkd/blob/d21e613d83a6049f3856a50d5f8eff8bcc29e91f/swhkd/src/perms.rs#L21-L30

I am not sure if this error happens in the call from the drop_privileges function or the raise_privileges function.

swhkd and swhks are usually started from my xinitrc:

swhks &
pkexec swhkd --config "$XDG_CONFIG_HOME/swhkd/swhkdrc" --device "USB-compliant keyboard" &

I have also tried to start swhkd from the tty, with a dummy config file and without specifying a device (just in case), but to no avail.

[Shinyzenith]

Hi, thank you so much for putting in the effort. This bug has been known for a while now ever since we developed the drop_priv function to patch a CVE.

I haven't gotten around to patching this yet but contributions are welcome! If you'd like more info on what's going wrong here, I'm more than happy to explain!

[ajanon]

Thank you for your quick answer!

I have taken a look at it, and I think I understand what is happening: the first SIGHUP drops privileges, but swhkd never raises them again afterwards. From my understanding and my testing, raising back privileges right after reading the config file is enough to fix the issue:

        match config::load(&config_file_path) {
            Err(e) => {
                log::error!("Config Error: {}", e);
                exit(1)
            }
            Ok(out) => {
                // Escalate back to the root user after reading the config file.
                perms::raise_privileges();
                out
            }
        }

From: https://github.com/waycrate/swhkd/blob/d21e613d83a6049f3856a50d5f8eff8bcc29e91f/swhkd/src/daemon.rs#L84-L90

Is there something I am missing here where this would be undesirable, especially regarding the CVE?

This passes tests run from make check on my end and also solves the issue. If this is fine with you, I can write a PR!

[Shinyzenith]

Awesome! Please go ahead and send a PR! I didn't think this approach would work after dropping our privileges but oh well!

Here's the CVE if you're interested: https://www.cvedetails.com/cve/CVE-2022-27814/