<div dir="ltr">How old? O_CLOEXEC is part of the 2008 POSIX specification. I really wouldn't like to build on anything older than that... :( </div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Sep 22, 2014 at 1:24 PM, Aaron Plattner <<a href="mailto:aplattner@nvidia.com" target="_blank">aplattner@nvidia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Old versions of Linux don't support O_CLOEXEC, so rather than failing to build, allow building without O_CLOEXEC if a new ALLOW_NO_O_CLOEXEC flag is defined. Create a new helper function, pci_open_cloexec that uses O_CLOEXEC when it's available and falls back to open + fcntl when it's not. Route all calls to open that pass O_CLOEXEC through this new function instead using the following Coccinelle patch: @@ expression path, flags; @@ -open(path, flags | O_CLOEXEC) +pci_open_cloexec(path, flags) Signed-off-by: Aaron Plattner <<a href="mailto:aplattner@nvidia.com">aplattner@nvidia.com</a>> --- There are three calls to open() that don't use O_CLOEXEC -- are those bugs? src/linux_devmem.c: fd = open("/dev/mem", O_RDONLY, 0); src/netbsd_pci.c: fd = open("/dev/ttyE0", O_RDONLY); src/netbsd_pci.c: pcifd = open(netbsd_devname, O_RDWR); src/common_vgaarb.c | 2 +- src/freebsd_pci.c | 10 +++++----- src/linux_sysfs.c | 26 +++++++++++++------------- src/netbsd_pci.c | 2 +- src/openbsd_pci.c | 2 +- src/pciaccess_private.h | 33 +++++++++++++++++++++++++++++---- src/solx_devfs.c | 4 ++-- src/x86_pci.c | 4 ++-- 8 files changed, 54 insertions(+), 29 deletions(-) diff --git a/src/common_vgaarb.c b/src/common_vgaarb.c index c59bd788dc5e..f3b523e67c3a 100644 --- a/src/common_vgaarb.c +++ b/src/common_vgaarb.c @@ -131,7 +131,7 @@ pci_device_vgaarb_init(void) if (!pci_sys) return -1; - if ((pci_sys->vgaarb_fd = open ("/dev/vga_arbiter", O_RDWR | O_CLOEXEC)) < 0) { + if ((pci_sys->vgaarb_fd = pci_open_cloexec ("/dev/vga_arbiter", O_RDWR)) < 0) { return errno; } diff --git a/src/freebsd_pci.c b/src/freebsd_pci.c index 9505a1c17d21..e40d80847bf0 100644 --- a/src/freebsd_pci.c +++ b/src/freebsd_pci.c @@ -112,7 +112,7 @@ pci_device_freebsd_map_range(struct pci_device *dev, int fd, err = 0; - fd = open("/dev/mem", O_RDWR | O_CLOEXEC); + fd = pci_open_cloexec("/dev/mem", O_RDWR); if (fd == -1) return errno; @@ -157,7 +157,7 @@ pci_device_freebsd_unmap_range( struct pci_device *dev, if ((map->flags & PCI_DEV_MAP_FLAG_CACHABLE) || (map->flags & PCI_DEV_MAP_FLAG_WRITE_COMBINE)) { - fd = open("/dev/mem", O_RDWR | O_CLOEXEC); + fd = pci_open_cloexec("/dev/mem", O_RDWR); if (fd != -1) { mrd.mr_base = map->base; mrd.mr_len = map->size; @@ -297,7 +297,7 @@ pci_device_freebsd_read_rom( struct pci_device * dev, void * buffer ) } printf("Using rom_base = 0x%lx\n", (long)rom_base); - memfd = open( "/dev/mem", O_RDONLY | O_CLOEXEC ); + memfd = pci_open_cloexec( "/dev/mem", O_RDONLY ); if ( memfd == -1 ) return errno; @@ -574,7 +574,7 @@ pci_device_freebsd_open_legacy_io(struct pci_io_handle *ret, struct pci_device *dev, pciaddr_t base, pciaddr_t size) { #if defined(__i386__) || defined(__amd64__) - ret->fd = open("/dev/io", O_RDWR | O_CLOEXEC); + ret->fd = pci_open_cloexec("/dev/io", O_RDWR); if (ret->fd < 0) return NULL; @@ -735,7 +735,7 @@ pci_system_freebsd_create( void ) int i; /* Try to open the PCI device */ - pcidev = open( "/dev/pci", O_RDWR | O_CLOEXEC ); + pcidev = pci_open_cloexec( "/dev/pci", O_RDWR ); if ( pcidev == -1 ) return ENXIO; diff --git a/src/linux_sysfs.c b/src/linux_sysfs.c index 08c99719bcba..120cd6411893 100644 --- a/src/linux_sysfs.c +++ b/src/linux_sysfs.c @@ -100,7 +100,7 @@ pci_system_linux_sysfs_create( void ) if ( pci_sys != NULL ) { pci_sys->methods = & linux_sysfs_methods; #ifdef HAVE_MTRR - pci_sys->mtrr_fd = open("/proc/mtrr", O_WRONLY | O_CLOEXEC); + pci_sys->mtrr_fd = pci_open_cloexec("/proc/mtrr", O_WRONLY); #endif err = populate_entries(pci_sys); } @@ -247,7 +247,7 @@ pci_device_linux_sysfs_probe( struct pci_device * dev ) dev->bus, dev->dev, dev->func ); - fd = open( name, O_RDONLY | O_CLOEXEC); + fd = pci_open_cloexec(name, O_RDONLY); if ( fd != -1 ) { char * next; pciaddr_t low_addr; @@ -309,7 +309,7 @@ pci_device_linux_sysfs_read_rom( struct pci_device * dev, void * buffer ) dev->dev, dev->func ); - fd = open( name, O_RDWR | O_CLOEXEC); + fd = pci_open_cloexec(name, O_RDWR); if ( fd == -1 ) { #ifdef LINUX_ROM /* If reading the ROM using sysfs fails, fall back to the old @@ -390,7 +390,7 @@ pci_device_linux_sysfs_read( struct pci_device * dev, void * data, dev->dev, dev->func ); - fd = open( name, O_RDONLY | O_CLOEXEC); + fd = pci_open_cloexec(name, O_RDONLY); if ( fd == -1 ) { return errno; } @@ -450,7 +450,7 @@ pci_device_linux_sysfs_write( struct pci_device * dev, const void * data, dev->dev, dev->func ); - fd = open( name, O_WRONLY | O_CLOEXEC); + fd = pci_open_cloexec(name, O_WRONLY); if ( fd == -1 ) { return errno; } @@ -501,7 +501,7 @@ pci_device_linux_sysfs_map_range_wc(struct pci_device *dev, dev->dev, dev->func, map->region); - fd = open(name, open_flags | O_CLOEXEC); + fd = pci_open_cloexec(name, open_flags); if (fd == -1) return errno; @@ -566,7 +566,7 @@ pci_device_linux_sysfs_map_range(struct pci_device *dev, dev->func, map->region); - fd = open(name, open_flags | O_CLOEXEC); + fd = pci_open_cloexec(name, open_flags); if (fd == -1) { return errno; } @@ -689,7 +689,7 @@ static void pci_device_linux_sysfs_enable(struct pci_device *dev) dev->dev, dev->func ); - fd = open( name, O_RDWR | O_CLOEXEC); + fd = pci_open_cloexec(name, O_RDWR); if (fd == -1) return; @@ -711,7 +711,7 @@ static int pci_device_linux_sysfs_boot_vga(struct pci_device *dev) dev->dev, dev->func ); - fd = open( name, O_RDONLY | O_CLOEXEC); + fd = pci_open_cloexec(name, O_RDONLY); if (fd == -1) return 0; @@ -754,7 +754,7 @@ pci_device_linux_sysfs_open_device_io(struct pci_io_handle *ret, snprintf(name, PATH_MAX, "%s/%04x:%02x:%02x.%1u/resource%d", SYS_BUS_PCI, dev->domain, dev->bus, dev->dev, dev->func, bar); - ret->fd = open(name, O_RDWR | O_CLOEXEC); + ret->fd = pci_open_cloexec(name, O_RDWR); if (ret->fd < 0) return NULL; @@ -778,7 +778,7 @@ pci_device_linux_sysfs_open_legacy_io(struct pci_io_handle *ret, snprintf(name, PATH_MAX, "/sys/class/pci_bus/%04x:%02x/legacy_io", dev->domain, dev->bus); - ret->fd = open(name, O_RDWR | O_CLOEXEC); + ret->fd = pci_open_cloexec(name, O_RDWR); if (ret->fd >= 0) break; @@ -925,7 +925,7 @@ pci_device_linux_sysfs_map_legacy(struct pci_device *dev, pciaddr_t base, snprintf(name, PATH_MAX, "/sys/class/pci_bus/%04x:%02x/legacy_mem", dev->domain, dev->bus); - fd = open(name, flags | O_CLOEXEC); + fd = pci_open_cloexec(name, flags); if (fd >= 0) break; @@ -934,7 +934,7 @@ pci_device_linux_sysfs_map_legacy(struct pci_device *dev, pciaddr_t base, /* If not, /dev/mem is the best we can do */ if (!dev) - fd = open("/dev/mem", flags | O_CLOEXEC); + fd = pci_open_cloexec("/dev/mem", flags); if (fd < 0) return errno; diff --git a/src/netbsd_pci.c b/src/netbsd_pci.c index 52591b08f737..87bb2feb2504 100644 --- a/src/netbsd_pci.c +++ b/src/netbsd_pci.c @@ -909,7 +909,7 @@ pci_system_netbsd_create(void) ndevs = 0; nbuses = 0; snprintf(netbsd_devname, 32, "/dev/pci%d", nbuses); - pcifd = open(netbsd_devname, O_RDWR | O_CLOEXEC); + pcifd = pci_open_cloexec(netbsd_devname, O_RDWR); while (pcifd > 0) { ioctl(pcifd, PCI_IOC_BUSINFO, &businfo); buses[nbuses].fd = pcifd; diff --git a/src/openbsd_pci.c b/src/openbsd_pci.c index fe034f3b046b..3d66598b5b77 100644 --- a/src/openbsd_pci.c +++ b/src/openbsd_pci.c @@ -571,7 +571,7 @@ pci_system_openbsd_create(void) for (domain = 0; domain < sizeof(pcifd) / sizeof(pcifd[0]); domain++) { snprintf(path, sizeof(path), "/dev/pci%d", domain); - pcifd[domain] = open(path, O_RDWR | O_CLOEXEC); + pcifd[domain] = pci_open_cloexec(path, O_RDWR); if (pcifd[domain] == -1) break; ndomains++; diff --git a/src/pciaccess_private.h b/src/pciaccess_private.h index 9f4e8f9ab573..0de989bceab4 100644 --- a/src/pciaccess_private.h +++ b/src/pciaccess_private.h @@ -40,13 +40,16 @@ /* * O_CLOEXEC fixes an fd leak case (see 'man 2 open' for details). I don't * know of any OS we support where this isn't available in a sufficiently - * new version, so warn unconditionally. + * new version, so fail if ALLOW_NO_O_CLOEXEC is not defined. */ #include <sys/fcntl.h> -#ifndef O_CLOEXEC -#warning O_CLOEXEC not available, please upgrade. -#define O_CLOEXEC 0 +#if !defined(O_CLOEXEC) +# if defined(ALLOW_NO_O_CLOEXEC) +# include <unistd.h> +# else +# error O_CLOEXEC not available, please upgrade. +# endif #endif @@ -191,3 +194,25 @@ extern void pci_system_openbsd_init_dev_mem( int ); extern int pci_system_solx_devfs_create( void ); extern int pci_system_x86_create( void ); extern void pci_io_cleanup( void ); + +static inline int pci_open_cloexec( const char *path, int flags ) +{ +#if defined(O_CLOEXEC) + return open(path, flags | O_CLOEXEC); +#else + int fd = open(path, flags); + if (fd >= 0) { + int flags = fcntl(fd, F_GETFD); + if (flags == -1) { + close(fd); + return -1; + } + flags |= FD_CLOEXEC; + if (fcntl(fd, F_SETFD, flags) == -1) { + close(fd); + return -1; + } + } + return fd; +#endif +} diff --git a/src/solx_devfs.c b/src/solx_devfs.c index f57239304a43..2d4b102f8cf1 100644 --- a/src/solx_devfs.c +++ b/src/solx_devfs.c @@ -415,7 +415,7 @@ probe_nexus_node(di_node_t di_node, di_minor_t minor, void *arg) nexus_path, first_bus, last_bus); #endif - if ((fd = open(nexus_path, O_RDWR | O_CLOEXEC)) >= 0) { + if ((fd = pci_open_cloexec(nexus_path, O_RDWR)) >= 0) { probe_args_t args; nexus->fd = fd; @@ -718,7 +718,7 @@ pci_device_solx_devfs_map_range(struct pci_device *dev, #endif if (map_fd < 0) { - if ((map_fd = open(map_dev, O_RDWR | O_CLOEXEC)) < 0) { + if ((map_fd = pci_open_cloexec(map_dev, O_RDWR)) < 0) { err = errno; (void) fprintf(stderr, "can not open %s: %s\n", map_dev, strerror(errno)); diff --git a/src/x86_pci.c b/src/x86_pci.c index 49c1cabc3c6c..7088ff9a016d 100644 --- a/src/x86_pci.c +++ b/src/x86_pci.c @@ -457,7 +457,7 @@ pci_device_x86_read_rom(struct pci_device *dev, void *buffer) return ENOSYS; } - memfd = open("/dev/mem", O_RDONLY | O_CLOEXEC); + memfd = pci_open_cloexec("/dev/mem", O_RDONLY); if (memfd == -1) return errno; @@ -637,7 +637,7 @@ static int pci_device_x86_map_range(struct pci_device *dev, struct pci_device_mapping *map) { - int memfd = open("/dev/mem", O_RDWR | O_CLOEXEC); + int memfd = pci_open_cloexec("/dev/mem", O_RDWR); int prot = PROT_READ; if (memfd == -1) -- 2.1.0 _______________________________________________ <a href="mailto:xorg-devel@lists.x.org">xorg-devel@lists.x.org</a>: X.Org development Archives: <a href="http://lists.x.org/archives/xorg-devel" target="_blank">http://lists.x.org/archives/xorg-devel</a> Info: <a href="http://lists.x.org/mailman/listinfo/xorg-devel" target="_blank">http://lists.x.org/mailman/listinfo/xorg-devel</a> </blockquote></div> -- Jasper </div>