View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0009295 | GNUnet | util library | public | 2024-10-23 14:28 | 2024-10-23 21:12 |
| Reporter | fefe | Assigned To | |||
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | new | Resolution | open | ||
| Summary | 0009295: GNUNET_DISK_file_test is inherently racy | ||||
| Description | This function is called, for example, from GNUNET_CONFIGURATION_default. It will call stat() on the filename and then access. The access man page explicitly warns of using it because that would be a race condition. One of the checks is to make sure it's a regular file. Here is the code from GNUNET_CONFIGURATION_default: 2501 /* First, try user configuration. */ 2502 if (NULL != xdg) 2503 GNUNET_asprintf (&cfgname, 2504 "%s/%s", 2505 xdg, 2506 dpd->config_file); 2507 else 2508 cfgname = GNUNET_strdup (dpd->user_config_file); 2509 2510 /* If user config doesn't exist, try in 2511 /etc/<projdir>/<cfgfile> and /etc/<cfgfile> */ 2512 if (GNUNET_OK != GNUNET_DISK_file_test (cfgname)) 2513 { 2514 GNUNET_free (cfgname); 2515 GNUNET_asprintf (&cfgname, 2516 "/etc/%s", 2517 dpd->config_file); 2518 } 2519 if (GNUNET_OK != GNUNET_DISK_file_test (cfgname)) 2520 { 2521 GNUNET_free (cfgname); 2522 GNUNET_asprintf (&cfgname, 2523 "/etc/%s/%s", 2524 dpd->project_dirname, 2525 dpd->config_file); 2526 } 2527 if (GNUNET_OK != GNUNET_DISK_file_test (cfgname)) 2528 { 2529 LOG (GNUNET_ERROR_TYPE_ERROR, 2530 "Unable to top-level configuration file.\n"); 2531 GNUNET_OS_init (pd); 2532 GNUNET_CONFIGURATION_destroy (cfg); 2533 GNUNET_free (cfgname); 2534 return NULL; 2535 } Note that this does the same stat test multiple times. Also all the stat tests are worthless if the filesystem changes between the access and the open. Here is the open: 2537 /* We found a configuration file that looks good, try to load it. */ 2538 2539 LOG (GNUNET_ERROR_TYPE_DEBUG, 2540 "Loading top-level configuration from '%s'\n", 2541 cfgname); 2542 if (GNUNET_OK != 2543 GNUNET_CONFIGURATION_load (cfg, cfgname)) The preferred method is to call open directly and then use fstat to check we opened a regular file. | ||||
| Tags | No tags attached. | ||||
|
|
Yeah we obviously call stat way too often. And fstat is missing from the open API. |
|
|
Ok I took a jab at this. GNUNET_DISK_file_test is not inherently racy. It is just unreliable when used in combination with open() like this. It also has other uses. GNUNET_CONFIGURATION_parse uses GNUNET_DISK_fn_read which does not return a file descriptor and can be used with non-regular files as well (e.g. device files). So, this would require rewriting GNUNET_CONFIGURATION_parse to use another API. We may want to check all uses of GNUNET_DISK_fn_read, but I don't think this is a critical bug. Leaving this open for now. |
