View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0009306 | GNUnet | util library | public | 2024-11-01 12:03 | 2024-12-09 17:03 |
| Reporter | fefe | Assigned To | schanzen | Due Date | |
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | closed | Resolution | fixed | ||
| Target Version | 0.23.0 | Fixed in Version | 0.23.0 | ||
| Summary | 0009306: GNUNET_DISK_file_backup: filesystem race condition | ||||
| Description | 369 do 370 { 371 GNUNET_snprintf (target, slen, "%s.%u~", fil, num++); 372 } 373 while (0 == access (target, F_OK)); 374 if (0 != rename (fil, target)) 375 GNUNET_log_strerror_file (GNUNET_ERROR_TYPE_ERROR, "rename", fil); Never use access. It is inherently racy. If two copies of this code run in parallel, both call access to see the target name is free, then copy A runs and does the rename, returns to the caller, which creates a new file under the name. Then copy B gets to run and renames the new file under the old name to the backup name, destroying the backup. There are some ways to tackle this. One would be to use link instead of rename. link fails if the target name already exists. However you would have to unlink the old name afterwards, which introduces a new race. Linux added a way to do it cleanly: renameat2(). That function takes a flags argument and defines RENAME_NOREPLACE as a possible flag value. Or, even better, it has a flag called RENAME_EXCHANGE. That allows a completely race-free implementation of "back up old file and create new file": You use open with O_EXCL to create a new file under the backup name, then use renameat2 to switch it with the old name. That way if two processes run this at the same time, no data would be lost. | ||||
| Tags | No tags attached. | ||||
| It is very unlikely that two processes under the same user access the files in question at the same time. We can, of course, use renameat2, if available. | |
| Can you review https://git.gnunet.org/gnunet.git/diff/src/lib/util/disk.c?id=bf84274b70903f9ab65238c4615e291980270cc0 if that is what you meant? Thanks EDIT: This implementation with renameat2 does not unlik the old file, however, which seems wrong to me? | |
| Reopen if necessary | |
| Released | |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2024-11-01 12:03 | fefe | New Issue | |
| 2024-11-03 19:34 | schanzen | Target Version | => 0.22.3 |
| 2024-11-04 09:55 | schanzen | Note Added: 0023653 | |
| 2024-11-04 09:59 | schanzen | Assigned To | => schanzen |
| 2024-11-04 09:59 | schanzen | Status | new => confirmed |
| 2024-11-07 11:38 | schanzen | ||
| 2024-11-07 11:38 | schanzen | Note Added: 0023673 | |
| 2024-11-07 11:39 | schanzen | Status | confirmed => feedback |
| 2024-11-07 12:00 | schanzen | Note Edited: 0023673 | |
| 2024-11-14 08:43 | schanzen | Target Version | 0.22.3 => 0.23.0 |
| 2024-11-26 19:53 | schanzen | Status | feedback => resolved |
| 2024-11-26 19:53 | schanzen | Resolution | open => fixed |
| 2024-11-26 19:53 | schanzen | Note Added: 0023756 | |
| 2024-12-09 17:03 | schanzen | Fixed in Version | => 0.23.0 |
| 2024-12-09 17:03 | schanzen | Note Added: 0023840 | |
| 2024-12-09 17:03 | schanzen | Status | resolved => closed |