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 <[email protected]
<mailto:[email protected]>> 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 <[email protected]
    <mailto:[email protected]>>
    ---
    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

    _______________________________________________
    [email protected] <mailto:[email protected]>: X.Org
    development
    Archives: http://lists.x.org/archives/xorg-devel
    Info: http://lists.x.org/mailman/listinfo/xorg-devel




--
   Jasper


--
Aaron
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to