chore!: drop daemonize crate#372
Conversation
| return Err(anyhow!("redirecting stdin to /dev/null: {}", nix::errno::Errno::last())); | ||
| } | ||
| // Safety: nullfd is valid and newfd need not be open for saftey | ||
| let fd = unsafe { libc::dup2(nullfd, libc::STDOUT_FILENO) }; |
There was a problem hiding this comment.
should you be using nix's dup2 instead? https://docs.rs/nix/latest/nix/unistd/fn.dup2.html
There was a problem hiding this comment.
I gave it a try, but there is no way to make a OwnedFd for the std file descriptors because they are shared globals. This made me realize I need to mark this function unsafe and document the precondution that the std file descriptors can't be under contention when it is called though (we were already meeting this precondition but the function needs to document it's preconditions in order to be sound).
|
|
||
| fn redirect_std_fds_to_null() -> anyhow::Result<()> { | ||
| // Safety: path is a valid null terminated string, O_RDWR is a valid flag. | ||
| let nullfd = unsafe { libc::open(b"/dev/null\0" as *const [u8; 10] as _, libc::O_RDWR) }; |
There was a problem hiding this comment.
Do we need to close nullfd after we finish duping?
There was a problem hiding this comment.
Good point, fixed with fcntl::open
|
|
||
| fn redirect_std_fds_to_null() -> anyhow::Result<()> { | ||
| // Safety: path is a valid null terminated string, O_RDWR is a valid flag. | ||
| let nullfd = unsafe { libc::open(b"/dev/null\0" as *const [u8; 10] as _, libc::O_RDWR) }; |
There was a problem hiding this comment.
Good call, that takes care of cleaning up the fd as well.
ethanpailes
force-pushed
the
drop-daemonize-crate
branch
from
e85915d to
630bc34
Compare
This patch drops teh daemonize crate in favor of our own daemonization implementation. The daemonize crate is apparently unmaintained, so we need to migrate off of it for security. In writing this patch, I realized that libshpool::run was technically unsound (though in practice, no reasonable caller would trigger unsoundness). Unfortunately, to fix it properly I needed to mark the entrypoint `unsafe` and document the required preconditions (can't call from a multi-threaded program in a situation where it might be self-execed to auto-daemonize). Unfortunately this requires a breaking version upgrade.
ethanpailes
force-pushed
the
drop-daemonize-crate
branch
from
630bc34 to
70f5c89
Compare
2 participants
Issue Link
The nightly
cargo denytests have been failing for a while. This is me attempting to clean them up.AI Policy Ack
Ack
This PR was:
I vibed the tests
Description
This patch drops teh daemonize crate in favor of our own daemonization implementation. The daemonize crate is apparently unmaintained, so we need to migrate off of it for security.
In writing this patch, I realized that libshpool::run was technically unsound (though in practice, no reasonable caller would trigger unsoundness). Unfortunately, to fix it properly I needed to mark the entrypoint
unsafeand document the required preconditions (can't call from a multi-threaded program in a situation where it might be self-execed to auto-daemonize). Unfortunately this requires a breaking version upgrade.