fd: reopen: clear O_NOFOLLOW even if provided

Since reopening symlinks is forbidden, we can safely ignore O_NOFOLLOW
as it is conceptually a no-op (but it causes practical issues because
reopening works through magic-links).

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

CHANGELOG.md

@@ -27,6 +27,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
   you would get various errors and unexpected behaviour. If you wish to make an
   `O_PATH|O_NOFOLLOW` copy of a symlink handle, you can simply use `try_clone`
   (i.e. `dup(2)` the file descriptor).
+- `Handle::reopen(O_NOFOLLOW)` will now return reasonable results. Previously,
+  it would return `-ELOOP` in most cases and in other cases it would return
+  unexpected results because the `O_NOFOLLOW` would have an effect on the
+  magic-link used internally by `Handle::reopen`.
 
 ### Changed ###
 - syscalls: switch to rustix for most of our syscall wrappers to simplify how

src/tests/common/handle.rs

@@ -132,7 +132,12 @@ pub(in crate::tests) fn check_reopen<H: HandleImpl>(
         "cloned handle should be equivalent to old handle",
     );
 
-    check_oflags(&file, flags, H::FORCED_CLOEXEC)?;
+    check_oflags(
+        &file,
+        // NOTE: Handle::reopen() drops O_NOFOLLOW, so we shouldn't see it.
+        flags.difference(OpenFlags::O_NOFOLLOW),
+        H::FORCED_CLOEXEC,
+    )?;
 
     Ok(())
 }

src/utils/fd.rs

@@ -190,7 +190,7 @@ impl<Fd: AsFd> FdExt for Fd {
         Ok(Metadata(stat))
     }
 
-    fn reopen(&self, procfs: &ProcfsHandle, flags: OpenFlags) -> Result<OwnedFd, Error> {
+    fn reopen(&self, procfs: &ProcfsHandle, mut flags: OpenFlags) -> Result<OwnedFd, Error> {
         let fd = self.as_fd();
 
         // For file descriptors referencing a symlink (i.e. opened with
@@ -207,6 +207,11 @@ impl<Fd: AsFd> FdExt for Fd {
             .wrap("symlink file handles cannot be reopened")?
         }
 
+        // Now that we are sure the file descriptor is not a symlink, we can
+        // clear O_NOFOLLOW since it is a no-op (but due to the procfs reopening
+        // implementation, O_NOFOLLOW will cause strange behaviour).
+        flags.remove(OpenFlags::O_NOFOLLOW);
+
         // TODO: Add support for O_EMPTYPATH once that exists...
         procfs
             .open_follow(ProcfsBase::ProcThreadSelf, proc_subpath(fd)?, flags)