Add access() calls to ensure fopen(...,"r") doesn't segfault

One of the invariants of the fopen() function called with the "r" option is that the file exist before the function is called. If this is not the case behavior is undefined (in the case of glibc on some Crays and Redhat machines this means you get a segfault in fileno_unlocked()). This commit is a first attempt at ensuring this invariant holds when calls to fopen() are made. Signed-off-by: Noah Evans <[email protected]>
open-mpi · Oct 13, 2017 · a574862 · a574862
1 parent fba6990
commit a574862
Show file tree

Hide file tree

Showing 40 changed files with 190 additions and 65 deletions.
diff --git a/ompi/debuggers/dlopen_test.c b/ompi/debuggers/dlopen_test.c
@@ -65,12 +65,17 @@ static int do_test(void)
 
     /* Double check that the .la file is there that we expect; if it's
        not, skip this test. */
+    if (!access(full_filename, F_OK)) {
+      fprintf(stderr,
+              "File %s.la doesn't seem to exist; skipping this test\n",
+              full_filename);
+      exit(77);
+    }
     fp = fopen(full_filename, "r");
-    if (NULL == fp) {
-        fprintf(stderr,
-                "File %s.la doesn't seem to exist; skipping this test\n",
-                full_filename);
-        exit(77);
+    if (fp == NULL) {
+      fprintf(stderr, "File %s.la fopen failed; skipping this test\n",
+              full_filename);
+      exit(77);
     }
     /* We know the .la file is there, so read it, looking for the
        dlopen value.  If the dlopen value is '' (i.e., empty), then

diff --git a/ompi/debuggers/ompi_debuggers.c b/ompi/debuggers/ompi_debuggers.c
@@ -148,7 +148,7 @@ static void check(char *dir, char *file, char **locations)
         FILE *fp;
 
         /* Just try to open the file */
-        if (NULL != (fp = fopen(str, "r"))) {
+        if (access(str, F_OK) && (NULL != (fp = fopen(str, "r"))) {
             fclose(fp);
             opal_argv_append_nosize(&locations, file);
         }

diff --git a/ompi/mca/coll/tuned/coll_tuned_dynamic_file.c b/ompi/mca/coll/tuned/coll_tuned_dynamic_file.c
@@ -88,6 +88,10 @@ int ompi_coll_tuned_read_rules_config_file (char *fname, ompi_coll_alg_rule_t**
         return (-3);
     }
 
+    if (!access(fname, F_OK)) {
+        OPAL_OUTPUT((ompi_coll_tuned_stream,"rules file does not exist [%s]\n", fname));
+        goto on_file_error;
+    }
     fptr = fopen (fname, "r");
     if (!fptr) {
         OPAL_OUTPUT((ompi_coll_tuned_stream,"cannot read rules file [%s]\n", fname));

diff --git a/ompi/mca/topo/treematch/treematch/tm_mapping.c b/ompi/mca/topo/treematch/treematch/tm_mapping.c
@@ -73,6 +73,12 @@ int nb_lines(char *filename)
   char line[LINE_SIZE];
   int N = 0;
 
+  if(!access(filename, F_OK)) {
+    if(tm_get_verbose_level() >= CRITICAL)
+      fprintf(stderr,"%s does not exist\n",filename);
+    exit(-1);
+  }
+
   if(!(pf = fopen(filename,"r"))){
     if(tm_get_verbose_level() >= CRITICAL)
       fprintf(stderr,"Cannot open %s\n",filename);
@@ -97,6 +103,11 @@ void init_mat(char *filename,int N, double **mat, double *sum_row)
   int i,j;
   unsigned int vl = tm_get_verbose_level();
 
+  if(!access(filename, F_OK)) {
+    if(tm_get_verbose_level() >= CRITICAL)
+      fprintf(stderr,"%s does not exist\n",filename);
+    exit(-1);
+  }
 
   if(!(pf=fopen(filename,"r"))){
     if(vl >= CRITICAL)

diff --git a/ompi/mca/topo/treematch/treematch/tm_topology.c b/ompi/mca/topo/treematch/treematch/tm_topology.c
@@ -481,6 +481,11 @@ int  tm_topology_add_binding_constraints(char *constraints_filename, tm_topology
   int i,n;
   unsigned int vl = tm_get_verbose_level();
 
+  if (!access(constraints_filename, F_OK)) {
+    if(tm_get_verbose_level() >= CRITICAL)
+      fprintf(stderr,"%s does not exist\n",constraints_filename);
+    exit(-1);
+  }
 
   if (!(pf = fopen(constraints_filename,"r"))) {
     if(vl >= CRITICAL)

diff --git a/opal/mca/btl/openib/btl_openib_ini.c b/opal/mca/btl/openib/btl_openib_ini.c
@@ -229,9 +229,17 @@ static int parse_file(char *filename)
 
     /* Open the file */
     ini_filename = filename;
+
+    if (!access(filename, F_OK)) {
+        opal_show_help("help-mpi-btl-openib.txt", "ini file:file not found",
+                       true, filename);
+        ret = OPAL_ERR_NOT_FOUND;
+        goto cleanup;
+    }
+
     btl_openib_ini_yyin = fopen(filename, "r");
     if (NULL == btl_openib_ini_yyin) {
-        opal_show_help("help-mpi-btl-openib.txt", "ini file:file not found",
+        opal_show_help("help-mpi-btl-openib.txt", "ini file:could not open",
                        true, filename);
         ret = OPAL_ERR_NOT_FOUND;
         goto cleanup;

diff --git a/opal/mca/btl/ugni/btl_ugni_init.c b/opal/mca/btl/ugni/btl_ugni_init.c
@@ -264,12 +264,13 @@ int mca_btl_ugni_init (void)
         mca_btl_ugni_component.virtual_device_count = MCA_BTL_UGNI_MAX_DEV_HANDLES;
     }
 
-    fh = fopen ("/proc/sys/kernel/pid_max", "r");
-    if (NULL != fh) {
-        fscanf (fh, "%d", &pid_max);
-        fclose (fh);
+    if (access("/proc/sys/kernel/pid_max", F_OK)) {
+        fh = fopen ("/proc/sys/kernel/pid_max", "r");
+        if (NULL != fh) {
+            fscanf (fh, "%d", &pid_max);
+            fclose (fh);
+        }
     }
-
     /* Use pid to generate the cdm_id.  Although its not stated in the uGNI
      * documentation, the cdm_id only needs to be unique within a node for a
      * given ptag/cookie tuple */

diff --git a/opal/mca/crs/blcr/crs_blcr_module.c b/opal/mca/crs/blcr/crs_blcr_module.c
@@ -739,6 +739,14 @@ static int blcr_cold_start(opal_crs_blcr_snapshot_t *snapshot) {
      * Find the snapshot directory, read the metadata file
      */
     if( NULL == snapshot->super.metadata ) {
+        if(access(snapshot->super.metadata_filename, F_OK) {
+            opal_output(mca_crs_blcr_component.super.output_handle,
+                        "crs:blcr: checkpoint(): Error: %s does not exist",
+                        snapshot->super.metadata_filename);
+            exit_status = OPAL_ERROR;
+            goto cleanup;
+        }
+
         if (NULL == (snapshot->super.metadata = fopen(snapshot->super.metadata_filename, "r")) ) {
             opal_output(mca_crs_blcr_component.super.output_handle,
                         "crs:blcr: checkpoint(): Error: Unable to open the file (%s)",

diff --git a/opal/mca/crs/dmtcp/crs_dmtcp_module.c b/opal/mca/crs/dmtcp/crs_dmtcp_module.c
@@ -477,13 +477,14 @@ static int dmtcp_cold_start(opal_crs_dmtcp_snapshot_t *snapshot) {
     char **tmp_argv = NULL;
     char * component_name = NULL;
     int prev_pid;
-
+    char *smfname;
     /*
      * Find the snapshot directory, read the metadata file for
      * component name and previous pid
      */
     if( NULL == snapshot->super.metadata ) {
-        if (NULL == (snapshot->super.metadata = fopen(snapshot->super.metadata_filename, "r")) ) {
+        smfname = snapshot->super.metadata_filename;
+        if (!access(smfname, F_OK) || NULL == (snapshot->super.metadata = fopen(smfname, "r")) ) {
             opal_output(mca_crs_dmtcp_component.super.output_handle,
                         "crs:dmtcp: dmtcp_cold_start(): Error: Unable to open the file (%s)",
                         snapshot->super.metadata_filename);

diff --git a/opal/mca/if/linux_ipv6/if_linux_ipv6.c b/opal/mca/if/linux_ipv6/if_linux_ipv6.c
@@ -95,7 +95,8 @@ static int if_linux_ipv6_open(void)
 {
 #if OPAL_ENABLE_IPV6
     FILE *f;
-    if ((f = fopen("/proc/net/if_inet6", "r"))) {
+    if (access("/proc/net/if_inet6", F_OK) &&
+        (f = fopen("/proc/net/if_inet6", "r"))) {
         char ifname[IF_NAMESIZE];
         unsigned int idx, pfxlen, scope, dadstat;
         struct in6_addr a6;

diff --git a/opal/mca/pmix/cray/pmix_cray_component.c b/opal/mca/pmix/cray/pmix_cray_component.c
@@ -114,16 +114,18 @@ static int pmix_cray_component_query(mca_base_module_t **module, int *priority)
 
     /* disqualify ourselves if not running in a Cray PAGG container, or we
        were launched by the orte/mpirun launcher */
-    fd = fopen(proc_job_file, "r");
-    if ((fd == NULL) || (getenv("OMPI_NO_USE_CRAY_PMI") != NULL)) {
+    if (!access(proc_job_file, F_OK) ||
+        fd = fopen(proc_job_file, "r") == NULL ||
+        (getenv("OMPI_NO_USE_CRAY_PMI") != NULL)) {
         *priority = 0;
         *module = NULL;
         rc = OPAL_ERROR;
     } else {
         snprintf(task_is_app_fname,sizeof(task_is_app_fname),
                  "/proc/self/task/%ld/task_is_app",syscall(SYS_gettid));
-        fd_task_is_app = fopen(task_is_app_fname, "r");
-        if (fd_task_is_app != NULL) {   /* okay we're in a PAGG container,
+        if(access(fd_task_is_app, F_OK) &&
+           NULL != fd_task_is_app = fopen(task_is_app_fname, "r") {
+                                        /* okay we're in a PAGG container,
                                            and we are an app task (not just a process
                                            running on a mom node, for example),
                                            so we should give cray pmi a shot. */

diff --git a/opal/mca/pmix/pmix3x/pmix/contrib/perf_tools/pmi_intra_perf.c b/opal/mca/pmix/pmix3x/pmix/contrib/perf_tools/pmi_intra_perf.c
@@ -129,7 +129,7 @@ int get_mem_usage(double *_pss, double *_rss) {
     memset(data, 0, sizeof(data));
     snprintf(data, sizeof(data), "/proc/%d/smaps", pid);
 
-    if (NULL == (smaps = fopen(data, "r"))) {
+    if (!access(data, F_OK) || NULL == (smaps = fopen(data, "r"))) {
         return -1;
     }
 

diff --git a/opal/mca/pmix/pmix3x/pmix/src/mca/pif/linux_ipv6/pif_linux_ipv6.c b/opal/mca/pmix/pmix3x/pmix/src/mca/pif/linux_ipv6/pif_linux_ipv6.c
@@ -94,7 +94,7 @@ pmix_pif_base_component_t mca_pif_linux_ipv6_component = {
 static int if_linux_ipv6_open(void)
 {
     FILE *f;
-    if ((f = fopen("/proc/net/if_inet6", "r"))) {
+    if (access("/proc/net/if_inet6", F_OK) && NULL != (f = fopen("/proc/net/if_inet6", "r"))) {
         char ifname[IF_NAMESIZE];
         unsigned int idx, pfxlen, scope, dadstat;
         struct in6_addr a6;

diff --git a/opal/mca/pmix/pmix3x/pmix/src/mca/ptl/tcp/ptl_tcp.c b/opal/mca/pmix/pmix3x/pmix/src/mca/ptl/tcp/ptl_tcp.c
@@ -471,8 +471,7 @@ static pmix_status_t parse_uri_file(char *filename,
     struct timeval tv;
     int retries;
 
-    fp = fopen(filename, "r");
-    if (NULL == fp) {
+    if (!access(filename, F_OK) || NULL == (fp = fopen(filename, "r"))) {
         /* if we cannot open the file, then the server must not
          * be configured to support tool connections, or this
          * user isn't authorized to access it - or it may just
@@ -495,8 +494,7 @@ static pmix_status_t parse_uri_file(char *filename,
                     pmix_event_evtimer_add(&ev, &tv);
                     PMIX_WAIT_THREAD(&lock);
                     PMIX_DESTRUCT_LOCK(&lock);
-                    fp = fopen(filename, "r");
-                    if (NULL != fp) {
+                    if (access(filename, F_OK) && NULL != (fp = fopen(filename, "r"))) {
                         /* we found it! */
                         goto process;
                     }

diff --git a/opal/mca/pmix/pmix3x/pmix/src/util/keyval_parse.c b/opal/mca/pmix/pmix3x/pmix/src/util/keyval_parse.c
@@ -28,6 +28,7 @@
 #include "src/util/output.h"
 #include <string.h>
 #include <ctype.h>
+#include <unistd.h>
 
 int pmix_util_keyval_parse_lineno = 0;
 
@@ -70,8 +71,7 @@ pmix_util_keyval_parse(const char *filename,
     keyval_callback = callback;
 
     /* Open the pmix */
-    pmix_util_keyval_yyin = fopen(keyval_filename, "r");
-    if (NULL == pmix_util_keyval_yyin) {
+    if (!access(keyval_filename, F_OK) || NULL == (pmix_util_keyval_yyin = fopen(keyval_filename, "r"))) {
         ret = PMIX_ERR_NOT_FOUND;
         goto cleanup;
     }

diff --git a/opal/mca/pmix/pmix3x/pmix/src/util/show_help.c b/opal/mca/pmix/pmix3x/pmix/src/util/show_help.c
@@ -26,6 +26,7 @@
 #include <string.h>
 #include <locale.h>
 #include <errno.h>
+#include <unistd.h>
 
 #include "src/mca/pinstalldirs/pinstalldirs.h"
 #include "src/util/show_help.h"
@@ -170,6 +171,9 @@ static int open_file(const char *base, const char *topic)
                     if (0 > asprintf(&filename, "%s%s%s.txt", search_dirs[i], PMIX_PATH_SEP, base)) {
                         return PMIX_ERR_OUT_OF_RESOURCE;
                     }
+                    if(!access(filename, F_OK)){
+                      return PMIX_ERR_NOT_FOUND;
+                    }
                     pmix_show_help_yyin = fopen(filename, "r");
                 }
             }

diff --git a/opal/mca/pstat/linux/pstat_linux_module.c b/opal/mca/pstat/linux/pstat_linux_module.c
@@ -289,7 +289,7 @@ static int query(pid_t pid,
             return OPAL_ERR_VALUE_OUT_OF_BOUNDS;
         }
 
-        if (NULL == (fp = fopen(data, "r"))) {
+        if (!access(data, F_OK) || NULL == (fp = fopen(data, "r"))) {
             /* ignore this */
             return OPAL_SUCCESS;
         }
@@ -318,7 +318,7 @@ static int query(pid_t pid,
             return OPAL_ERR_VALUE_OUT_OF_BOUNDS;
         }
 
-        if (NULL == (fp = fopen(data, "r"))) {
+        if (!access(data, F_OK) || NULL == (fp = fopen(data, "r"))) {
             /* ignore this */
             return OPAL_SUCCESS;
         }
@@ -365,7 +365,7 @@ static int query(pid_t pid,
         nstats->la15 = strtof(eptr, NULL);
 
         /* see if we can open the meminfo file */
-        if (NULL == (fp = fopen("/proc/meminfo", "r"))) {
+        if (!access("/proc/meminfo", F_OK) || NULL == (fp = fopen("/proc/meminfo", "r"))) {
             /* ignore this */
             goto diskstats;
         }
@@ -398,7 +398,7 @@ static int query(pid_t pid,
 
     diskstats:
         /* look for the diskstats file */
-        if (NULL == (fp = fopen("/proc/diskstats", "r"))) {
+        if (!access("/proc/diskstats", F_OK) || NULL == (fp = fopen("/proc/diskstats", "r"))) {
             /* not an error if we don't find this one as it
              * isn't critical
              */
@@ -441,7 +441,7 @@ static int query(pid_t pid,
 
     netstats:
         /* look for the netstats file */
-        if (NULL == (fp = fopen("/proc/net/dev", "r"))) {
+        if (!access("/proc/net/dev", F_OK) || NULL == (fp = fopen("/proc/net/dev", "r"))) {
             /* not an error if we don't find this one as it
              * isn't critical
              */

diff --git a/opal/mca/timer/linux/timer_linux_component.c b/opal/mca/timer/linux/timer_linux_component.c
@@ -26,6 +26,7 @@
 #include "opal_config.h"
 
 #include <string.h>
+#include <unistd.h>
 
 #include "opal/mca/timer/timer.h"
 #include "opal/mca/timer/base/base.h"
@@ -110,6 +111,9 @@ static int opal_timer_linux_find_freq(void)
     int ret;
     char buf[1024];
 
+    if(!access("/proc/cpuinfo", F_OK)) {
+        return OPAL_ERR_IN_ERRNO;
+    }
     fp = fopen("/proc/cpuinfo", "r");
     if (NULL == fp) {
         return OPAL_ERR_IN_ERRNO;

diff --git a/opal/runtime/opal_cr.c b/opal/runtime/opal_cr.c
@@ -987,7 +987,7 @@ static int extract_env_vars(int prev_pid, char * file_name)
         goto cleanup;
     }
 
-    if (NULL == (env_data = fopen(file_name, "r")) ) {
+    if (!access(env_data, F_OK) || NULL == (env_data = fopen(file_name, "r")) ) {
         exit_status = OPAL_ERROR;
         goto cleanup;
     }

diff --git a/opal/tools/opal-restart/opal-restart.c b/opal/tools/opal-restart/opal-restart.c
@@ -214,7 +214,7 @@ main(int argc, char *argv[])
                  opal_restart_globals.snapshot_loc,
                  opal_restart_globals.snapshot_ref,
                  opal_restart_globals.snapshot_metadata);
-        if( NULL == (metadata = fopen(full_metadata_path, "r")) ) {
+        if(!access(full_metadata_path, F_OK) || NULL == (metadata = fopen(full_metadata_path, "r")) ) {
             opal_show_help("help-opal-restart.txt", "invalid_metadata", true,
                            opal_restart_globals.snapshot_metadata,
                            full_metadata_path);
@@ -657,7 +657,7 @@ static int post_env_vars(int prev_pid, opal_crs_base_snapshot_t *snapshot)
     /*
      * Any directories that need to be created
      */
-    if( NULL == (snapshot->metadata = fopen(snapshot->metadata_filename, "r")) ) {
+    if(!access(snapshot->metadata_filename, F_OK) || NULL == (snapshot->metadata = fopen(snapshot->metadata_filename, "r")) ) {
         opal_show_help("help-opal-restart.txt", "invalid_metadata", true,
                        opal_restart_globals.snapshot_metadata,
                        snapshot->metadata_filename);

diff --git a/opal/util/keyval_parse.c b/opal/util/keyval_parse.c
@@ -28,6 +28,7 @@
 #include "opal/threads/mutex.h"
 #include <string.h>
 #include <ctype.h>
+#include <unistd.h>
 
 int opal_util_keyval_parse_lineno = 0;
 
@@ -76,6 +77,10 @@ opal_util_keyval_parse(const char *filename,
     keyval_filename = filename;
     keyval_callback = callback;
 
+    if (!access(keyval_filename, F_OK)) {
+        ret = OPAL_ERR_NOT_FOUND;
+        goto cleanup;
+    }
     /* Open the opal */
     opal_util_keyval_yyin = fopen(keyval_filename, "r");
     if (NULL == opal_util_keyval_yyin) {

diff --git a/opal/util/show_help.c b/opal/util/show_help.c
@@ -25,6 +25,7 @@
 #include <string.h>
 #include <locale.h>
 #include <errno.h>
+#include <unistd.h>
 
 #include "opal/mca/installdirs/installdirs.h"
 #include "opal/util/show_help.h"
@@ -158,13 +159,20 @@ static int open_file(const char *base, const char *topic)
          */
         for (i=0; NULL != search_dirs[i]; i++) {
             filename = opal_os_path( false, search_dirs[i], base, NULL );
+            if(!access(filename, F_OK)) {
+              goto doesntexist;
+            }
             opal_show_help_yyin = fopen(filename, "r");
             if (NULL == opal_show_help_yyin) {
+            doesntexist:
                 asprintf(&err_msg, "%s: %s", filename, strerror(errno));
                 base_len = strlen(base);
                 if (4 > base_len || 0 != strcmp(base + base_len - 4, ".txt")) {
                     free(filename);
                     asprintf(&filename, "%s%s%s.txt", search_dirs[i], OPAL_PATH_SEP, base);
+                    if(!access(filename, F_OK)) {
+                      break;
+                    }
                     opal_show_help_yyin = fopen(filename, "r");
                 }
             }