[PATCH libpciaccess] Allow building without O_CLOEXEC
Aaron Plattner
aplattner at nvidia.com
Mon Sep 22 13:12:11 PDT 2014
On 09/22/2014 12:56 PM, Jasper St. Pierre wrote:
> How old? O_CLOEXEC is part of the 2008 POSIX specification. I really
> wouldn't like to build on anything older than that... :(
As old as possible. :)
For context, I want to use libpciaccess's device enumeration stuff in
nvidia-installer to identify legacy devices. I don't need any of the
mapping code and I don't need to worry about the application forking,
which is why not having O_CLOEXEC is safe in my case.
According to our README [1], the minimum supported kernel is 2.6.18.
The open(2) man page says that O_CLOEXEC was introduced in Linux 2.6.23.
I agree that allowing the library to be built normally (i.e. outside of
special custom configurations) with the fallback isn't a good idea,
which is why I didn't wire up ALLOW_NO_O_CLOEXEC to a configure option.
[1]
http://us.download.nvidia.com/XFree86/Linux-x86/343.22/README/minimumrequirements.html
> On Mon, Sep 22, 2014 at 1:24 PM, Aaron Plattner <aplattner at nvidia.com
> <mailto:aplattner at nvidia.com>> wrote:
>
> 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 <aplattner at nvidia.com
> <mailto:aplattner at nvidia.com>>
> ---
> 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
>
> _______________________________________________
> xorg-devel at lists.x.org <mailto:xorg-devel at lists.x.org>: X.Org
> development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
>
>
>
> --
> Jasper
--
Aaron
More information about the xorg-devel
mailing list