Skip to content

chroot_realpath: do not add a separator after expanding a symlink - #2422

Merged
kolyshkin merged 1 commit into
containers:mainfrom
kolyshkin:chroot-realpath-dup-sep
Oct 30, 2026
Merged

kolyshkin merged 1 commit into
containers:mainfrom
kolyshkin:chroot-realpath-dup-sep

Conversation

@kolyshkin

@kolyshkin kolyshkin commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

After expanding a symlink that is not the last path component, chroot_realpath() appends an extra separator, resulting in // in the resolved path. This has two effects:

  • a .. that follows (e.g. from a sym -> ../escaped target) is silently dropped, so /dir/sym/file resolves to <root>/dir/escaped/file instead of <root>/escaped/file (a regression since 513d3ce);
  • an absolute intermediate symlink yields <root>//target/file, and callers stripping the rootfs prefix get //target/file.

The result always stays under the rootfs; only the path inside it may differ from what the kernel would resolve. This is reachable via the safe_openat() fallback (no openat2(2)) and the CRIU external bind mount handling.

The fix is to continue right after the symlink expansion, since new_path already points after a separator.

Unit tests for intermediate symlinks are added (they fail without the fix).

Reported-by: @inanna-malick

@packit-as-a-service

Copy link
Copy Markdown

Ephemeral COPR build failed. @containers/packit-build please check.

1 similar comment
@packit-as-a-service

Copy link
Copy Markdown

Ephemeral COPR build failed. @containers/packit-build please check.

@kolyshkin

Copy link
Copy Markdown
Collaborator Author

CI is being fixed by #2423

@giuseppe giuseppe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

After a symlink is expanded, new_path already points right after a
separator: the root one for an absolute symlink, and the parent
directory one for a relative symlink (since commit 513d3ce).  When the
symlink is not the last component, the code then appends another
separator, so the result gets a "//" in it.

This has two effects:

 * a ".." that follows (e.g. coming from a "sym -> ../escaped" target)
   backs up to the duplicate separator only, so it is silently dropped:
   "/dir/sym/file" resolves to "<root>/dir/escaped/file" instead of
   "<root>/escaped/file";

 * an absolute intermediate symlink yields "<root>//target/file", so
   callers stripping the rootfs prefix get "//target/file".

The result stays under the rootfs in all cases, only the path inside
it may differ from what the kernel would resolve.

Since the expanded symlink contents become the remaining path to
process, there is nothing left to do in the current iteration, so just
continue with the loop.

Add test cases for intermediate symlinks.

Fixes: 513d3ce ("chroot_realpath: keep separator when expanding a relative symlink")
Reported-by: Inanna Malick <inanna@recursion.wtf>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
kolyshkin force-pushed the chroot-realpath-dup-sep branch from e38f17c to cb66e4d Compare October 30, 2026 19:08
@kolyshkin
kolyshkin merged commit 1847da0 into containers:main Oct 30, 2026
67 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.

2 participants