-
Notifications
You must be signed in to change notification settings - Fork 3k
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.
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
Close stdout/stderr/stdin fds in auth-plugin fork #84
Open
gollub
wants to merge
1
commit into
OpenVPN:master
Choose a base branch
from
gollub:fix/pam_fd_leak
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The auth-pam plugin gets forked to run in background and kept stdin/stdout/stderr open. This might block the callee of the OpenVPN which expect that it nicely turns into a daemon. Signed-off-by: Daniel Gollub <[email protected]>
Any progress on this? |
Hi,
On Fri, Jun 07, 2019 at 06:25:22AM -0700, kawakami wrote:
Any progress on this?
Please send patches to the [email protected] mailing
list. Our current process requires the patch to be in the list archives
and ACKed on the list.
PRs are - as the intro document says - not something we work with
(but you cannot turn them off on github).
The patch description itself sounds like something we want to fix :-)
gert
…--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany [email protected]
|
Before considering this patch we should understand what the issue is. I am not sure how closing stdout/err could help here. Any extra detail @gollub ? (Sorry for the latency, but we really not pay attention to GH, as development happens on the ml) |
@Jnchi do you know why this patch was important and what it was fixing? |
Hi,
On Fri, Mar 03, 2017 at 01:44:12AM -0800, Daniel Gollub wrote:
The auth-pam plugin gets forked to run in background and kept
stdin/stdout/stderr open. This might block the callee of the OpenVPN
which expect that it nicely turns into a daemon.
Not sure I am following - in which scenario is this relevant? If
I run openvpn with "--daemon" (which would be the expected scenario),
OpenVPN takes care of that before forking plugins.
This is what I see if I do "lsof -p $pluginprocess" on one of my
instances using plugin-auth-pam
$ ps axwu |grep pam
root 16081 0.0 0.1 7744 5468 ? Ss 12:25 0:00 ./bin/openvpn --daemon tun-udp-p2mp-global-authpam --cd tun-udp-p2mp-global-authpam --config server.conf --writepid ../openvpn-tun-udp-p2mp-global-authpam.pid
root 16083 0.0 0.0 7612 656 ? S 12:25 0:00 ./bin/openvpn --daemon tun-udp-p2mp-global-authpam --cd tun-udp-p2mp-global-authpam --config server.conf --writepid ../openvpn-tun-udp-p2mp-global-authpam.pid
gert 16102 0.0 0.0 7764 2320 pts/0 S+ 12:25 0:00 grep --colour=auto pam
$ SU lsof -p 16083
...
openvpn 16083 root 0u CHR 1,3 0t0 5780 /dev/null
openvpn 16083 root 1u CHR 1,3 0t0 5780 /dev/null
openvpn 16083 root 2u CHR 1,3 0t0 5780 /dev/null
So - this is how it should be.
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany ***@***.***
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The auth-pam plugin gets forked to run in background and kept
stdin/stdout/stderr open. This might block the callee of the OpenVPN
which expect that it nicely turns into a daemon.