Shouldn't System.Process.runInteractiveCommand close inherited descriptors above 2?

Hello! My coworker just experienced a strange situation: a network connection wasn't automatically closed when the process finished. It was because a child process unneccesarily inherited this connection's descriptor. So the question is: shouldn't runInteractiveCommand / runInteractiveProcess close descriptors greater then 2 in the child process? The following program shows this behaviour on Linux: import System.Process import System.IO main = do h1 <- openFile "/etc/passwd" ReadMode (_, pout, _, phandle) <- runInteractiveCommand "ls -l /proc/self/fd" hGetContents pout >>= putStr waitForProcess phandle hClose h1 The result: tomekz@tomekz:~/devel/haskell/interactive-process-filedes$ ./IP total 5 lr-x------ 1 tomekz tomekz 64 2007-06-04 11:50 0 -> pipe:[22830] l-wx------ 1 tomekz tomekz 64 2007-06-04 11:50 1 -> pipe:[22831] l-wx------ 1 tomekz tomekz 64 2007-06-04 11:50 2 -> pipe:[22832] lr-x------ 1 tomekz tomekz 64 2007-06-04 11:50 3 -> /etc/passwd lr-x------ 1 tomekz tomekz 64 2007-06-04 11:50 4 -> /proc/13687/fd As you can see, the "ls" process inherits the file descriptor to /etc/passwd from the Haskell program process. This is in GHC 6.6. I wonder if anyone explicitly wants such behavior, but even if so, IMHO the default behaviour should be to close such descriptors. I think the current situation could even cause some security problems. Best regards Tomek

On 6/4/07, Tomasz Zielonka
So the question is: shouldn't runInteractiveCommand / runInteractiveProcess close descriptors greater then 2 in the child process?
This is actually a pretty subtle area of POSIX (and probably something that was badly designed in the first place). Inheriting of fds is often surprising, but if the runtime were to close everything > 2, then how would you be able to pass one to a child process? There are many instances where you want to do this[1]. Maybe there needs to be an additional argument which contains fd numbers which should be passed down? [1] http://cr.yp.to/ucspi-tcp/tcpclient.html -- Adam Langley agl@imperialviolet.org http://www.imperialviolet.org 650-283-9641

On Mon, Jun 04, 2007 at 09:17:44AM -0700, Adam Langley wrote:
On 6/4/07, Tomasz Zielonka
wrote: So the question is: shouldn't runInteractiveCommand / runInteractiveProcess close descriptors greater then 2 in the child process?
This is actually a pretty subtle area of POSIX (and probably something that was badly designed in the first place). Inheriting of fds is often surprising, but if the runtime were to close everything > 2, then how would you be able to pass one to a child process? There are many instances where you want to do this[1].
I agree there are some such instances, but IMO they constitute only a small part of all uses in code. In most cases when you use these functions, you don't want such behavior. If you are lucky, it makes no harm and you simply don't care. I wonder if there is any Haskell program using this possibility... (please come forward :-)) If you use those functions and you want the child process to inherit some additional fds, you need to control file descriptors (know them, change them) - this may force you to abandon or break Haskell's Handle abstraction. Well, at least you have those functions in System.Posix: fdToHandle :: Fd -> IO GHC.IOBase.Handle handleToFd :: GHC.IOBase.Handle -> IO Fd dupTo :: Fd -> Fd -> IO Fd dup :: Fd -> IO Fd
Maybe there needs to be an additional argument which contains fd numbers which should be passed down?
I forgot to propose this explicitly, but this is the spirit of my suggestion for closing by default. OTOH, adding new parameters to library functions is not without cost. This could also be a new function, perhaps armed with options for many such subtleties - like starting a new process group, etc. It could be a list of options, eg. runInteractiveProcessWithOpts ... [CloseOtherFiles, SetSessionID]
BTW, I've used tcpclient :-) Best regards Tomek

