-
-
Notifications
You must be signed in to change notification settings - Fork 589
Fix ENOENT when resolving kernel pseudo-paths in ona_open #1054
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
base: master
Are you sure you want to change the base?
Changes from 3 commits
84a3b71
4f4186d
1bb3c5b
916db38
ce157e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -319,8 +319,7 @@ static int ona_open(const char *path, int flags, mode_t mode, char *out_abs, siz | |
| * (abspath_outside_confinement). A relative operator path starts at the | ||
| * daemon's cwd == the module root; an absolute one (or a followed absolute | ||
| * symlink target) restarts at "/". */ | ||
| char abspath[MAXPATHLEN]; | ||
| abspath[0] = '\0'; | ||
| char abspath[MAXPATHLEN] = {0}; | ||
| if (am_daemon && module_dir && module_dir[0] == '/') | ||
| strlcpy(abspath, module_dir, sizeof abspath); /* "/" for a path=/ module */ | ||
| else if (confine_root) { | ||
|
|
@@ -434,6 +433,46 @@ static int ona_open(const char *path, int flags, mode_t mode, char *out_abs, siz | |
| } | ||
| target[n] = '\0'; | ||
|
|
||
| /* Detect Linux kernel pseudo-paths (pipes, sockets, anon_inodes). | ||
| * These are not real paths on disk and never contain slashes. */ | ||
| const char *ptail = fd_pin_tail(abspath); | ||
| int is_fd_dir = (ptail != NULL && *ptail == '\0'); | ||
|
|
||
| if (is_fd_dir && strchr(target, '/') == NULL && | ||
| (strncmp(target, "pipe:[", 6) == 0 || | ||
| strncmp(target, "socket:[", 8) == 0 || | ||
| strncmp(target, "anon_inode:", 11) == 0)) { | ||
|
Comment on lines
+448
to
+451
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are sockets and anonymous inodes included?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like that vacation actually affected my focus in a bad way, but I'm back now! I took a step back and used strace to map out exactly how rsync is handling this flags. I initially tried to strictly validate By safely ignoring To handle the actual dangerous flags you mentioned (like Regarding
I’ve updated the test suite to include a edge case ensuring that an Let me know how this looks! |
||
|
|
||
| if (!is_last) { | ||
| /* Cannot traverse a pseudo-path like a directory */ | ||
| saved_errno = ENOTDIR; | ||
| goto out; | ||
| } | ||
|
|
||
| /* Verify comp is entirely numeric (it must be the FD number) */ | ||
| int is_num = 1; | ||
| for (int i = 0; comp[i] != '\0'; i++) { | ||
| if (comp[i] < '0' || comp[i] > '9') { | ||
| is_num = 0; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (is_num) { | ||
| int fd_num = atoi(comp); | ||
| #ifdef F_DUPFD_CLOEXEC | ||
| retfd = fcntl(fd_num, F_DUPFD_CLOEXEC, 0); | ||
| #else | ||
| retfd = dup(fd_num); | ||
| #endif | ||
|
steadytao marked this conversation as resolved.
Outdated
|
||
| saved_errno = (retfd >= 0) ? 0 : errno; | ||
| goto out; | ||
| } | ||
|
|
||
| /* If it's in an FD dir but fails checks, treat as dead link */ | ||
| saved_errno = ENOENT; | ||
| goto out; | ||
| } | ||
| /* Splice: new `remaining` = <target> + <tail-after-comp>. | ||
| * Absolute target restarts the walk from "/". */ | ||
| char tail[MAXPATHLEN]; | ||
|
|
@@ -3988,3 +4027,4 @@ int do_stat_atfd(int dfd, const char *name, STRUCT_STAT *st) | |
| return -1; | ||
| #endif | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| #!/usr/bin/env python3 | ||
| """Process substitution /dev/fd/ pipe pseudo-paths must not crash with ENOENT.""" | ||
|
|
||
| import os | ||
| import shlex | ||
| import shutil | ||
| import subprocess | ||
| import tempfile | ||
| from pathlib import Path | ||
|
|
||
| from rsyncfns import makepath, rmtree, rsync_argv, test_fail, test_skipped | ||
|
|
||
| # We require bash specifically because standard POSIX /bin/sh does not | ||
| # guarantee support for <(...) process substitution syntax. | ||
| bash = shutil.which('bash') | ||
| if bash is None: | ||
| test_skipped('bash is unavailable, cannot test process substitution') | ||
|
|
||
| # Verify the host bash actually supports process substitution (it can be | ||
| # disabled in certain restricted environments). | ||
| probe = subprocess.run( | ||
| [bash, '-c', 'cat <(echo "probe")'], | ||
| capture_output=True | ||
| ) | ||
| if probe.returncode != 0: | ||
| test_skipped('bash process substitution is not supported on this system') | ||
|
|
||
| base = Path(tempfile.mkdtemp(prefix='rsync-pseudo-path-')) | ||
| src = base / 'src' | ||
| dest = base / 'dest' | ||
| makepath(src, dest) | ||
|
|
||
| (src / 'keep_me.txt').write_text('keep this\n') | ||
| (src / 'exclude_me.txt').write_text('drop this\n') | ||
|
|
||
| # rsync_argv() provides the correct binary path, which we must wrap into a | ||
| # raw string so bash can parse the <(...) syntax rather than Python. | ||
| rsync_base_cmd = shlex.join(rsync_argv('-a')) | ||
| src_path = shlex.quote(str(src) + '/') | ||
| dest_path = shlex.quote(str(dest) + '/') | ||
|
|
||
| bash_script = f"{rsync_base_cmd} --exclude-from=<(echo 'exclude_me.txt') {src_path} {dest_path}" | ||
|
|
||
| try: | ||
| proc = subprocess.run( | ||
| [bash, '-c', bash_script], | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=10, | ||
| ) | ||
| except subprocess.TimeoutExpired: | ||
| rmtree(base) | ||
| test_fail('process substitution test timed out') | ||
|
|
||
| ctx = f'rc={proc.returncode}, stderr={proc.stderr.strip()!r}' | ||
|
|
||
| if proc.returncode != 0: | ||
| rmtree(base) | ||
| test_fail(f'rsync crashed reading a pseudo-path pipe ({ctx})') | ||
|
|
||
| if not (dest / 'keep_me.txt').is_file(): | ||
| rmtree(base) | ||
| test_fail(f'rsync failed to transfer the allowed file ({ctx})') | ||
|
|
||
| if (dest / 'exclude_me.txt').is_file(): | ||
| rmtree(base) | ||
| test_fail(f'rsync ignored the process substitution exclude list ({ctx})') | ||
|
|
||
| rmtree(base) | ||
| print('rsync successfully read process substitution pseudo-paths') | ||
| raise SystemExit(0) |
Uh oh!
There was an error while loading. Please reload this page.