On 16/07/13 04:32, Dan McGee wrote:
> When calling open(), use O_CLOEXEC as much as possible to ensure the
> file descriptor is closed when and if a process using libalpm forks.
>
> For most of these cases, and especially in utility functions, the file
> descriptor is opened and closed in the same function, so we don't have
> too much to worry about. However, for things like the log file and
> database lock file, we should ensure descriptors aren't left hanging
> around for children to touch.
>
> This patch is inspired by the problem in FS#36161, where an open file
> descriptor to the current working directory prevents chroot() from
> working on FreeBSD. We don't need this file descriptor in the child
> process, so open it (and now several others) with O_CLOEXEC.
>
> Signed-off-by: Dan McGee <[email protected]>
> ---
Compile error:
CC libalpm_la-log.lo
In file included from /usr/include/fcntl.h:296:0,
from util.h:41,
from <command-line>:27:
In function 'open',
inlined from 'alpm_logaction' at <command-line>:52:3:
/usr/include/bits/fcntl2.h:50:24: error: call to '__open_missing_mode'
declared with attribute error: open with O_CREAT in second argument
needs 3 arguments
__open_missing_mode ();
^
> lib/libalpm/add.c | 2 +-
> lib/libalpm/handle.c | 2 +-
> lib/libalpm/log.c | 8 ++++++--
> lib/libalpm/util.c | 14 +++++++-------
> 4 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
> index 8ef9ef0..2d93077 100644
> --- a/lib/libalpm/add.c
> +++ b/lib/libalpm/add.c
> @@ -542,7 +542,7 @@ static int commit_single_pkg(alpm_handle_t *handle,
> alpm_pkg_t *newpkg,
> }
>
> /* save the cwd so we can restore it later */
> - OPEN(cwdfd, ".", O_RDONLY);
> + OPEN(cwdfd, ".", O_RDONLY | O_CLOEXEC);
> if(cwdfd < 0) {
> _alpm_log(handle, ALPM_LOG_ERROR, _("could not get
> current working directory\n"));
> }
> diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c
> index 53c86c5..274e3bc 100644
> --- a/lib/libalpm/handle.c
> +++ b/lib/libalpm/handle.c
> @@ -110,7 +110,7 @@ int _alpm_handle_lock(alpm_handle_t *handle)
> FREE(dir);
>
> do {
> - fd = open(handle->lockfile, O_WRONLY | O_CREAT | O_EXCL, 0000);
> + fd = open(handle->lockfile, O_WRONLY | O_CREAT | O_EXCL |
> O_BINARY | O_CLOEXEC, 0000);
> } while(fd == -1 && errno == EINTR);
> if(fd >= 0) {
> FILE *f = fdopen(fd, "w");
> diff --git a/lib/libalpm/log.c b/lib/libalpm/log.c
> index 271bd00..5f3b094 100644
> --- a/lib/libalpm/log.c
> +++ b/lib/libalpm/log.c
> @@ -49,9 +49,13 @@ int SYMEXPORT alpm_logaction(alpm_handle_t *handle, const
> char *prefix,
>
> /* check if the logstream is open already, opening it if needed */
> if(handle->logstream == NULL) {
> - handle->logstream = fopen(handle->logfile, "a");
> + int fd;
> + OPEN(fd, handle->logfile, O_WRONLY | O_APPEND | O_CREAT |
> O_CLOEXEC);
> + if(fd >= 0) {
> + handle->logstream = fdopen(fd, "a");
> + }
> /* if we couldn't open it, we have an issue */
> - if(handle->logstream == NULL) {
> + if(fd < 0 || handle->logstream == NULL) {
> if(errno == EACCES) {
> handle->pm_errno = ALPM_ERR_BADPERMS;
> } else if(errno == ENOENT) {
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index 1e21362..863614e 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -154,9 +154,9 @@ int _alpm_copyfile(const char *src, const char *dest)
>
> MALLOC(buf, (size_t)ALPM_BUFFER_SIZE, return 1);
>
> - OPEN(in, src, O_RDONLY);
> + OPEN(in, src, O_RDONLY | O_CLOEXEC);
> do {
> - out = open(dest, O_WRONLY | O_CREAT, 0000);
> + out = open(dest, O_WRONLY | O_CREAT | O_BINARY | O_CLOEXEC,
> 0000);
> } while(out == -1 && errno == EINTR);
> if(in < 0 || out < 0) {
> goto cleanup;
> @@ -245,7 +245,7 @@ int _alpm_open_archive(alpm_handle_t *handle, const char
> *path,
> archive_read_support_format_all(*archive);
>
> _alpm_log(handle, ALPM_LOG_DEBUG, "opening archive %s\n", path);
> - OPEN(fd, path, O_RDONLY);
> + OPEN(fd, path, O_RDONLY | O_CLOEXEC);
> if(fd < 0) {
> _alpm_log(handle, ALPM_LOG_ERROR,
> _("could not open file %s: %s\n"), path,
> strerror(errno));
> @@ -327,7 +327,7 @@ int _alpm_unpack(alpm_handle_t *handle, const char *path,
> const char *prefix,
> oldmask = umask(0022);
>
> /* save the cwd so we can restore it later */
> - OPEN(cwdfd, ".", O_RDONLY);
> + OPEN(cwdfd, ".", O_RDONLY | O_CLOEXEC);
> if(cwdfd < 0) {
> _alpm_log(handle, ALPM_LOG_ERROR, _("could not get current
> working directory\n"));
> }
> @@ -503,7 +503,7 @@ int _alpm_run_chroot(alpm_handle_t *handle, const char
> *cmd, char *const argv[])
> int retval = 0;
>
> /* save the cwd so we can restore it later */
> - OPEN(cwdfd, ".", O_RDONLY);
> + OPEN(cwdfd, ".", O_RDONLY | O_CLOEXEC);
> if(cwdfd < 0) {
> _alpm_log(handle, ALPM_LOG_ERROR, _("could not get current
> working directory\n"));
> }
> @@ -769,7 +769,7 @@ static int md5_file(const char *path, unsigned char
> output[16])
>
> MALLOC(buf, (size_t)ALPM_BUFFER_SIZE, return 1);
>
> - OPEN(fd, path, O_RDONLY);
> + OPEN(fd, path, O_RDONLY | O_CLOEXEC);
> if(fd < 0) {
> free(buf);
> return 1;
> @@ -811,7 +811,7 @@ static int sha2_file(const char *path, unsigned char
> output[32], int is224)
>
> MALLOC(buf, (size_t)ALPM_BUFFER_SIZE, return 1);
>
> - OPEN(fd, path, O_RDONLY);
> + OPEN(fd, path, O_RDONLY | O_CLOEXEC);
> if(fd < 0) {
> free(buf);
> return 1;
>