-----Original Message----- From: libraries-bounces@haskell.org [mailto:libraries-bounces@haskell.org] On Behalf Of Tomasz Zielonka Sent: Monday, June 04, 2007 2:12 PM To: Adam Langley Cc: libraries@haskell.org Subject: Re: Shouldn't System.Process.runInteractiveCommand close inherited descriptors above 2? On Mon, Jun 04, 2007 at 09:17:44AM -0700, Adam Langley wrote:
On 6/4/07, Tomasz Zielonka
wrote: So the question is: shouldn't runInteractiveCommand / runInteractiveProcess close descriptors greater then 2 in the child process?
This is actually a pretty subtle area of POSIX (and probably something that was badly designed in the first place). Inheriting of fds is often surprising, but if the runtime were to close everything > 2, then how would you be able to pass one to a child process? There are many instances where you want to do this[1].
I agree there are some such instances, but IMO they constitute only a small part of all uses in code. In most cases when you use these functions, you don't want such behavior. If you are lucky, it makes no harm and you simply don't care. I wonder if there is any Haskell program using this possibility... (please come forward :-)) I do rely on this behavior, and I use it in applications that are commercially deployed, so I would argue strongly against changing the default behavior w.r.t. this issue. A new function which allows the parent to control which handles are available to the child is an excellent idea. I would be in favor of an argument (to runInteractiveCommandExtended, or whatever) which is a list of handles that the child process can use. The function can then close handles that are not part of the list. I guess you would need to use (handleToFd handle) to map the handles in the list to the integer file descriptors. This is certainly esthetically unpleasing, but as far as I can see it would work. Seth Kurtzberg Software Engineer Specializing in Security, Reliability, and the Hardware/Software Interface If you use those functions and you want the child process to inherit some additional fds, you need to control file descriptors (know them, change them) - this may force you to abandon or break Haskell's Handle abstraction. Well, at least you have those functions in System.Posix: fdToHandle :: Fd -> IO GHC.IOBase.Handle handleToFd :: GHC.IOBase.Handle -> IO Fd dupTo :: Fd -> Fd -> IO Fd dup :: Fd -> IO Fd
Maybe there needs to be an additional argument which contains fd numbers which should be passed down?
I forgot to propose this explicitly, but this is the spirit of my suggestion for closing by default. OTOH, adding new parameters to library functions is not without cost. This could also be a new function, perhaps armed with options for many such subtleties - like starting a new process group, etc. It could be a list of options, eg. runInteractiveProcessWithOpts ... [CloseOtherFiles, SetSessionID]
BTW, I've used tcpclient :-) Best regards Tomek _______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

Tomasz Zielonka wrote:
So the question is: shouldn't runInteractiveCommand / runInteractiveProcess close descriptors greater then 2 in the child process?
No. What your colleague experienced is normal Unix behaviour with about 30 years of history under its belt. Having Haskell do something different would be the surprise here. You can mark file descriptors that you don't want shared with fcntl's F_SETFD command, and FD_CLOEXEC: noExec :: Handle -> IO () noExec h = do fd <- handleToFd h setFdOption fd CloseOnExec True

On Mon, Jun 04, 2007 at 11:03:30AM -0700, Bryan O'Sullivan wrote:
Tomasz Zielonka wrote:
So the question is: shouldn't runInteractiveCommand / runInteractiveProcess close descriptors greater then 2 in the child process?
No. What your colleague experienced is normal Unix behaviour with about 30 years of history under its belt. Having Haskell do something different would be the surprise here.
OK, so it's not a good idea to close fds by default. But if I want to? Sometimes there are good reasons to do so.
You can mark file descriptors that you don't want shared with fcntl's F_SETFD command, and FD_CLOEXEC:
noExec :: Handle -> IO () noExec h = do fd <- handleToFd h setFdOption fd CloseOnExec True
Interesting, I didn't know about such option. It might help in this situation, but in general it would be unnatural. It seems more natural to specify which descriptors you want passed through a particular exec - because you should know that (I can imagine some counter-examples, but I'm not sure they would be a good idea anyway). Additionaly, in a complex application you may want a particular descriptor to be passed through one exec(), and not passed through another. The other solution to our problem is to create a wrapper program that will close all unneccesary descriptors and then execute the actual program. I think that's what we are going to do. So my initial question changes to: would it be useful to have an extended version of runInteractiveProcess with options for using non-standard behaviour and things like setsid(2)? Did anyone other than us wanted such functionality? I guess System.Process wouldn't be a good place for such function, because this module is meant to be portable across different OSes? Best regards Tomek

