Nothing Special   »   [go: up one dir, main page]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Talos leaks file descriptors to child processes #9412

Open
3 tasks
smira opened this issue Oct 1, 2024 · 4 comments
Open
3 tasks

Talos leaks file descriptors to child processes #9412

smira opened this issue Oct 1, 2024 · 4 comments
Assignees
@smira

Comments

@smira
Copy link
Member
smira commented Oct 1, 2024 •
edited
Loading

tl;dr is missing O_CLOEXEC in some libraries we're using

Tasks
Beta Give feedback
  1. fix: add CLOEXEC flag to socket created directly insomniacslk/dhcp#550
  2. fix: add SOCK_CLOEXEC to socket creation safchain/ethtool#88
  3. fix: add missing CLOEXEC flag vishvananda/netlink#1023
@smira smira self-assigned this Oct 1, 2024
smira added a commit to smira/talos that referenced this issue Oct 1, 2024
@smira
fix: prevent file descriptors leaks to child processes
ed6d080 
See siderolabs#9412

I'll keep the issue open to track upstream PR status and remove replace
directives.

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
@smira smira mentioned this issue Oct 1, 2024
Merged
smira added a commit to smira/talos that referenced this issue Oct 1, 2024
@smira
fix: prevent file descriptors leaks to child processes
a341bdb 
See siderolabs#9412

I'll keep the issue open to track upstream PR status and remove replace
directives.

Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
@dsseng
Copy link
Member
dsseng commented Oct 1, 2024

Should we maybe consider adding a rule to golangci-lint to check for syscalls without cloexec to avoid problematic code being merged later?

@smira
Copy link
Member Author
smira commented Oct 1, 2024

Should we maybe consider adding a rule to golangci-lint to check for syscalls without cloexec to avoid problematic code being merged later?

I would rather prefer a test, but not sure how to put #9414 into a useful thing, as we need to fork directly from machined to verify this. System extensions don't work, I don't want to put test binary into any Talos build. I could use some debug feature as well to enable this test controller

@dsseng
Copy link
Member
dsseng commented Oct 1, 2024

Will take a look a bit later, maybe I have some ideas about this

@smira
Copy link
Member Author
smira commented Oct 3, 2024

Update: the problem itself is fixed, but the issue is left open to track upstream PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@smira smira

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@smira @dsseng