Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

common/cryptfs: Adapt integrity dev formatting #521

Merged
merged 6 commits into from
Feb 25, 2025

Conversation

k0ch4lo
Copy link
Member

@k0ch4lo k0ch4lo commented Feb 12, 2025

This PR leverages a buffer allocated via calloc's copy-on-write-based approach together with the fd_write implementation in common/fd.c implementation to increase the speed of initial formatting while not requiring a large amount of physical memory.

@k0ch4lo k0ch4lo force-pushed the cryptfs_zero_calloc branch 3 times, most recently from c8d1d4f to 5dd1b9e Compare February 18, 2025 17:24
@k0ch4lo k0ch4lo force-pushed the cryptfs_zero_calloc branch 2 times, most recently from a10403d to f57ae88 Compare February 20, 2025 07:39
@k0ch4lo k0ch4lo requested a review from MPeisl February 20, 2025 07:40
common/cryptfs.c Outdated
close(fd);

if (0 != cryptfs_write_zeros(crypto_blkdev, fs_size * 512)) {
WARN("Failed to format volume %s using calloc, falling back to sendfile",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change this from sendfile to file_copy. file_copy will also use its fallback, as /dev/zero is not support by sendfile ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file_copy needs to be adapted for use with /dev/zero in future commits, switched to existing implementation as fallback

common/cryptfs.c Outdated
WARN("Failed to format volume %s using calloc, falling back to sendfile",
crypto_blkdev);

if (0 > file_copy("/dev/zero", crypto_blkdev, fs_size * 512, fs_size * 512,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be file_copy("/dev/zero", crypto_blkdev, count: fs_size, bs: 512, 0)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above, switched to existing implementation as fallback

common/cryptfs.c Outdated

if (0 > file_copy("/dev/zero", crypto_blkdev, fs_size * 512, fs_size * 512,
0)) {
ERROR("Failed to format volume %s using sendfile", crypto_blkdev);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s.o.

@k0ch4lo k0ch4lo force-pushed the cryptfs_zero_calloc branch 10 times, most recently from ed8fd88 to 21fb254 Compare February 21, 2025 23:13
daemon/main.c Outdated

if (!cmld_is_hostedmode_active()) {
SYNC_INFO()
}
Copy link
Member

@quitschbo quitschbo Feb 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

provide an atexit() handler and put this
block into that wrapper. Otherwise, sync is done before all containers have been stopped
and not at termination of cmld. Additionally, remove all other SYNC_INFO in signal handlers and main().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good, switched to atexit

scd/scd.c Outdated
@@ -80,6 +80,8 @@ scd_sigterm_cb(UNUSED int signum, UNUSED event_signal_t *sig, UNUSED void *data)
logf_unregister(scd_logfile_handler);
logf_file_close(scd_logfile_p);
}

SYNC_INFO()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as for cmld, just register an atexit handler

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@k0ch4lo k0ch4lo force-pushed the cryptfs_zero_calloc branch from 21fb254 to 48a1789 Compare February 25, 2025 06:50
This commit leverages a buffer allocated via calloc's copy-on-write-based approach
together with the fd_write implementation in common/fd.c implementation to increase
the speed of initial formatting while not requiring a large amount of physical memory.

Signed-off-by: Felix Wruck <[email protected]>
This commit adds an attempt to sync fds after write operatins via fsync.
As fsync is not possible on all file types, e.g. on procfs pseudo files,
failed attempts to rsync are not treated as error.

Signed-off-by: Felix Wruck <[email protected]>
This commit adapts the the fd_write function in common/fd.{.c,.h} to use ssize_t
consistently for input parameters and the return value to prevent overflows.

Signed-off-by: Felix Wruck <[email protected]>
@k0ch4lo k0ch4lo force-pushed the cryptfs_zero_calloc branch 3 times, most recently from 70a1970 to 59c0a79 Compare February 25, 2025 07:07
This commit syncs file systems before the cmld exists to ensure all
pending file system operations are properly completed.

Signed-off-by: Felix Wruck <[email protected]>
This commit syncs file systems before the cmld exists to ensure all
pending file system operations are properly completed.

Signed-off-by: Felix Wruck <[email protected]>
This commit adds proper checks for the return value of fsync.

Signed-off-by: Felix Wruck <[email protected]>
@k0ch4lo k0ch4lo force-pushed the cryptfs_zero_calloc branch from 59c0a79 to 6ec974a Compare February 25, 2025 07:12
@quitschbo quitschbo merged commit c0d878a into gyroidos:main Feb 25, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants