Skip to content

Conversation

@robhoes
Copy link

@robhoes robhoes commented May 20, 2016

The two calls to Unix.dup are intended to point stdout and stderr at
/dev/null - this will normally be the case, as Unix.dup will open the
next available file descriptor and stdout/stderr have just been closed.

However, there are many ways this can go wrong - if an open() happens
in another thread it could mean that...

  • nullfd <> Unix.stdin - this is checked for, but there is no error
    handling if this fails
  • one or both of the Unix.dup calls do not point stdout/stderr at
    /dev/null as intended, but instead open new file descriptors.

This change instead uses Unix.dup2 which atomically closes the second
file descriptor and copies it from the first. This also allows the
explicit calls to Unix.close to be removed. Finally, nullfd is closed as
it is not needed after the calls to Unix.dup2.

Signed-off-by: John Else [email protected]

The two calls to Unix.dup are intended to point stdout and stderr at
/dev/null - this will normally be the case, as Unix.dup will open the
next available file descriptor and stdout/stderr have just been closed.

However, there are many ways this can go wrong - if an open() happens
in another thread it could mean that...

* nullfd <> Unix.stdin - this is checked for, but there is no error
  handling if this fails
* one or both of the Unix.dup calls do not point stdout/stderr at
  /dev/null as intended, but instead open new file descriptors.

This change instead uses Unix.dup2 which atomically closes the second
file descriptor and copies it from the first. This also allows the
explicit calls to Unix.close to be removed. Finally, nullfd is closed as
it is not needed after the calls to Unix.dup2.

Signed-off-by: John Else <[email protected]>
@robhoes robhoes merged commit 3cce388 into xenserver:dundee-bugfix May 20, 2016
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