View Issue Details

IDProjectCategoryView StatusLast Update
0009306GNUnetutil librarypublic2024-12-09 18:03
Reporterfefe Assigned Toschanzen  
PrioritynormalSeverityminorReproducibilityhave not tried
Status closedResolutionfixed 
Target Version0.23.0Fixed in Version0.23.0 
Summary0009306: 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.
TagsNo tags attached.

Activities

schanzen

2024-11-04 10:55

administrator   ~0023653

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.

schanzen

2024-11-07 12:38

administrator   ~0023673

Last edited: 2024-11-07 13:00

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?

schanzen

2024-11-26 20:53

administrator   ~0023756

Reopen if necessary

schanzen

2024-12-09 18:03

administrator   ~0023840

Released

Related Changesets

gnunet: master bf84274b

2024-11-07 13:37

schanzen


Details Diff
util: Attempt to fix issue 0009306 Affected Issues
0009306
mod - configure.ac Diff File
mod - meson.build Diff File
mod - src/include/gnunet_disk_lib.h Diff File
mod - src/lib/util/disk.c Diff File
mod - src/lib/util/test_bio.c Diff File

Issue History

Date Modified Username Field Change
2024-11-01 13:03 fefe New Issue
2024-11-03 20:34 schanzen Target Version => 0.22.3
2024-11-04 10:55 schanzen Note Added: 0023653
2024-11-04 10:59 schanzen Assigned To => schanzen
2024-11-04 10:59 schanzen Status new => confirmed
2024-11-07 12:38 schanzen Changeset attached => gnunet master bf84274b
2024-11-07 12:38 schanzen Note Added: 0023673
2024-11-07 12:39 schanzen Status confirmed => feedback
2024-11-07 13:00 schanzen Note Edited: 0023673
2024-11-14 09:43 schanzen Target Version 0.22.3 => 0.23.0
2024-11-26 20:53 schanzen Status feedback => resolved
2024-11-26 20:53 schanzen Resolution open => fixed
2024-11-26 20:53 schanzen Note Added: 0023756
2024-12-09 18:03 schanzen Fixed in Version => 0.23.0
2024-12-09 18:03 schanzen Note Added: 0023840
2024-12-09 18:03 schanzen Status resolved => closed