From 0205f8e66ce8c2d9c439542953f237a17f23b194 Mon Sep 17 00:00:00 2001 From: guenther Date: Fri, 15 Aug 2014 03:51:40 +0000 Subject: [PATCH] Use O_CLOEXEC wherever we open a file and then call fcntl(F_SETFD, FD_CLOEXEC) on it, simplifying error checking, reducing system calls, and improving thread-safety for libraries. ok miod@ --- lib/libc/db/hash/hash.c | 5 +-- lib/libkvm/kvm.c | 66 ++++++++++++++--------------- lib/libsndio/mio_rmidi.c | 11 +---- lib/libsndio/sio_sun.c | 8 +--- lib/libutil/passwd.c | 13 ++---- usr.bin/mail/names.c | 5 +-- usr.bin/usbhidaction/usbhidaction.c | 8 +--- usr.sbin/apmd/apmd.c | 7 ++- usr.sbin/cron/misc.c | 7 ++- 9 files changed, 51 insertions(+), 79 deletions(-) diff --git a/lib/libc/db/hash/hash.c b/lib/libc/db/hash/hash.c index 4f428e86377..389afb5f8a3 100644 --- a/lib/libc/db/hash/hash.c +++ b/lib/libc/db/hash/hash.c @@ -1,4 +1,4 @@ -/* $OpenBSD: hash.c,v 1.24 2013/04/29 00:28:23 okan Exp $ */ +/* $OpenBSD: hash.c,v 1.25 2014/08/15 03:51:40 guenther Exp $ */ /*- * Copyright (c) 1990, 1993, 1994 @@ -115,9 +115,8 @@ __hash_open(const char *file, int flags, int mode, hashp->flags = flags; if (file) { - if ((hashp->fp = open(file, flags, mode)) == -1) + if ((hashp->fp = open(file, flags | O_CLOEXEC, mode)) == -1) RETURN_ERROR(errno, error0); - (void)fcntl(hashp->fp, F_SETFD, FD_CLOEXEC); new_table = fstat(hashp->fp, &statbuf) == 0 && statbuf.st_size == 0 && (flags & O_ACCMODE) != O_RDONLY; } else diff --git a/lib/libkvm/kvm.c b/lib/libkvm/kvm.c index 2f393c6c24a..7fb4036ba82 100644 --- a/lib/libkvm/kvm.c +++ b/lib/libkvm/kvm.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kvm.c,v 1.52 2013/11/16 00:41:44 guenther Exp $ */ +/* $OpenBSD: kvm.c,v 1.53 2014/08/15 03:51:40 guenther Exp $ */ /* $NetBSD: kvm.c,v 1.43 1996/05/05 04:31:59 gwr Exp $ */ /*- @@ -64,11 +64,11 @@ extern int __fdnlist(int, struct nlist *); static int kvm_dbopen(kvm_t *, const char *); +static int kvm_opennamelist(kvm_t *, const char *); static int _kvm_get_header(kvm_t *); static kvm_t *_kvm_open(kvm_t *, const char *, const char *, const char *, int, char *); static int clear_gap(kvm_t *, FILE *, int); -static int kvm_setfd(kvm_t *); char * kvm_geterr(kvm_t *kd) @@ -202,10 +202,12 @@ _kvm_open(kvm_t *kd, const char *uf, const char *mf, const char *sf, _kvm_err(kd, kd->program, "bad flags arg"); goto failed; } + flag |= O_CLOEXEC; + if (mf == 0) mf = _PATH_MEM; - if ((kd->pmfd = open(mf, flag, 0)) < 0) { + if ((kd->pmfd = open(mf, flag)) < 0) { _kvm_syserr(kd, kd->program, "%s", mf); goto failed; } @@ -225,12 +227,12 @@ _kvm_open(kvm_t *kd, const char *uf, const char *mf, const char *sf, "%s: not physical memory device", mf); goto failed; } - if ((kd->vmfd = open(_PATH_KMEM, flag, 0)) < 0) { + if ((kd->vmfd = open(_PATH_KMEM, flag)) < 0) { _kvm_syserr(kd, kd->program, "%s", _PATH_KMEM); goto failed; } kd->alive = 1; - if (sf != NULL && (kd->swfd = open(sf, flag, 0)) < 0) { + if (sf != NULL && (kd->swfd = open(sf, flag)) < 0) { _kvm_syserr(kd, kd->program, "%s", sf); goto failed; } @@ -244,12 +246,8 @@ _kvm_open(kvm_t *kd, const char *uf, const char *mf, const char *sf, * fall back to _PATH_UNIX. */ if (kvm_dbopen(kd, uf ? uf : _PATH_UNIX) == -1 && - ((uf && (kd->nlfd = open(uf, O_RDONLY, 0)) == -1) || (!uf && - (kd->nlfd = open((uf = _PATH_KSYMS), O_RDONLY, 0)) == -1 && - (kd->nlfd = open((uf = _PATH_UNIX), O_RDONLY, 0)) == -1))) { - _kvm_syserr(kd, kd->program, "%s", uf); + kvm_opennamelist(kd, uf)) goto failed; - } } else { /* * This is a crash dump. @@ -258,12 +256,8 @@ _kvm_open(kvm_t *kd, const char *uf, const char *mf, const char *sf, * If no file is specified, try opening _PATH_KSYMS and * fall back to _PATH_UNIX. */ - if ((uf && (kd->nlfd = open(uf, O_RDONLY, 0)) == -1) || (!uf && - (kd->nlfd = open((uf = _PATH_KSYMS), O_RDONLY, 0)) == -1 && - (kd->nlfd = open((uf = _PATH_UNIX), O_RDONLY, 0)) == -1)) { - _kvm_syserr(kd, kd->program, "%s", uf); + if (kvm_opennamelist(kd, uf)) goto failed; - } /* * If there is no valid core header, fail silently here. @@ -276,10 +270,7 @@ _kvm_open(kvm_t *kd, const char *uf, const char *mf, const char *sf, goto failed; } } - if (kvm_setfd(kd) == 0) - return (kd); - else - _kvm_syserr(kd, kd->program, "can't set close on exec flag"); + return (kd); failed: /* * Copy out the error if doing sane error semantics. @@ -290,6 +281,28 @@ failed: return (0); } +static int +kvm_opennamelist(kvm_t *kd, const char *uf) +{ + int fd; + + if (uf != NULL) + fd = open(uf, O_RDONLY | O_CLOEXEC); + else { + fd = open(_PATH_KSYMS, O_RDONLY | O_CLOEXEC); + uf = _PATH_UNIX; + if (fd == -1) + fd = open(uf, O_RDONLY | O_CLOEXEC); + } + if (fd == -1) { + _kvm_syserr(kd, kd->program, "%s", uf); + return (-1); + } + + kd->nlfd = fd; + return (0); +} + /* * The kernel dump file (from savecore) contains: * kcore_hdr_t kcore_hdr; @@ -902,18 +915,3 @@ kvm_write(kvm_t *kd, u_long kva, const void *buf, size_t len) } /* NOTREACHED */ } - -static int -kvm_setfd(kvm_t *kd) -{ - if (kd->pmfd >= 0 && fcntl(kd->pmfd, F_SETFD, FD_CLOEXEC) < 0) - return (-1); - if (kd->vmfd >= 0 && fcntl(kd->vmfd, F_SETFD, FD_CLOEXEC) < 0) - return (-1); - if (kd->nlfd >= 0 && fcntl(kd->nlfd, F_SETFD, FD_CLOEXEC) < 0) - return (-1); - if (kd->swfd >= 0 && fcntl(kd->swfd, F_SETFD, FD_CLOEXEC) < 0) - return (-1); - - return (0); -} diff --git a/lib/libsndio/mio_rmidi.c b/lib/libsndio/mio_rmidi.c index 79f78933f05..cb9d30cb997 100644 --- a/lib/libsndio/mio_rmidi.c +++ b/lib/libsndio/mio_rmidi.c @@ -1,4 +1,4 @@ -/* $OpenBSD: mio_rmidi.c,v 1.13 2013/11/13 22:38:22 ratchov Exp $ */ +/* $OpenBSD: mio_rmidi.c,v 1.14 2014/08/15 03:51:40 guenther Exp $ */ /* * Copyright (c) 2008 Alexandre Ratchov * @@ -77,21 +77,14 @@ _mio_rmidi_open(const char *str, unsigned int mode, int nbio) flags = O_RDWR; else flags = (mode & MIO_OUT) ? O_WRONLY : O_RDONLY; - while ((fd = open(path, flags | O_NONBLOCK)) < 0) { + while ((fd = open(path, flags | O_NONBLOCK | O_CLOEXEC)) < 0) { if (errno == EINTR) continue; DPERROR(path); goto bad_free; } - if (fcntl(fd, F_SETFD, FD_CLOEXEC) < 0) { - DPERROR("FD_CLOEXEC"); - goto bad_close; - } hdl->fd = fd; return (struct mio_hdl *)hdl; - bad_close: - while (close(hdl->fd) < 0 && errno == EINTR) - ; /* retry */ bad_free: free(hdl); return NULL; diff --git a/lib/libsndio/sio_sun.c b/lib/libsndio/sio_sun.c index 948302eaa96..db2b1ce39c9 100644 --- a/lib/libsndio/sio_sun.c +++ b/lib/libsndio/sio_sun.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sio_sun.c,v 1.11 2014/03/05 20:40:49 ratchov Exp $ */ +/* $OpenBSD: sio_sun.c,v 1.12 2014/08/15 03:51:40 guenther Exp $ */ /* * Copyright (c) 2008 Alexandre Ratchov * @@ -363,16 +363,12 @@ _sio_sun_open(const char *str, unsigned int mode, int nbio) else flags = (mode & SIO_PLAY) ? O_WRONLY : O_RDONLY; - while ((fd = open(path, flags | O_NONBLOCK)) < 0) { + while ((fd = open(path, flags | O_NONBLOCK | O_CLOEXEC)) < 0) { if (errno == EINTR) continue; DPERROR(path); goto bad_free; } - if (fcntl(fd, F_SETFD, FD_CLOEXEC) < 0) { - DPERROR("FD_CLOEXEC"); - goto bad_close; - } /* * pause the device diff --git a/lib/libutil/passwd.c b/lib/libutil/passwd.c index 26db941e83c..31c1ccc006a 100644 --- a/lib/libutil/passwd.c +++ b/lib/libutil/passwd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: passwd.c,v 1.52 2013/08/17 06:54:21 guenther Exp $ */ +/* $OpenBSD: passwd.c,v 1.53 2014/08/15 03:51:40 guenther Exp $ */ /* * Copyright (c) 1987, 1993, 1994, 1995 @@ -93,7 +93,6 @@ pw_lock(int retries) { int i, fd; mode_t old_mode; - int save_errno; if (!pw_lck) { errno = EINVAL; @@ -101,18 +100,12 @@ pw_lock(int retries) } /* Acquire the lock file. */ old_mode = umask(0); - fd = open(pw_lck, O_WRONLY|O_CREAT|O_EXCL, 0600); + fd = open(pw_lck, O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0600); for (i = 0; i < retries && fd < 0 && errno == EEXIST; i++) { sleep(1); - fd = open(pw_lck, O_WRONLY|O_CREAT|O_EXCL, 0600); - } - save_errno = errno; - if (fd != -1 && fcntl(fd, F_SETFD, FD_CLOEXEC) == -1) { - close(fd); - fd = -1; + fd = open(pw_lck, O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0600); } (void) umask(old_mode); - errno = save_errno; return (fd); } diff --git a/usr.bin/mail/names.c b/usr.bin/mail/names.c index 7c0e9e0eef3..9deadf04190 100644 --- a/usr.bin/mail/names.c +++ b/usr.bin/mail/names.c @@ -1,4 +1,4 @@ -/* $OpenBSD: names.c,v 1.19 2013/04/29 00:28:23 okan Exp $ */ +/* $OpenBSD: names.c,v 1.20 2014/08/15 03:51:40 guenther Exp $ */ /* $NetBSD: names.c,v 1.5 1996/06/08 19:48:32 christos Exp $ */ /* @@ -240,7 +240,7 @@ outof(struct name *names, FILE *fo, struct header *hp) senderr++; goto cant; } - image = open(tempname, O_RDWR); + image = open(tempname, O_RDWR | O_CLOEXEC); (void)rm(tempname); if (image < 0) { warn("%s", tempname); @@ -248,7 +248,6 @@ outof(struct name *names, FILE *fo, struct header *hp) (void)Fclose(fout); goto cant; } - (void)fcntl(image, F_SETFD, FD_CLOEXEC); fprintf(fout, "From %s %s", myname, date); puthead(hp, fout, GTO|GSUBJECT|GCC|GNL); while ((c = getc(fo)) != EOF) diff --git a/usr.bin/usbhidaction/usbhidaction.c b/usr.bin/usbhidaction/usbhidaction.c index 4197e6e65a5..aed6e14d238 100644 --- a/usr.bin/usbhidaction/usbhidaction.c +++ b/usr.bin/usbhidaction/usbhidaction.c @@ -1,4 +1,4 @@ -/* $OpenBSD: usbhidaction.c,v 1.17 2013/11/27 00:13:23 deraadt Exp $ */ +/* $OpenBSD: usbhidaction.c,v 1.18 2014/08/15 03:51:40 guenther Exp $ */ /* $NetBSD: usbhidaction.c,v 1.7 2002/01/18 14:38:59 augustss Exp $ */ /* @@ -135,14 +135,10 @@ main(int argc, char **argv) if (demon && conf[0] != '/') errx(1, "config file must have an absolute path, %s", conf); - fd = open(dev, O_RDWR); + fd = open(dev, O_RDWR | O_CLOEXEC); if (fd < 0) err(1, "%s", dev); - /* Avoid passing the device file descriptor to executed commands */ - if (fcntl(fd, F_SETFD, FD_CLOEXEC) == -1) - err(1, "fcntl(F_SETFD, FD_CLOEXEC)"); - if (ioctl(fd, USB_GET_REPORT_ID, &reportid) < 0) reportid = -1; repd = hid_get_report_desc(fd); diff --git a/usr.sbin/apmd/apmd.c b/usr.sbin/apmd/apmd.c index 75efde8fd97..4c0a62bb9a0 100644 --- a/usr.sbin/apmd/apmd.c +++ b/usr.sbin/apmd/apmd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: apmd.c,v 1.65 2014/07/26 10:48:59 mpi Exp $ */ +/* $OpenBSD: apmd.c,v 1.66 2014/08/15 03:51:40 guenther Exp $ */ /* * Copyright (c) 1995, 1996 John T. Kohl @@ -596,11 +596,10 @@ main(int argc, char *argv[]) (void) signal(SIGHUP, sigexit); (void) signal(SIGINT, sigexit); - if ((ctl_fd = open(fname, O_RDWR)) == -1) { + if ((ctl_fd = open(fname, O_RDWR | O_CLOEXEC)) == -1) { if (errno != ENXIO && errno != ENOENT) error("cannot open device file `%s'", fname); - } else if (fcntl(ctl_fd, F_SETFD, FD_CLOEXEC) == -1) - error("cannot set close-on-exec for `%s'", fname); + } sock_fd = bind_socket(sockname); diff --git a/usr.sbin/cron/misc.c b/usr.sbin/cron/misc.c index bc3bae6a0bb..8edf62c35a6 100644 --- a/usr.sbin/cron/misc.c +++ b/usr.sbin/cron/misc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: misc.c,v 1.46 2013/04/21 00:14:51 tedu Exp $ */ +/* $OpenBSD: misc.c,v 1.47 2014/08/15 03:51:40 guenther Exp $ */ /* Copyright 1988,1990,1993,1994 by Paul Vixie * All rights reserved @@ -460,13 +460,12 @@ log_it(const char *username, PID_T xpid, const char *event, const char *detail) return; if (LogFD < OK) { - LogFD = open(LOG_FILE, O_WRONLY|O_APPEND|O_CREAT, 0600); + LogFD = open(LOG_FILE, O_WRONLY|O_APPEND|O_CREAT|O_CLOEXEC, + 0600); if (LogFD < OK) { fprintf(stderr, "%s: can't open log file\n", ProgramName); perror(LOG_FILE); - } else { - (void) fcntl(LogFD, F_SETFD, FD_CLOEXEC); } } -- 2.20.1