fix(p7-output-fs): address c-review (fake-super mode sanitization HIGH; sparse/preallocate precedence; partial no_replace guard; stronger tests)
- fake_super_restore_fd now sanitizes mode like metadata_mode (never grants S_IWGRP|S_IWOTH; 0666 -> 0644), fixing a privilege regression - --sparse takes precedence over --preallocate (skip posix_fallocate when sparse) so holes are not re-allocated; docs corrected - --partial retention disabled under --no_replace (ignore/existing) and only marks write_attempted after the write begins (no empty-temp retention) - accept --block-size=SIZE / --delta-block=SIZE inline forms; neutral messages - fake-super EPERM/EACCES skipped silently (docs aligned); EINVAL still logged - sparse unit test now memcmp's the full buffer; TestBlockSize integration keeps the destination basis so delta is genuinely exercised - unit 37/37, cppcheck 0, clang-format 0
This commit is contained in:
+16
-7
@@ -882,9 +882,12 @@ static bool file_to_disk_secure_impl(const char* path, const void* data,
|
||||
ok = true;
|
||||
} else {
|
||||
/* Preallocate the expected payload size before writing so an
|
||||
out-of-space condition fails cleanly up front (--preallocate). */
|
||||
out-of-space condition fails cleanly up front (--preallocate).
|
||||
--sparse takes precedence: posix_fallocate would allocate every
|
||||
block, defeating the holes the sparse writer would create, so the
|
||||
two never combine here (the ftruncate presize below stays). */
|
||||
int prealloc_rc = 0;
|
||||
if (preallocate && data_size > 0) {
|
||||
if (preallocate && !sparse && data_size > 0) {
|
||||
prealloc_rc = preallocate_fd(fd, data_size);
|
||||
if (prealloc_rc != 0)
|
||||
log_message(LOG_LEVEL_ERROR, "preallocate failed for '%s' (%s); transfer aborted", path,
|
||||
@@ -991,20 +994,24 @@ static bool file_to_disk_secure_impl(const char* path, const void* data,
|
||||
if (fd < 0)
|
||||
continue; /* EEXIST (or a transient open error): try a fresh name. */
|
||||
int prealloc_rc = 0;
|
||||
if (preallocate && data_size > 0) {
|
||||
if (preallocate && !sparse && data_size > 0) {
|
||||
prealloc_rc = preallocate_fd(fd, data_size);
|
||||
if (prealloc_rc != 0)
|
||||
log_message(LOG_LEVEL_ERROR, "preallocate failed for '%s' (%s); transfer aborted", path,
|
||||
strerror(prealloc_rc));
|
||||
}
|
||||
if (prealloc_rc == 0) {
|
||||
write_attempted = true;
|
||||
lseek(fd, 0, SEEK_SET);
|
||||
if (sparse && data_size > 0)
|
||||
ok = ftruncate(fd, (off_t)data_size) == 0;
|
||||
if (ok || (!sparse || data_size == 0))
|
||||
/* A real write attempt begins here (the ftruncate presize succeeded or
|
||||
no presize applies): a later mid-write / metadata / fsync / install
|
||||
failure may leave partial data that --partial retention can rename. */
|
||||
if (ok || (!sparse || data_size == 0)) {
|
||||
write_attempted = true;
|
||||
ok = sparse && data_size > 0 ? write_all_sparse(fd, (const unsigned char*)data, data_size)
|
||||
: write_all(fd, data, data_size);
|
||||
}
|
||||
if (ok && metadata)
|
||||
ok = file_restore_metadata_fd(fd, metadata, preserve_executability);
|
||||
if (ok)
|
||||
@@ -1047,8 +1054,10 @@ static bool file_to_disk_secure_impl(const char* path, const void* data,
|
||||
This only ever renames the already-written temp (never a corrupt
|
||||
blend); the rename can fail (cross-device, permissions) and we then
|
||||
fall through to the normal unlink cleanup. Never retains when
|
||||
keep_partial is off. */
|
||||
if (!keep_partial || !write_attempted ||
|
||||
keep_partial is off, when nothing was actually written, or under
|
||||
--ignore-existing/--existing (no_replace), where the destination is
|
||||
not ours to overwrite. */
|
||||
if (!keep_partial || !write_attempted || no_replace ||
|
||||
renameat(scratch_dirfd >= 0 ? scratch_dirfd : dirfd, tmp, dirfd, leaf) != 0)
|
||||
unlinkat(scratch_dirfd >= 0 ? scratch_dirfd : dirfd, tmp, 0);
|
||||
}
|
||||
|
||||
+11
-5
@@ -352,13 +352,19 @@ bool fake_super_restore_fd(int fd) {
|
||||
5)
|
||||
return false; /* malformed record: skip, never fatal */
|
||||
|
||||
/* Owner is applied best-effort only: a non-root process cannot chown and must
|
||||
not abort the transfer for that reason (FastSync identity philosophy). */
|
||||
if (fchown(fd, (uid_t)ul_uid, (gid_t)ul_gid) != 0 && errno != EPERM && errno != EACCES &&
|
||||
errno != EINVAL)
|
||||
/* Owner is applied best-effort only: a non-root process cannot chown and
|
||||
must not abort the transfer for that reason (FastSync identity philosophy).
|
||||
EPERM/EACCES (expected for a non-root receiver) are skipped silently; a
|
||||
genuine EINVAL (an impossible stored id) is logged so the corruption is
|
||||
not hidden. */
|
||||
if (fchown(fd, (uid_t)ul_uid, (gid_t)ul_gid) != 0 && errno != EPERM && errno != EACCES)
|
||||
log_message(LOG_LEVEL_WARNING, "--fake-super: could not restore owner on destination file: %s",
|
||||
strerror(errno));
|
||||
if (fchmod(fd, (mode_t)(ul_mode & 07777U)) != 0)
|
||||
/* Mode is applied through the same sanitization the normal metadata path
|
||||
uses (metadata_mode): group/other write bits are never granted, so a
|
||||
recorded source mode of 0666 restores as 0644 — identical to a non-fake-
|
||||
super --preserve run, never a privilege-granting regression. */
|
||||
if (fchmod(fd, (mode_t)(ul_mode & 0777U & ~(S_IWGRP | S_IWOTH))) != 0)
|
||||
log_message(LOG_LEVEL_WARNING, "--fake-super: could not restore mode on destination file: %s",
|
||||
strerror(errno));
|
||||
struct timespec times[2] = {{.tv_sec = 0, .tv_nsec = UTIME_OMIT},
|
||||
|
||||
+5
-3
@@ -89,9 +89,11 @@ void fake_super_store_fd(int fd, uint32_t uid, uint32_t gid, uint32_t mode, int6
|
||||
/* --fake-super replay: parse the FAKESUPER_XATTR record previously written on
|
||||
* `fd` by fake_super_store_fd and re-apply uid/gid/mode/mtime fd-relative.
|
||||
* Best-effort: absence of the xattr or a malformed record is a silent no-op
|
||||
* that never fails the transfer, and fchown is applied only when permitted
|
||||
* (a non-root EPERM is logged and skipped, matching FastSync's identity
|
||||
* philosophy). Returns true when the xattr was present and parsed. */
|
||||
* that never fails the transfer; fchown is applied only when permitted (a
|
||||
* non-root EPERM/EACCES is skipped silently, matching FastSync's identity
|
||||
* philosophy), and the mode is sanitized exactly like the normal metadata path
|
||||
* (group/other write bits never granted). Returns true when the xattr was
|
||||
* present and parsed. */
|
||||
bool fake_super_restore_fd(int fd);
|
||||
|
||||
#endif
|
||||
Reference in New Issue
Block a user