Skip to content

Conversation

@joanbm
Copy link
Contributor

@joanbm joanbm commented May 4, 2025

To create a hotspot, the UI launches create_ap with popen and waits for it to print "AP-ENABLED" to confirm the hotspot has been created. When this happens, it immediately returns, without calling pclose.

This not only leaks the FILE * opened by popen, but it can also inadvertently cause create_ap to get terminated:

  • The user launches wihotspot-gui as root
  • wihotspot-gui launches pkexec --user root create_ap ...
  • pkexec sets a SIGTERM parent-death signal via prctl
  • pkexec launches create_ap and it starts running
  • create_ap creates the hotspot and prints "AP-ENABLED"
  • wihotspot-gui updates the UI, and returns without waiting for create_ap to exit
  • Immediately after, the init_running_info thread exits
  • A SIGTERM parent-death signal is sent to create_ap
  • create_ap terminates and the hotspot stops running

(This only reproduces if wihotspot-gui runs as root, likely due to setuid restrictions for prctl applying otherwise)

Wait for create_ap to exit using pclose to fix the problem.

joanbm and others added 2 commits May 4, 2025 00:05
To create a hotspot, the UI launches `create_ap` with `popen` and waits
for it to print "AP-ENABLED" to confirm the hotspot has been created.
When this happens, it immediately returns, without calling `pclose`.

This not only leaks the `FILE *` opened by `popen`, but it can also
inadvertently cause `create_ap` to get terminated:
* The user launches `wihotspot-gui` as root
* `wihotspot-gui` launches `pkexec --user root create_ap ...`
* `pkexec` sets a `SIGTERM` parent-death signal via `prctl`
  (https://github.com/polkit-org/polkit/blob/7d44d62a02f090a5a71c4ca10d58c7c2b54881eb/src/programs/pkexec.c#L757)
* `pkexec` launches `create_ap` and it starts running
* `create_ap` creates the hotspot and prints "AP-ENABLED"
* `wihotspot-gui` updates the UI, and returns without waiting for
  `create_ap` to exit
* Immediately after, the `init_running_info` thread exits
* A `SIGTERM` parent-death signal is sent to `create_ap`
* `create_ap` terminates and the hotspot stops running

(This only reproduces if `wihotspot-gui` runs as root,
 likely due to setuid restrictions for `prctl` applying otherwise)

Wait for `create_ap` to exit using `pclose` to fix the problem.
@lakinduakash lakinduakash merged commit 34fd80f into lakinduakash:master Jul 15, 2025
1 of 2 checks passed
@lakinduakash lakinduakash requested a review from Copilot July 15, 2025 07:02
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR ensures the UI waits for the create_ap process to finish after detecting "AP-ENABLED", preventing file-pointer leaks and unintended process termination.

  • Add a pclose(fp) call immediately after detecting "AP-ENABLED"
  • Ensures the popen'd create_ap process is properly reaped
Comments suppressed due to low confidence (1)

src/ui/ui.c:777

  • To ensure the FILE * is always closed in every exit path, you might move pclose(fp) into a common cleanup section or after the loop, rather than only before this early return.
            return 0;


if (strstr(buf, AP_ENABLED) != NULL) {
init_running_info(NULL);
pclose(fp);
Copy link

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider checking the return value of pclose(fp) to detect and handle potential errors when closing the pipe to the create_ap process.

Suggested change
pclose(fp);
int pclose_status = pclose(fp);
if (pclose_status != 0) {
printf("Error: pclose() failed with status %d\n", pclose_status);
init_running_info(NULL);
return NULL;
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants