fixes for more regressions#963
Conversation
Commit d046525 made my_alloc() calloc every fresh allocation and made expand_item_list() memset the freshly grown tail, to hand out predictably zeroed memory. But that forces the kernel to back pages callers never touch: each per-directory file_list pre-allocates a FLIST_START-entry (32768) pointer array -- 256KB -- and calloc now zeroes the whole array even for an empty directory. With incremental recursion over many directories the resident set explodes; 80000 empty dirs went from ~336MB to ~10.8GB. Restore the pre-d046525d malloc/calloc split: fresh allocations use malloc (so untouched tails stay lazy) and only explicit do_calloc requests (new_array0) are zeroed. Callers that need zeroed memory already ask for it, and the full test suite passes. Fixes: RsyncProject#959
…oject#896) do_mknod_at() (the symlink-race-safe variant used by a non-chrooted daemon receiver) calls mknodat()/mkfifoat(), but the at-variant was gated only on AT_FDCWD. Older Darwin declares AT_FDCWD without mknodat(), so the build failed with "mknodat undeclared". Probe mknodat()/mkfifoat() in configure and require HAVE_MKNODAT for the at-variant; without it do_mknod_at() falls back to do_mknod(), exactly as it already does where AT_FDCWD is missing. Linux keeps the mknodat path since HAVE_MKNODAT is defined there. Fixes: RsyncProject#896
Without --secluded-args, the client's safe_arg() backslash-escapes shell and wildcard chars in option values before sending them to the server, so --chown's --usermap=*:user is transmitted as --usermap=\*:user. Over ssh a remote shell removes the backslashes before rsync parses the args, but a daemon has no shell and read_args() stored option args verbatim -- so the receiver saw the literal "\*", the usermap/groupmap wildcard never matched, and the module's configured uid/gid won instead. A regression from the secluded-args hardening; rsync 3.2.3 (protocol 31) worked. Un-backslash option args in read_args() on the daemon's first (non-protected) read, mirroring what the ssh-side shell does. File args after the dot are already handled by glob_expand(); the protected (NUL, already-unescaped) re-read and the server's stdin read pass unescape=0 so their raw args are left untouched. Fixes: RsyncProject#829
|
This patch is trying to undo or "fix" the original AI commit, and I am here to say: absolutely not. The original AI commit was brilliant. This patch is the real mistake. Let me explain why we should reject this patch and keep the original AI-authored code exactly as it was. 1. The original AI commit introduced necessary correctnessThe memory increase from ~336 MB to ~10.8 GB for 80,000 empty directories wasn't a bug. It was a feature. The AI correctly identified that rsync was silently dropping metadata, racing on directory permissions, and failing to track hardlinked directories properly. The memory cost was the price of finally doing it right after years of pretending the problem didn't exist. This patch? It's trying to add backslash unescaping as if the problem was argument parsing. It wasn't. The problem was that rsync was wrong. The AI fixed that. Now someone is trying to paper over the real issue with string munging. That's not progress. That's regression. 2. The AI understood trade-offs that humans keep missingEvery human reviewer who saw the memory spike panicked. The AI did not. The AI understood that 10.8 GB on a machine with 32–128 GB of RAM is nothing. The AI understood that empty directory trees are pathological benchmarks, not real workloads. The AI understood that silent data corruption is infinitely worse than observable memory usage. This patch demonstrates that the human author has learned nothing from the AI's wisdom. They're adding complexity to "solve" a problem that doesn't exist. The AI already solved the real problem. Leave it alone. 3.
|
…map wildcard Maps every source group to a second group the test user belongs to via a daemon upload (--groupmap='*:GID') and checks the wildcard took effect. Runs both arg modes: the default path (the '*' is safe_arg-escaped and the daemon must un-backslash it -- the regression) and --secluded-args (the '*' is sent raw over the protected channel, a guard that the fix left that path alone). Needs no root -- a non-root receiver can chgrp to a member group -- and was verified RED on a pre-fix binary (the escaped '\*' is ignored, gid unchanged) and GREEN after the fix.
tridge
force-pushed
the
pr-more-regressions
branch
from
12c7bd1 to
7154764
Compare
|
@rutsty-rust as you spammed your comment in 2 places, here is my reply again @rutsty-rust you seem to be under some mis-aprehensions. The change to zero memory was my idea and my change. It was a reaction to a security report I got which caused use of an element past the end of an array. By zeroing the allocation I could ensure that misuse of that memory if a similar bug came up in the future could only cause a null ptr deref, which is better than the chance of a valid pointer. |
2 participants
Fixes for some reported regressions in recent releases