Skip to content

rrsync: support fd pins in user namespaces - #1048

Open
steadytao wants to merge 2 commits into
RsyncProject:masterfrom
steadytao:fix/rrsync-userns-procfs
Open

rrsync: support fd pins in user namespaces#1048
steadytao wants to merge 2 commits into
RsyncProject:masterfrom
steadytao:fix/rrsync-userns-procfs

Conversation

@steadytao

Copy link
Copy Markdown
Member

Fixes #1044

Summary

  • allow the exact /proc/self component of a recognised rrsync fd pin when procfs reports its owner as the overflow UID inside a user namespace
  • retain the normal symlink-owner and confinement checks for every other component and for the resolved fd target
  • add a regression test that enters an unprivileged user namespace and exercises a confined /proc/self/fd/N log path

@steadytao
steadytao force-pushed the fix/rrsync-userns-procfs branch 3 times, most recently from 619165d to cc56351 Compare August 13, 2026 15:41
@steadytao
steadytao force-pushed the fix/rrsync-userns-procfs branch from cc56351 to 6a52827 Compare August 13, 2026 15:48
@tridge

tridge commented Aug 13, 2026

Copy link
Copy Markdown
Member

@samueloph please test

@samueloph

Copy link
Copy Markdown
Member

@tridge @steadytao I confirm this solves the problem on Debian

@seks99x

seks99x commented Aug 14, 2026

Copy link
Copy Markdown
Member

@steadytao Great job on this fix! I did notice one small edge case during testing, if we change the test to use /dev/fd/ instead of /proc/self/fd/, the test still fails. It looks like /dev/fd/ will require some extra handling or allowlisting alongside /proc/self/fd/

----- new log follows
confined log path through an fd pin was rejected (rc=0, stderr='rsync: [client] failed to open log-file /dev/fd/3/rsync.log: Too many levels of symbolic links (40)')
user-namespace regression test failed (rc=1)
----- new log ends
FAIL    fd_test
------------------------------------------------------------
----- overall results:
      0 passed
      1 failed
------------------------------------------------------------
overall result is 1

@steadytao

Copy link
Copy Markdown
Member Author

Thanks... /dev/fd reaches a different kernel magic-link path. I would keep this change limited to the user-namespace /proc/self ownership exception rather than broaden the confined path traversal here.

@steadytao

Copy link
Copy Markdown
Member Author

Just realised that #1054 would probably cover it anyways

@steadytao

Copy link
Copy Markdown
Member Author

Yes it would, should be good to merge this.

@seks99x

seks99x commented Aug 14, 2026

Copy link
Copy Markdown
Member

Actually I don't think this is covered in #1054 because I didn't modify any permission changes but we could fix that on the same PR if you would like.

@steadytao

Copy link
Copy Markdown
Member Author

Ah, yeah, I just assumed from 1053's scope. Nonetheless, I don't fully see why we need to broaden the confinement exception to /dev/fd/N seeing as those pins are never generated?

@seks99x

seks99x commented Aug 14, 2026

Copy link
Copy Markdown
Member

Im not with broadening as what you saying. The issue is Bash process substitution uses /dev/fd. If the same case happened using Bash substitution it will mostly fail with ELOOP. Some other cases we may be missing (which might be ok) but I think if we will already make exceptions to broaden the checks more we would add /dev/fd & /proc/self/ only as they are the most common. What do you think?

@steadytao

Copy link
Copy Markdown
Member Author

Ah yeah true, didn't think of those cases. Will broaden a bit. Thank you, Omar.

@steadytao steadytao removed the run-ci label Aug 15, 2026
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.5.0 regression] rrsync fails inside a user namespace: "/proc/self/fd/N" ELOOP

4 participants