Tomasz Zielonka wrote:
Additionaly, in a complex application you may want a particular descriptor to be passed through one exec(), and not passed through another.
Well, you have all the building blocks you need to write a function that will do just that. It shouldn't take more than a few lines of code.
So my initial question changes to: would it be useful to have an extended version of runInteractiveProcess with options for using non-standard behaviour and things like setsid(2)? Did anyone other than us wanted such functionality?
If you want to manipulate sessions and stuff like that, you're deep in application-specific territory, not to mention POSIX specificity. The libraries already provide the basic pieces that you need; I don't see much value in cooking some particular combination of them into a "blessed" library function.

On Mon, Jun 04, 2007 at 01:08:11PM -0700, Bryan O'Sullivan wrote:
Tomasz Zielonka wrote:
Additionaly, in a complex application you may want a particular descriptor to be passed through one exec(), and not passed through another.
Well, you have all the building blocks you need to write a function that will do just that. It shouldn't take more than a few lines of code.
You mean using System.Posix? Writing our version of runInteractiveProcess in the same way as the official one would mean writing much more than a few lines of code. The official implementation does much of its work through FFI. I wonder why it doesn't use System.Posix more... Did it lack efficiency or functionality? Anyway, we will rather use an external wrapper program - that should be enough for us.
So my initial question changes to: would it be useful to have an extended version of runInteractiveProcess with options for using non-standard behaviour and things like setsid(2)? Did anyone other than us wanted such functionality?
If you want to manipulate sessions and stuff like that, you're deep in application-specific territory, not to mention POSIX specificity. The libraries already provide the basic pieces that you need; I don't see much value in cooking some particular combination of them into a "blessed" library function.
I guess you are right. But now I wonder if it would be possible to make it easier to create such special versions of those functions. Building them from the primitives probably doesn't take much code, but certainly requires care and knowledge of many details. Maybe some kind of a combinator library could help here, but this is only a vague idea right now. Best regards Tomek

Tomasz Zielonka wrote:
On Mon, Jun 04, 2007 at 01:08:11PM -0700, Bryan O'Sullivan wrote:
Tomasz Zielonka wrote:
Additionaly, in a complex application you may want a particular descriptor to be passed through one exec(), and not passed through another. Well, you have all the building blocks you need to write a function that will do just that. It shouldn't take more than a few lines of code.
You mean using System.Posix?
Writing our version of runInteractiveProcess in the same way as the official one would mean writing much more than a few lines of code. The official implementation does much of its work through FFI. I wonder why it doesn't use System.Posix more... Did it lack efficiency or functionality?
It really shoud use System.Posix (and System.Win32). The reason it doesn't is because it was in the base package and therefore couldn't depend on the unix package, but now System.Process is in a package of its own we can now use the unix/Win32 packages and clean up the implementation. Cheers, Simon

"Bryan O'Sullivan"
Tomasz Zielonka wrote:
So the question is: shouldn't runInteractiveCommand / runInteractiveProcess close descriptors greater then 2 in the child process?
No. What your colleague experienced is normal Unix behaviour with about 30 years of history under its belt. Having Haskell do something different would be the surprise here.
I like the way Scsh handles this. On exec, it closes file descriptors for ports (Scheme's Handles) that haven't had their file descriptor "revealed" (i.e., been the argument to handleToFd or the return value from fdToHandle) . See http://www.scsh.net/docu/html/man-Z-H-4.html#node_sec_3.2.4 for details. If the program doesn't know what file descriptor corresponds to a handle, it's hard to see what good could come from keeping the descriptor open across an exec(). (I suppose you could exec() then find out and communicate the descriptor afterward--but I doubt that's important.) (Scsh also uses the revealed file descriptor information to support shifting around the unrevealed descriptors of ports when that descriptor is the target of a dupTo.) -m

On Mon, Jun 04, 2007 at 02:03:34PM -0700, Mike Gunter wrote:
I like the way Scsh handles this. On exec, it closes file descriptors for ports (Scheme's Handles) that haven't had their file descriptor "revealed" (i.e., been the argument to handleToFd or the return value from fdToHandle).
Interesting idea! One more reason to finally learn scsh. Best regards Tomek
participants (6)
-
Adam Langley
-
Bryan O'Sullivan
-
Mike Gunter
-
Seth Kurtzberg
-
Simon Marlow
-
Tomasz Zielonka