Skip to content

procfs: return EINVAL from readlink on non-symlinks - #280

Open
AlbertSanoe wants to merge 2 commits into
multikernel:mainfrom
AlbertSanoe:fix/readlink-errno-279
Open

AlbertSanoe wants to merge 2 commits into
multikernel:mainfrom
AlbertSanoe:fix/readlink-errno-279

Conversation

@AlbertSanoe

Copy link
Copy Markdown

readlink(2) and readlinkat(2) calls with a non-empty pathname returned
ENOENT instead of EINVAL when the target was a regular file or
directory.

The metadata handler pins the target and then calls
readlinkat(fd, "", ...). For a non-symlink, the kernel reports ENOENT
when the pathname is empty but EINVAL for a named lookup. glibc
realpath(3) treats EINVAL as "not a symlink" and continues resolving,
but treats ENOENT as failure, so existing paths such as /etc appeared
to be missing.

Translate ENOENT to EINVAL only when the caller's original pathname is
non-empty and fstat(2) on the same pinned descriptor confirms that the
target is not a symlink. All other cases keep the original errno,
including explicit empty-path requests, errors reported by actual
symlinks (e.g. procfs magic links whose target is gone), and fstat
failures.

Add six regression tests to the existing procfs integration suite,
covering readlink and readlinkat with absolute and relative paths;
regular files, directories, valid and dangling symlinks; ENOENT and
ENOTDIR path errors; empty-path requests and invalid descriptors;
zero-length and truncated buffers; proc links; and libc realpath.

Testing:

Tested on Linux 6.2 x86_64 with Landlock ABI v3, with unsupported
newer protections explicitly allowed to degrade. The full
integration suite was not run because this host does not meet
its strict Landlock ABI v6 requirements.

Fixes #279

@congwang-mk congwang-mk left a comment

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.

Thanks, the diagnosis and fix are correct. A few requests:

  1. Rather than translating the errno after readlinkat(fd, "") fails, check the type first and only call readlinkat for links:

    let mut st: libc::stat = unsafe { std::mem::zeroed() };
    if unsafe { libc::fstat(fd.as_raw_fd(), &mut st) } < 0 {
        return errno_action();
    }
    if !path.is_empty() && st.st_mode & libc::S_IFMT != libc::S_IFLNK {
        return NotifAction::Errno(libc::EINVAL);
    }

    This states the rule directly (named readlink on a non-link is EINVAL), keeps errno_action(), and drops the comment.

  2. Please drop the allow_degraded(...) calls in run_readlink_test. No other test in this file needs them and CI runs full Landlock.

  3. Six sandbox launches is a lot for one errno fix; consider folding them into one or two tests, keeping the libc realpath one.

Separately, COW has the same bug: readlink_in_root in sys/fs.rs uses the same readlinkat(fd, "") pattern, and handle_cow_readlink maps every failure to ENOENT. Fine to leave for a follow-up.

@AlbertSanoe

Copy link
Copy Markdown
Author

Updated as requested: check the target type before readlinkat, remove allow_degraded, and consolidate the coverage into two tests while keeping the libc realpath case.

This branch has not been deployed

No deployments
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.

readlink returns ENOENT instead of EINVAL for non-symlinks (v0.8.9 regression)

2 participants