From d39402295c90b791774b90e007a73d8bcca53519 Mon Sep 17 00:00:00 2001 From: Albert Sanoe Date: Thu, 8 Oct 2026 00:27:39 +0800 Subject: [PATCH 1/2] procfs: preserve readlink errno for non-symlinks --- crates/sandlock-core/src/procfs/metadata.rs | 17 +- .../tests/integration/test_procfs.rs | 158 +++++++++++++++++- 2 files changed, 173 insertions(+), 2 deletions(-) diff --git a/crates/sandlock-core/src/procfs/metadata.rs b/crates/sandlock-core/src/procfs/metadata.rs index d64f717d..3fcb6229 100644 --- a/crates/sandlock-core/src/procfs/metadata.rs +++ b/crates/sandlock-core/src/procfs/metadata.rs @@ -385,7 +385,22 @@ pub(crate) async fn handle_pinned_metadata( ) }; if count < 0 { - return errno_action(); + let errno = std::io::Error::last_os_error() + .raw_os_error() + .unwrap_or(libc::EIO); + // An empty-path readlinkat on a non-link returns ENOENT, + // while the caller's non-empty pathname requires EINVAL. + // Preserve explicit empty-path requests and errors from + // actual links (e.g. a procfs link whose target vanished). + if !path.is_empty() && errno == libc::ENOENT { + let mut st: libc::stat = unsafe { std::mem::zeroed() }; + if unsafe { libc::fstat(fd.as_raw_fd(), &mut st) } == 0 + && st.st_mode & libc::S_IFMT != libc::S_IFLNK + { + return NotifAction::Errno(libc::EINVAL); + } + } + return NotifAction::Errno(errno); } if resolved == "/proc/self" { target = tgid.to_string().into_bytes(); diff --git a/crates/sandlock-core/tests/integration/test_procfs.rs b/crates/sandlock-core/tests/integration/test_procfs.rs index 645b41e2..3ea510e8 100644 --- a/crates/sandlock-core/tests/integration/test_procfs.rs +++ b/crates/sandlock-core/tests/integration/test_procfs.rs @@ -1,5 +1,5 @@ use sandlock_core::sandbox::ByteSize; -use sandlock_core::{Sandbox}; +use sandlock_core::{Protection, Sandbox}; /// Test that num_cpus virtualizes both /proc/cpuinfo and sched_getaffinity. #[tokio::test] @@ -1238,3 +1238,159 @@ print('OK') let result = policy.run(&["python3", "-c", &script]).await.unwrap(); assert!(result.success(), "{}", result.stderr_str().unwrap_or("")); } + +// Preserve readlink errno, including the EINVAL that libc realpath expects +// for ordinary files and directories. +const READLINK_HELPERS: &str = r#" +import ctypes, errno, os, sys + +root = sys.argv[1] +os.chdir(root) +libc = ctypes.CDLL(None, use_errno=True) +libc.readlink.argtypes = [ctypes.c_char_p, ctypes.c_void_p, ctypes.c_size_t] +libc.readlink.restype = ctypes.c_ssize_t +libc.readlinkat.argtypes = [ctypes.c_int, ctypes.c_char_p, ctypes.c_void_p, ctypes.c_size_t] +libc.readlinkat.restype = ctypes.c_ssize_t +dirfd = os.open(root, os.O_RDONLY | os.O_DIRECTORY) + +def check_readlink(path, expected, fd=None, size=4096): + buf = ctypes.create_string_buffer(max(size, 1)) + ctypes.set_errno(0) + if fd is None: + count = libc.readlink(os.fsencode(path), buf, size) + else: + count = libc.readlinkat(fd, os.fsencode(path), buf, size) + if isinstance(expected, int): + actual = (count, ctypes.get_errno()) + assert actual == (-1, expected), (path, fd, size, actual, expected) + else: + actual = (count, buf.raw[:max(count, 0)]) + assert actual == (len(expected), expected), (path, fd, size, actual, expected) +"#; + +async fn run_readlink_test(body: &str) { + let root = tempfile::tempdir().unwrap(); + std::fs::write(root.path().join("file"), b"fixture\n").unwrap(); + std::fs::create_dir(root.path().join("dir")).unwrap(); + std::os::unix::fs::symlink("file", root.path().join("link")).unwrap(); + std::os::unix::fs::symlink("missing", root.path().join("dangling")).unwrap(); + + let script = format!("{READLINK_HELPERS}\n{body}\nprint('OK')\n"); + // These metadata semantics do not require the newer Landlock rights. + // Unsupported rights may degrade, but the sandbox run must still succeed. + let mut policy = proc_grant() + .fs_read(root.path()) + .allow_degraded(Protection::NetTcp) + .allow_degraded(Protection::FsIoctlDev) + .allow_degraded(Protection::SignalScope) + .allow_degraded(Protection::AbstractUnixSocketScope) + .build() + .unwrap(); + let result = tokio::time::timeout( + std::time::Duration::from_secs(20), + policy.run(&["python3", "-c", &script, root.path().to_str().unwrap()]), + ) + .await + .expect("readlink regression test timed out") + .unwrap(); + assert!(result.success(), "{}", result.stderr_str().unwrap_or("")); + assert_eq!(result.stdout_str().unwrap_or("").trim(), "OK"); +} + +#[tokio::test] +async fn test_readlink_non_symlinks_return_einval() { + let script = r#" +for name in ['file', 'dir']: + absolute = os.path.join(root, name) + check_readlink(absolute, errno.EINVAL) + check_readlink(name, errno.EINVAL) + check_readlink(absolute, errno.EINVAL, dirfd) + check_readlink(name, errno.EINVAL, dirfd) +"#; + run_readlink_test(script).await; +} + +#[tokio::test] +async fn test_readlink_symlinks_and_path_errors() { + let script = r#" +for name, expected in [ + ('link', b'file'), + ('dangling', b'missing'), + ('missing', errno.ENOENT), + ('file/child', errno.ENOTDIR), +]: + check_readlink(os.path.join(root, name), expected) + check_readlink(name, expected, dirfd) +"#; + run_readlink_test(script).await; +} + +#[tokio::test] +async fn test_readlinkat_empty_path_preserves_errno() { + let script = r#" +# An explicit empty-path request on a non-link must retain ENOENT, not EINVAL. +for name, expected in [ + ('file', errno.ENOENT), + ('dir', errno.ENOENT), + ('link', b'file'), + ('dangling', b'missing'), +]: + fd = os.open(name, os.O_PATH | os.O_NOFOLLOW) + check_readlink('', expected, fd) + os.close(fd) +check_readlink('link', errno.EBADF, -1) +check_readlink('', errno.EBADF, -1) +"#; + run_readlink_test(script).await; +} + +#[tokio::test] +async fn test_readlink_buffer_sizes() { + let script = r#" +check_readlink('link', errno.EINVAL, size=0) +check_readlink('link', b'fi', size=2) +"#; + run_readlink_test(script).await; +} + +#[tokio::test] +async fn test_readlink_proc_links() { + let script = r#" +check_readlink('/proc/net', b'self/net') +check_readlink('/proc/self', str(os.getpid()).encode()) +fd = os.open('file', os.O_RDONLY) +check_readlink('/proc/self/fd/' + str(fd), os.fsencode(os.path.join(root, 'file'))) +os.close(fd) +"#; + run_readlink_test(script).await; +} + +#[tokio::test] +async fn test_readlink_libc_realpath() { + // Python's os.path.realpath tolerates the wrong readlink errno, so call + // libc realpath itself to reproduce the reported regression on glibc. + let script = r#" +libc.realpath.argtypes = [ctypes.c_char_p, ctypes.c_void_p] +libc.realpath.restype = ctypes.c_void_p +libc.free.argtypes = [ctypes.c_void_p] +libc.free.restype = None +for name, target in [ + ('file', 'file'), + ('dir', 'dir'), + ('link', 'file'), + ('dangling', None), + ('missing', None), +]: + ctypes.set_errno(0) + resolved = libc.realpath(os.fsencode(os.path.join(root, name)), None) + if target is None: + assert not resolved and ctypes.get_errno() == errno.ENOENT, (name, ctypes.get_errno()) + else: + assert resolved, (name, ctypes.get_errno()) + actual = ctypes.string_at(resolved) + libc.free(resolved) + expected = os.fsencode(os.path.join(root, target)) + assert actual == expected, (name, actual, expected) +"#; + run_readlink_test(script).await; +} From e2570297e12ebb7714587268cc8303aff2d69879 Mon Sep 17 00:00:00 2001 From: Albert Sanoe Date: Sat, 10 Oct 2026 19:32:10 +0800 Subject: [PATCH 2/2] procfs: simplify readlink checks and tests --- crates/sandlock-core/src/procfs/metadata.rs | 24 +-- .../tests/integration/test_procfs.rs | 188 +++++------------- 2 files changed, 57 insertions(+), 155 deletions(-) diff --git a/crates/sandlock-core/src/procfs/metadata.rs b/crates/sandlock-core/src/procfs/metadata.rs index 3fcb6229..2df8463e 100644 --- a/crates/sandlock-core/src/procfs/metadata.rs +++ b/crates/sandlock-core/src/procfs/metadata.rs @@ -375,6 +375,13 @@ pub(crate) async fn handle_pinned_metadata( if request.size == 0 || request.size > i32::MAX as u64 { return NotifAction::Errno(libc::EINVAL); } + 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); + } let mut target = vec![0u8; (request.size as usize).min(4096)]; let count = unsafe { libc::readlinkat( @@ -385,22 +392,7 @@ pub(crate) async fn handle_pinned_metadata( ) }; if count < 0 { - let errno = std::io::Error::last_os_error() - .raw_os_error() - .unwrap_or(libc::EIO); - // An empty-path readlinkat on a non-link returns ENOENT, - // while the caller's non-empty pathname requires EINVAL. - // Preserve explicit empty-path requests and errors from - // actual links (e.g. a procfs link whose target vanished). - if !path.is_empty() && errno == libc::ENOENT { - let mut st: libc::stat = unsafe { std::mem::zeroed() }; - if unsafe { libc::fstat(fd.as_raw_fd(), &mut st) } == 0 - && st.st_mode & libc::S_IFMT != libc::S_IFLNK - { - return NotifAction::Errno(libc::EINVAL); - } - } - return NotifAction::Errno(errno); + return errno_action(); } if resolved == "/proc/self" { target = tgid.to_string().into_bytes(); diff --git a/crates/sandlock-core/tests/integration/test_procfs.rs b/crates/sandlock-core/tests/integration/test_procfs.rs index 3ea510e8..ca130aa9 100644 --- a/crates/sandlock-core/tests/integration/test_procfs.rs +++ b/crates/sandlock-core/tests/integration/test_procfs.rs @@ -1,5 +1,5 @@ use sandlock_core::sandbox::ByteSize; -use sandlock_core::{Protection, Sandbox}; +use sandlock_core::{Sandbox}; /// Test that num_cpus virtualizes both /proc/cpuinfo and sched_getaffinity. #[tokio::test] @@ -1239,158 +1239,68 @@ print('OK') assert!(result.success(), "{}", result.stderr_str().unwrap_or("")); } -// Preserve readlink errno, including the EINVAL that libc realpath expects -// for ordinary files and directories. -const READLINK_HELPERS: &str = r#" -import ctypes, errno, os, sys - -root = sys.argv[1] -os.chdir(root) -libc = ctypes.CDLL(None, use_errno=True) -libc.readlink.argtypes = [ctypes.c_char_p, ctypes.c_void_p, ctypes.c_size_t] -libc.readlink.restype = ctypes.c_ssize_t -libc.readlinkat.argtypes = [ctypes.c_int, ctypes.c_char_p, ctypes.c_void_p, ctypes.c_size_t] -libc.readlinkat.restype = ctypes.c_ssize_t -dirfd = os.open(root, os.O_RDONLY | os.O_DIRECTORY) - -def check_readlink(path, expected, fd=None, size=4096): - buf = ctypes.create_string_buffer(max(size, 1)) - ctypes.set_errno(0) - if fd is None: - count = libc.readlink(os.fsencode(path), buf, size) - else: - count = libc.readlinkat(fd, os.fsencode(path), buf, size) - if isinstance(expected, int): - actual = (count, ctypes.get_errno()) - assert actual == (-1, expected), (path, fd, size, actual, expected) - else: - actual = (count, buf.raw[:max(count, 0)]) - assert actual == (len(expected), expected), (path, fd, size, actual, expected) -"#; - -async fn run_readlink_test(body: &str) { +fn readlink_fixture() -> tempfile::TempDir { let root = tempfile::tempdir().unwrap(); - std::fs::write(root.path().join("file"), b"fixture\n").unwrap(); + std::fs::write(root.path().join("file"), b"").unwrap(); std::fs::create_dir(root.path().join("dir")).unwrap(); std::os::unix::fs::symlink("file", root.path().join("link")).unwrap(); std::os::unix::fs::symlink("missing", root.path().join("dangling")).unwrap(); - - let script = format!("{READLINK_HELPERS}\n{body}\nprint('OK')\n"); - // These metadata semantics do not require the newer Landlock rights. - // Unsupported rights may degrade, but the sandbox run must still succeed. - let mut policy = proc_grant() - .fs_read(root.path()) - .allow_degraded(Protection::NetTcp) - .allow_degraded(Protection::FsIoctlDev) - .allow_degraded(Protection::SignalScope) - .allow_degraded(Protection::AbstractUnixSocketScope) - .build() - .unwrap(); - let result = tokio::time::timeout( - std::time::Duration::from_secs(20), - policy.run(&["python3", "-c", &script, root.path().to_str().unwrap()]), - ) - .await - .expect("readlink regression test timed out") - .unwrap(); - assert!(result.success(), "{}", result.stderr_str().unwrap_or("")); - assert_eq!(result.stdout_str().unwrap_or("").trim(), "OK"); -} - -#[tokio::test] -async fn test_readlink_non_symlinks_return_einval() { - let script = r#" -for name in ['file', 'dir']: - absolute = os.path.join(root, name) - check_readlink(absolute, errno.EINVAL) - check_readlink(name, errno.EINVAL) - check_readlink(absolute, errno.EINVAL, dirfd) - check_readlink(name, errno.EINVAL, dirfd) -"#; - run_readlink_test(script).await; -} - -#[tokio::test] -async fn test_readlink_symlinks_and_path_errors() { - let script = r#" -for name, expected in [ - ('link', b'file'), - ('dangling', b'missing'), - ('missing', errno.ENOENT), - ('file/child', errno.ENOTDIR), -]: - check_readlink(os.path.join(root, name), expected) - check_readlink(name, expected, dirfd) -"#; - run_readlink_test(script).await; + root } +/// readlink on an existing non-link must fail with EINVAL, as it does +/// natively, even though the handler reads through an empty-path readlinkat, +/// for which the kernel reports ENOENT. #[tokio::test] -async fn test_readlinkat_empty_path_preserves_errno() { +async fn test_readlink_errno_matches_native() { + let root = readlink_fixture(); let script = r#" -# An explicit empty-path request on a non-link must retain ENOENT, not EINVAL. -for name, expected in [ - ('file', errno.ENOENT), - ('dir', errno.ENOENT), - ('link', b'file'), - ('dangling', b'missing'), -]: - fd = os.open(name, os.O_PATH | os.O_NOFOLLOW) - check_readlink('', expected, fd) +import os, sys, errno +root = sys.argv[1] +def check(path, expected, **kw): + try: actual = os.readlink(path, **kw) + except OSError as e: actual = e.errno + assert actual == expected, (path, kw, actual, expected) +dirfd = os.open(root, os.O_RDONLY | os.O_DIRECTORY) +for name, expected in [('file', errno.EINVAL), ('dir', errno.EINVAL), ('link', 'file'), + ('dangling', 'missing'), ('missing', errno.ENOENT), + ('file/child', errno.ENOTDIR)]: + check(os.path.join(root, name), expected) + check(name, expected, dir_fd=dirfd) +# An explicit empty path keeps the kernel's ENOENT for a non-link. +for name, expected in [('file', errno.ENOENT), ('dir', errno.ENOENT), ('link', 'file')]: + fd = os.open(os.path.join(root, name), os.O_PATH | os.O_NOFOLLOW) + check('', expected, dir_fd=fd) os.close(fd) -check_readlink('link', errno.EBADF, -1) -check_readlink('', errno.EBADF, -1) -"#; - run_readlink_test(script).await; -} - -#[tokio::test] -async fn test_readlink_buffer_sizes() { - let script = r#" -check_readlink('link', errno.EINVAL, size=0) -check_readlink('link', b'fi', size=2) -"#; - run_readlink_test(script).await; -} - -#[tokio::test] -async fn test_readlink_proc_links() { - let script = r#" -check_readlink('/proc/net', b'self/net') -check_readlink('/proc/self', str(os.getpid()).encode()) -fd = os.open('file', os.O_RDONLY) -check_readlink('/proc/self/fd/' + str(fd), os.fsencode(os.path.join(root, 'file'))) -os.close(fd) +check('/proc/self', str(os.getpid())) +check('/proc/self/fd/%d' % dirfd, root) +print('OK') "#; - run_readlink_test(script).await; + let mut policy = proc_grant().fs_read(root.path()).build().unwrap(); + let result = policy.run(&["python3", "-c", script, root.path().to_str().unwrap()]).await.unwrap(); + assert!(result.success(), "{}", result.stderr_str().unwrap_or("")); } +/// glibc realpath(3) takes EINVAL from readlink to mean "not a link" but +/// fails on ENOENT, so a wrong errno made existing paths unresolvable. +/// Python's os.path.realpath tolerates either, hence the libc call. #[tokio::test] async fn test_readlink_libc_realpath() { - // Python's os.path.realpath tolerates the wrong readlink errno, so call - // libc realpath itself to reproduce the reported regression on glibc. + let root = readlink_fixture(); let script = r#" -libc.realpath.argtypes = [ctypes.c_char_p, ctypes.c_void_p] -libc.realpath.restype = ctypes.c_void_p -libc.free.argtypes = [ctypes.c_void_p] -libc.free.restype = None -for name, target in [ - ('file', 'file'), - ('dir', 'dir'), - ('link', 'file'), - ('dangling', None), - ('missing', None), -]: - ctypes.set_errno(0) - resolved = libc.realpath(os.fsencode(os.path.join(root, name)), None) - if target is None: - assert not resolved and ctypes.get_errno() == errno.ENOENT, (name, ctypes.get_errno()) - else: - assert resolved, (name, ctypes.get_errno()) - actual = ctypes.string_at(resolved) - libc.free(resolved) - expected = os.fsencode(os.path.join(root, target)) - assert actual == expected, (name, actual, expected) +import ctypes, os, sys +root = sys.argv[1] +libc = ctypes.CDLL(None, use_errno=True) +libc.realpath.restype = ctypes.c_char_p +buf = ctypes.create_string_buffer(4096) +for name, target in [('file', 'file'), ('dir', 'dir'), ('link', 'file'), + ('dangling', None), ('missing', None)]: + resolved = libc.realpath(os.fsencode(os.path.join(root, name)), buf) + expected = target and os.fsencode(os.path.join(root, target)) + assert resolved == expected, (name, resolved, ctypes.get_errno()) +print('OK') "#; - run_readlink_test(script).await; + let mut policy = proc_grant().fs_read(root.path()).build().unwrap(); + let result = policy.run(&["python3", "-c", script, root.path().to_str().unwrap()]).await.unwrap(); + assert!(result.success(), "{}", result.stderr_str().unwrap_or("")); }