Skip to content

DAOS-19631 pil4dfs: hand a symlink dfs cannot follow to the kernel - #19108

Open
mchaarawi wants to merge 1 commit into
masterfrom
mschaara/pil4dfs_symlink_fallback
Open

mchaarawi wants to merge 1 commit into
masterfrom
mschaara/pil4dfs_symlink_fallback

Conversation

@mchaarawi

Copy link
Copy Markdown
Contributor

dfs cannot follow a symlink whose value is absolute (EINVAL since DAOS-19630), nor one whose relative value climbs more than one level with ".." (ENOTSUP): the first needs the process root, the second the ancestry of the link. In a non-leaf position the error came out of the parent walk in query_path() with no parent record, so the application saw EINVAL or ENOTSUP where the kernel resolves the path correctly through dfuse.

Treat both errors from the parent walk, in query_path() and in resolve_dot_dot(), as "not handled by pil4dfs" so the libc function runs, and extend the leaf probe in need_kernel_to_resolve() to ENOTSUP. Drop root_symlink_escapes(): with the uniform EINVAL from dfs the root-level case is covered by the existing leaf probe.

Add tests for an absolute directory link in a subdirectory and in the container root, each shadowed by an in-container copy of the target path, and for "..", "../../x" and "../../d/x" values as leaf and non-leaf components, inside and out of the container.

Features: pil4dfs

Steps for the author:

  • Commit message follows the guidelines.
  • Appropriate Features or Test-tag pragmas were used.
  • Appropriate Functional Test Stages were run.
  • At least two positive code reviews including at least one code owner from each category referenced in the PR.
  • Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

  • Gatekeeper requested (daos-gatekeeper added as a reviewer).

dfs cannot follow a symlink whose value is absolute (EINVAL since
DAOS-19630), nor one whose relative value climbs more than one level
with ".." (ENOTSUP): the first needs the process root, the
second the ancestry of the link. In a non-leaf position the error came
out of the parent walk in query_path() with no parent record, so the
application saw EINVAL or ENOTSUP where the kernel resolves the path
correctly through dfuse.

Treat both errors from the parent walk, in query_path() and in
resolve_dot_dot(), as "not handled by pil4dfs" so the libc function
runs, and extend the leaf probe in need_kernel_to_resolve() to ENOTSUP.
Drop root_symlink_escapes(): with the uniform EINVAL from dfs the
root-level case is covered by the existing leaf probe.

Add tests for an absolute directory link in a subdirectory and in the
container root, each shadowed by an in-container copy of the target
path, and for "..", "../../x" and "../../d/x" values as leaf and
non-leaf components, inside and out of the container.

Features: pil4dfs

Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@hpe.com>
@github-actions

Copy link
Copy Markdown

Ticket title is 'pil4dfs: an absolute symlink in a non-leaf position of a path fails with EINVAL instead of being handed to the kernel'
Status is 'Open'
https://daosio.atlassian.net/browse/DAOS-19631

@mchaarawi
mchaarawi marked this pull request as ready for review September 18, 2026 14:56
@mchaarawi
mchaarawi requested review from a team as code owners September 18, 2026 14:56
* that a chain such as python3 -> python -> /usr/bin/python3.x is caught as well.
*/
static bool
root_symlink_escapes(struct dfs_mt *dfs_mt, struct dcache_rec *parent, const char *item_name,

@wangshilong wangshilong Sep 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems to miss the O_NOFOLLOW case.
For a root symlink such as:
python -> /usr/bin/python3

open("python", O_RDONLY | O_NOFOLLOW)  must fail with  ELOOP . 

after removing root_symlink_escapes() , dfs_lookup_rel(..., O_NOFOLLOW, ...) may successfully return the symlink itself, so the error-based kernel fallback is not triggered.

Could we preserve the kernel fallback for this flag combination, and add a regression test that verifies O_NOFOLLOW returns ELOOP ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants