Improve config file sync during configuration save.
Add fsync() calls to force sync to disk while saving configuration
file. It is necessary to do a sync on the temp file before rename,
and to sync again on its parent directory to ensure both the
file content and the directory are up-to-date.
Bug: 27354612
Change-Id: I3a862ad59c8ae5beb8ea2c727eb5f275a2d5823e
(cherry picked from commit 95dbe03a693f4a920204b8c6acbba5269915a59f)
diff --git a/btif/src/btif_config.c b/btif/src/btif_config.c
index f2a8806..24a8c17 100644
--- a/btif/src/btif_config.c
+++ b/btif/src/btif_config.c
@@ -489,12 +489,10 @@
pthread_mutex_lock(&lock);
rename(CONFIG_FILE_PATH, CONFIG_BACKUP_PATH);
- sync();
config_t *config_paired = config_new_clone(config);
btif_config_remove_unpaired(config_paired);
config_save(config_paired, CONFIG_FILE_PATH);
config_free(config_paired);
- sync();
pthread_mutex_unlock(&lock);
}
diff --git a/osi/src/config.c b/osi/src/config.c
index 9683de2..345f907 100644
--- a/osi/src/config.c
+++ b/osi/src/config.c
@@ -23,6 +23,8 @@
#include <assert.h>
#include <ctype.h>
#include <errno.h>
+#include <fcntl.h>
+#include <libgen.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -278,12 +280,39 @@
assert(filename != NULL);
assert(*filename != '\0');
- char *temp_filename = osi_calloc(strlen(filename) + 5);
+ // Steps to ensure content of config file gets to disk:
+ //
+ // 1) Open and write to temp file (e.g. bt_config.conf.new).
+ // 2) Sync the temp file to disk with fsync().
+ // 3) Rename temp file to actual config file (e.g. bt_config.conf).
+ // This ensures atomic update.
+ // 4) Sync directory that has the conf file with fsync().
+ // This ensures directory entries are up-to-date.
+ int dir_fd = -1;
+ FILE *fp = NULL;
- strcpy(temp_filename, filename);
- strcat(temp_filename, ".new");
+ // Build temp config file based on config file (e.g. bt_config.conf.new).
+ static const char *temp_file_ext = ".new";
+ const int filename_len = strlen(filename);
+ const int temp_filename_len = filename_len + strlen(temp_file_ext) + 1;
+ char *temp_filename = osi_calloc(temp_filename_len);
+ snprintf(temp_filename, temp_filename_len, "%s%s", filename, temp_file_ext);
- FILE *fp = fopen(temp_filename, "wt");
+ // Extract directory from file path (e.g. /data/misc/bluedroid).
+ char *temp_dirname = osi_strdup(filename);
+ const char *directoryname = dirname(temp_dirname);
+ if (!directoryname) {
+ LOG_ERROR(LOG_TAG, "%s error extracting directory from '%s': %s", __func__, filename, strerror(errno));
+ goto error;
+ }
+
+ dir_fd = open(directoryname, O_RDONLY);
+ if (dir_fd < 0) {
+ LOG_ERROR(LOG_TAG, "%s unable to open dir '%s': %s", __func__, directoryname, strerror(errno));
+ goto error;
+ }
+
+ fp = fopen(temp_filename, "wt");
if (!fp) {
LOG_ERROR(LOG_TAG, "%s unable to write file '%s': %s", __func__, temp_filename, strerror(errno));
goto error;
@@ -313,9 +342,13 @@
}
}
+ // Sync written temp file out to disk. fsync() is blocking until data makes it to disk.
+ if (fsync(fileno(fp)) < 0) {
+ LOG_WARN(LOG_TAG, "%s unable to fsync file '%s': %s", __func__, temp_filename, strerror(errno));
+ }
+
if (fclose(fp) == EOF) {
LOG_ERROR(LOG_TAG, "%s unable to close file '%s': %s", __func__, temp_filename, strerror(errno));
- fp = NULL;
goto error;
}
fp = NULL;
@@ -326,19 +359,35 @@
goto error;
}
+ // Rename written temp file to the actual config file.
if (rename(temp_filename, filename) == -1) {
LOG_ERROR(LOG_TAG, "%s unable to commit file '%s': %s", __func__, filename, strerror(errno));
goto error;
}
+ // This should ensure the directory is updated as well.
+ if (fsync(dir_fd) < 0) {
+ LOG_WARN(LOG_TAG, "%s unable to fsync dir '%s': %s", __func__, directoryname, strerror(errno));
+ }
+
+ if (close(dir_fd) < 0) {
+ LOG_ERROR(LOG_TAG, "%s unable to close dir '%s': %s", __func__, directoryname, strerror(errno));
+ goto error;
+ }
+
osi_free(temp_filename);
+ osi_free(temp_dirname);
return true;
-error:;
- if (fp != NULL)
- fclose(fp);
+error:
+ // This indicates there is a write issue. Unlink as partial data is not acceptable.
unlink(temp_filename);
+ if (fp)
+ fclose(fp);
+ if (dir_fd != -1)
+ close(dir_fd);
osi_free(temp_filename);
+ osi_free(temp_dirname);
return false;
}