From 91a9f83dd1d73cfd451f81306361df3fafad84a5 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 16 Oct 2020 09:09:20 -0700 Subject: [PATCH 1/5] Define `fs::hard_link` to not follow symlinks. POSIX leaves it implementation-defined whether `link` follows symlinks. In practice, for example, on Linux it does not and on FreeBSD it does. So, switch to `linkat`, so that we can pick a behavior rather than depending on OS defaults. Pick the option to not follow symlinks. This is somewhat arbitrary, but seems the less surprising choice because hard linking is a very low-level feature which requires the source and destination to be on the same mounted filesystem, and following a symbolic link could end up in a different mounted filesystem. --- library/std/src/fs.rs | 7 +++-- library/std/src/fs/tests.rs | 51 ++++++++++++++++++++++++++++++++++ library/std/src/sys/unix/fs.rs | 5 +++- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 161bfe3795c2c..c611bf4d74a49 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1701,10 +1701,13 @@ pub fn copy, Q: AsRef>(from: P, to: Q) -> io::Result { /// The `dst` path will be a link pointing to the `src` path. Note that systems /// often require these two paths to both be located on the same filesystem. /// +/// If `src` names a symbolic link, it is not followed. The created hard link +/// points to the symbolic link itself. +/// /// # Platform-specific behavior /// -/// This function currently corresponds to the `link` function on Unix -/// and the `CreateHardLink` function on Windows. +/// This function currently corresponds to the `linkat` function with no flags +/// on Unix and the `CreateHardLink` function on Windows. /// Note that, this [may change in the future][changes]. /// /// [changes]: io#platform-specific-behavior diff --git a/library/std/src/fs/tests.rs b/library/std/src/fs/tests.rs index 65a29076fefa8..8a723d3b4ae22 100644 --- a/library/std/src/fs/tests.rs +++ b/library/std/src/fs/tests.rs @@ -1337,3 +1337,54 @@ fn metadata_access_times() { } } } + +/// Test creating hard links to symlinks. +#[test] +fn symlink_hard_link() { + let tmpdir = tmpdir(); + + // Create "file", a file. + check!(fs::File::create(tmpdir.join("file"))); + + // Create "symlink", a symlink to "file". + check!(symlink_file("file", tmpdir.join("symlink"))); + + // Create "hard_link", a hard link to "symlink". + check!(fs::hard_link(tmpdir.join("symlink"), tmpdir.join("hard_link"))); + + // "hard_link" should appear as a symlink. + assert!(check!(fs::symlink_metadata(tmpdir.join("hard_link"))).file_type().is_symlink()); + + // We sould be able to open "file" via any of the above names. + let _ = check!(fs::File::open(tmpdir.join("file"))); + assert!(fs::File::open(tmpdir.join("file.renamed")).is_err()); + let _ = check!(fs::File::open(tmpdir.join("symlink"))); + let _ = check!(fs::File::open(tmpdir.join("hard_link"))); + + // Rename "file" to "file.renamed". + check!(fs::rename(tmpdir.join("file"), tmpdir.join("file.renamed"))); + + // Now, the symlink and the hard link should be dangling. + assert!(fs::File::open(tmpdir.join("file")).is_err()); + let _ = check!(fs::File::open(tmpdir.join("file.renamed"))); + assert!(fs::File::open(tmpdir.join("symlink")).is_err()); + assert!(fs::File::open(tmpdir.join("hard_link")).is_err()); + + // The symlink and the hard link should both still point to "file". + assert!(fs::read_link(tmpdir.join("file")).is_err()); + assert!(fs::read_link(tmpdir.join("file.renamed")).is_err()); + assert_eq!(check!(fs::read_link(tmpdir.join("symlink"))), Path::new("file")); + assert_eq!(check!(fs::read_link(tmpdir.join("hard_link"))), Path::new("file")); + + // Remove "file.renamed". + check!(fs::remove_file(tmpdir.join("file.renamed"))); + + // Now, we can't open the file by any name. + assert!(fs::File::open(tmpdir.join("file")).is_err()); + assert!(fs::File::open(tmpdir.join("file.renamed")).is_err()); + assert!(fs::File::open(tmpdir.join("symlink")).is_err()); + assert!(fs::File::open(tmpdir.join("hard_link")).is_err()); + + // "hard_link" should still appear as a symlink. + assert!(check!(fs::symlink_metadata(tmpdir.join("hard_link"))).file_type().is_symlink()); +} diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 819e8ef18415b..88693e4786c0f 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1067,7 +1067,10 @@ pub fn symlink(src: &Path, dst: &Path) -> io::Result<()> { pub fn link(src: &Path, dst: &Path) -> io::Result<()> { let src = cstr(src)?; let dst = cstr(dst)?; - cvt(unsafe { libc::link(src.as_ptr(), dst.as_ptr()) })?; + // Use `linkat` with `AT_FDCWD` instead of `link` as `link` leaves it + // implmentation-defined whether it follows symlinks. Pass 0 as the + // `linkat` flags argument so that we don't follow symlinks. + cvt(unsafe { libc::linkat(libc::AT_FDCWD, src.as_ptr(), libc::AT_FDCWD, dst.as_ptr(), 0) })?; Ok(()) } From 23a5c214150f462043ab411f87ef297309421d71 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 19 Oct 2020 07:21:41 -0700 Subject: [PATCH 2/5] Fix a typo in a comment. --- library/std/src/sys/unix/fs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 88693e4786c0f..69f6b88a3bc56 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1068,7 +1068,7 @@ pub fn link(src: &Path, dst: &Path) -> io::Result<()> { let src = cstr(src)?; let dst = cstr(dst)?; // Use `linkat` with `AT_FDCWD` instead of `link` as `link` leaves it - // implmentation-defined whether it follows symlinks. Pass 0 as the + // implementation-defined whether it follows symlinks. Pass 0 as the // `linkat` flags argument so that we don't follow symlinks. cvt(unsafe { libc::linkat(libc::AT_FDCWD, src.as_ptr(), libc::AT_FDCWD, dst.as_ptr(), 0) })?; Ok(()) From ce00b3e2e0c5c0c88fe59fb45a1c25a8ff9e1836 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Mon, 19 Oct 2020 07:47:32 -0700 Subject: [PATCH 3/5] Use `link` on platforms which lack `linkat`. --- library/std/src/sys/unix/fs.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 69f6b88a3bc56..bf4c941928719 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1067,10 +1067,20 @@ pub fn symlink(src: &Path, dst: &Path) -> io::Result<()> { pub fn link(src: &Path, dst: &Path) -> io::Result<()> { let src = cstr(src)?; let dst = cstr(dst)?; - // Use `linkat` with `AT_FDCWD` instead of `link` as `link` leaves it - // implementation-defined whether it follows symlinks. Pass 0 as the - // `linkat` flags argument so that we don't follow symlinks. - cvt(unsafe { libc::linkat(libc::AT_FDCWD, src.as_ptr(), libc::AT_FDCWD, dst.as_ptr(), 0) })?; + cfg_if::cfg_if! { + if #[cfg(any(target_os = "vxworks", target_os = "redox"))] { + // VxWorks and Redox lack `linkat`, so use `link` instead. POSIX + // leaves it implementation-defined whether `link` follows symlinks, + // so rely on the `symlink_hard_link` test in + // library/std/src/fs/tests.rs to check the behavior. + cvt(unsafe { libc::link(src.as_ptr(), dst.as_ptr()) })?; + } else { + // Use `linkat` with `AT_FDCWD` instead of `link` as `linkat` gives + // us a flag to specify how symlinks should be handled. Pass 0 as + // the flags argument, meaning don't follow symlinks. + cvt(unsafe { libc::linkat(libc::AT_FDCWD, src.as_ptr(), libc::AT_FDCWD, dst.as_ptr(), 0) })?; + } + } Ok(()) } From d0178b4f99c70d2443f9b76421429d0d23dadc45 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 20 Oct 2020 16:42:31 -0700 Subject: [PATCH 4/5] Make it platform-specific whether `hard_link` follows symlinks. Also mention that where possible, `hard_link` does not follow symlinks. --- library/std/src/fs.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index c611bf4d74a49..c256f556b3c8f 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1701,8 +1701,9 @@ pub fn copy, Q: AsRef>(from: P, to: Q) -> io::Result { /// The `dst` path will be a link pointing to the `src` path. Note that systems /// often require these two paths to both be located on the same filesystem. /// -/// If `src` names a symbolic link, it is not followed. The created hard link -/// points to the symbolic link itself. +/// If `src` names a symbolic link, it is platform-specific whether the symbolic +/// link is followed. On platforms where it's possible to not follow it, it is +/// not followed, and the created hard link points to the symbolic link itself. /// /// # Platform-specific behavior /// From 6249cda78f0cd32b60fb11702b7ffef3e3bab0b2 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 23 Oct 2020 15:39:02 -0700 Subject: [PATCH 5/5] Disable use of `linkat` on Android as well. According to [the bionic status page], `linkat` has only been available since API level 21. Since Android is based on Linux and Linux's `link` doesn't follow symlinks, just use `link` on Android. [the bionic status page]: https://android.googlesource.com/platform/bionic/+/master/docs/status.md --- library/std/src/sys/unix/fs.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index bf4c941928719..ec721fccaa622 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -1068,11 +1068,11 @@ pub fn link(src: &Path, dst: &Path) -> io::Result<()> { let src = cstr(src)?; let dst = cstr(dst)?; cfg_if::cfg_if! { - if #[cfg(any(target_os = "vxworks", target_os = "redox"))] { - // VxWorks and Redox lack `linkat`, so use `link` instead. POSIX - // leaves it implementation-defined whether `link` follows symlinks, - // so rely on the `symlink_hard_link` test in - // library/std/src/fs/tests.rs to check the behavior. + if #[cfg(any(target_os = "vxworks", target_os = "redox", target_os = "android"))] { + // VxWorks, Redox, and old versions of Android lack `linkat`, so use + // `link` instead. POSIX leaves it implementation-defined whether + // `link` follows symlinks, so rely on the `symlink_hard_link` test + // in library/std/src/fs/tests.rs to check the behavior. cvt(unsafe { libc::link(src.as_ptr(), dst.as_ptr()) })?; } else { // Use `linkat` with `AT_FDCWD` instead of `link` as `linkat` gives