Skip to content

linux: do not skip EACCES in do_masked_or_readonly_path - #2145

Merged
giuseppe merged 1 commit into
containers:mainfrom
Rajkaran-122:fix-eacces-skip-2139
Aug 6, 2026
Merged

linux: do not skip EACCES in do_masked_or_readonly_path#2145
giuseppe merged 1 commit into
containers:mainfrom
Rajkaran-122:fix-eacces-skip-2139

Conversation

@Rajkaran-122

Copy link
Copy Markdown
Contributor

Description:
When attempting to open a masked or read-only path, crun was silently ignoring both ENOENT and EACCES errors.

To maintain consistency with runc's fail-closed behavior, this commit updates the logic so that only ENOENT is silently ignored. If EACCES is encountered, the error is now properly propagated, preventing potential protection bypasses if a masked/readonly paths entry cannot be opened due to permissions.

Fixes: #2139

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@packit-as-a-service

Copy link
Copy Markdown

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

@giuseppe

Copy link
Copy Markdown
Member

I am not sure yet whether we need this change

@kolyshkin

Copy link
Copy Markdown
Collaborator

The EACCES was initially added by commit e2bf304 which explains why a path is skipped.

Since this is run by crun init in the container context (but before apply_security_settings), chances are if it got EACCES, then any container process will have the same. Ergo, no need to mask a path or make it ro.

If anything, runc should be changed to match that. Also, crun can print a warning but I'm not sure those warnings are visible by upper level runtimes (and their users).

@giuseppe

giuseppe commented Aug 4, 2026

Copy link
Copy Markdown
Member

@Rajkaran-122 are you ok to change it to a debug/warning message?

@Rajkaran-122

Rajkaran-122 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the clarification, @giuseppe sir and @kolyshkin sir. That makes sense. I'm happy to update the PR to log a debug/warning message on EACCES while preserving the current behavior. I'll make that change.

@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.

please squash in a single commit and adjust the commit message, no need to use a fixup.

Comment thread src/libcrun/linux.c Outdated
return pathfd;

if (errno == EACCES)
libcrun_debug ("skipping inaccessible path %s", rel_path);

@giuseppe giuseppe Aug 4, 2026

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.

quote the string %s with `%s`.

@Rajkaran-122
Rajkaran-122 force-pushed the fix-eacces-skip-2139 branch from 2484746 to ec5e29f Compare August 4, 2026 10:11
When opening a masked or read-only path returns EACCES,
log a debug message while preserving the existing behavior
of skipping the path. Continue silently skipping ENOENT.

Signed-off-by: Rajkaran Yadav <yadavrajkaran854@gmail.com>
@Rajkaran-122
Rajkaran-122 force-pushed the fix-eacces-skip-2139 branch from ec5e29f to 3cfb471 Compare August 4, 2026 10:15

@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

@Rajkaran-122

Rajkaran-122 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review @giuseppe sir ! I looked into the remaining Test (system-blake3) failure. It appears the GitHub-hosted runner lost communication before the system-blake3 test started. Since this PR only changes src/libcrun/linux.c and doesn't affect the test workflow or system-blake3, the failure appears unrelated to this change. Please let me know if you'd like me to investigate further.

@giuseppe

giuseppe commented Aug 4, 2026

Copy link
Copy Markdown
Member

Thanks for the review @giuseppe sir ! I looked into the remaining Test (system-blake3) failure. It appears the GitHub-hosted runner lost communication before the system-blake3 test started. Since this PR only changes src/libcrun/linux.c and doesn't affect the test workflow or system-blake3, the failure appears unrelated to this change. Please let me know if you'd like me to investigate further.

no it is fine, the failures are not related to this change.

@giuseppe

giuseppe commented Aug 4, 2026

Copy link
Copy Markdown
Member

@kolyshkin are you OK with the last version?

@giuseppe
giuseppe merged commit c3a49b8 into containers:main Aug 6, 2026
66 of 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.

do_masked_or_readonly_path skips masked/readonly paths on EACCES — intentional? (fail-closed consistency with runc)

3 participants