[PATCH] Mount config partition with MS_SYNCHRONOUS

[Christian Storm]

From: Christian Storm <christi...@siemens.com>

Currently, flushing a written EBGENV to disk is either done
on unmounting the configuration partition or left to the
kernel's determination, both leaving a time window while
which a power cut may cause data loss (or a corrupted
filesystem which is somewhat mitigated by redundancy).

Hence, shorten this window of vulnerability by mounting the
config partitions MS_SYNCHRONOUS which is identical to the
O_SYNC flag supplied to all open(2) calls:
"O_SYNC provides synchronized I/O file integrity completion,
meaning write operations will flush data and all associated
metadata to the underlying hardware."

Note that this applies to configuration partitions mounted by
EFI Boot Guard tools only. Configuration partitions mounted
by other means should specify -o sync when mounting.

While at it, change mount()'s data parameter to NULL as there
are no filesystem-specific options.

Signed-off-by: Christian Storm <christi...@siemens.com>
---
docs/TOOLS.md | 9 ++++++---
env/env_disk_utils.c | 2 +-
2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/docs/TOOLS.md b/docs/TOOLS.md
index 2b66aeb..940bbf8 100644
--- a/docs/TOOLS.md
+++ b/docs/TOOLS.md
@@ -18,9 +18,12 @@ Generation of a valid configuration partition is described in

*NOTE*: To access configuration data on FAT partitions, the partition must
either already be mounted, with access rights for the user using the tool, or
-the tool can mount the partition by itself. The latter is only possible if the
-tool has the `CAP_SYS_ADMIN` capability. This is the case if the user is `root`
-or the corresponding capability is set in the filesystem.
+the tool can mount the partition by itself. In the former case, it's advised
+to mount with `-o sync` to flush written data and associated metadata to the
+underlying hardware immediately so to reduce data loss probability on power
+cuts. The latter is only possible if the tool has the `CAP_SYS_ADMIN`
+capability. This is the case if the user is `root` or the corresponding
+capability is set in the filesystem.

## Updating a configuration ##

diff --git a/env/env_disk_utils.c b/env/env_disk_utils.c
index cb70407..e7fd4a1 100644
--- a/env/env_disk_utils.c
+++ b/env/env_disk_utils.c
@@ -58,7 +58,7 @@ bool mount_partition(CONFIG_PART *cfgpart)
VERBOSE(stderr, "Error creating temporary mount point.\n");
return false;
}
- if (mount(cfgpart->devpath, mountpoint, "vfat", 0, "")) {
+ if (mount(cfgpart->devpath, mountpoint, "vfat", MS_SYNCHRONOUS, NULL)) {
VERBOSE(stderr, "Error mounting to temporary mount point.\n");
if (rmdir(tmpdir_template)) {
VERBOSE(stderr,
--
2.40.1

[Jan Kiszka]

Thanks, applied.

Was this case observed in reality? Even then, it should at most have
ruined the day for the new boot path, not for the past working one, right?

Jan

--
Siemens AG, Technology
Competence Center Embedded Linux

[Christian Storm]

Hi Jan,

Not in the field reality but in my test setup reality :)

> Even then, it should at most have ruined the day for the new boot
> path, not for the past working one, right?

Yes, as long as there are no writes to both for whatever reason and the
power cut is done prior to the kernel having sync'd down to the platters.
I was playing around with such scenarios and this helped to reduce scrap.


Kind regards,
Christian

--
Dr. Christian Storm
Siemens AG, Technology, T CED SES-DE
Otto-Hahn-Ring 6, 81739 München, Germany