diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 775007d14..08ba27820 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -20,7 +20,8 @@ Other improvements - History is no longer corrupted with NUL bytes when fish receives SIGTERM or SIGHUP (:issue:`10300`). - :doc:`fish_update_completions ` now handles groff ``\X'...'`` device control escapes, fixing completion generation for man pages produced by help2man 1.50 and later (such as coreutils 9.10). - Improve user experience when removing history entries via the :doc:`web-based config `. -- :doc:`funced ` will no longer lose work if there are parse errors multiple times without new changes to the file +- :doc:`funced ` will no longer lose work if there are parse errors multiple times without new changes to the file. +- Move some internal file descriptors to number 10 or higher to reduce risk of clashes with those used by the user in scripts. For distributors and developers ------------------------------- diff --git a/src/bin/fish.rs b/src/bin/fish.rs index 0fdc730f3..d1b246a0f 100644 --- a/src/bin/fish.rs +++ b/src/bin/fish.rs @@ -32,6 +32,7 @@ }, eprintf, err_fmt, event::{self, Event}, + fds::heightenize_fd, flog::{self, activate_flog_categories_by_pattern, flog, flogf, set_flog_file_fd}, fprintf, function, history::{self, start_private_mode}, @@ -564,11 +565,11 @@ fn throwing_main() -> i32 { } res = reader_read(parser, libc::STDIN_FILENO, &IoChain::new()); } else { - let n = wcs2bytes(&args[my_optind]); + let filename = &args[my_optind]; + let n = wcs2bytes(filename); let path = OsStr::from_bytes(&n); my_optind += 1; // Rust sets cloexec by default, see above - // We don't need autoclose_fd_t when we use File, it will be closed on drop. match File::open(path) { Err(e) => { flogf!( @@ -579,24 +580,25 @@ fn throwing_main() -> i32 { eprintf!("%s\n", e); } Ok(f) => { - let list = &args[my_optind..]; - parser.set_var( - L!("argv"), - ParserEnvSetMode::default(), - list.iter().map(|s| s.to_owned()).collect(), - ); - let rel_filename = &args[my_optind - 1]; - let _filename_push = parser - .library_data - .scoped_set(Some(Arc::new(rel_filename.to_owned())), |s| { - &mut s.current_filename - }); - res = reader_read(parser, f.as_raw_fd(), &IoChain::new()); - if res.is_err() { - flog!( - warning, - wgettext_fmt!("Error while reading file %s", path.to_string_lossy()) + if let Ok(f) = heightenize_fd(f.into(), true).map(File::from) { + let list = &args[my_optind..]; + parser.set_var( + L!("argv"), + ParserEnvSetMode::default(), + list.iter().map(|s| s.to_owned()).collect(), ); + let _filename_push = parser + .library_data + .scoped_set(Some(Arc::new(filename.to_owned())), |s| { + &mut s.current_filename + }); + res = reader_read(parser, f.as_raw_fd(), &IoChain::new()); + if res.is_err() { + flog!( + warning, + wgettext_fmt!("Error while reading file %s", path.to_string_lossy()) + ); + } } } } diff --git a/src/fd_monitor.rs b/src/fd_monitor.rs index 95f806967..81502a5f3 100644 --- a/src/fd_monitor.rs +++ b/src/fd_monitor.rs @@ -1,4 +1,5 @@ use crate::fd_readable_set::{FdReadableSet, Timeout}; +use crate::fds::heightenize_fd; use crate::flog::flog; use crate::portable_atomic::AtomicU64; use crate::threads::assert_is_background_thread; @@ -49,8 +50,12 @@ pub fn new() -> Self { perror("eventfd"); exit_without_destructors(1); } + let Ok(fd) = heightenize_fd(unsafe { OwnedFd::from_raw_fd(fd) }, true) else { + perror("eventfd"); + exit_without_destructors(1); + }; Self { - fd: unsafe { OwnedFd::from_raw_fd(fd) }, + fd } } else { // Implementation using pipes. diff --git a/src/fds.rs b/src/fds.rs index e5b933cac..938f901ba 100644 --- a/src/fds.rs +++ b/src/fds.rs @@ -86,7 +86,7 @@ pub fn make_autoclose_pipes() -> nix::Result { /// setting it again. /// Return the fd, which always has CLOEXEC set; or an invalid fd on failure, in /// which case an error will have been printed, and the input fd closed. -fn heightenize_fd(fd: OwnedFd, input_has_cloexec: bool) -> nix::Result { +pub fn heightenize_fd(fd: OwnedFd, input_has_cloexec: bool) -> nix::Result { let raw_fd = fd.as_raw_fd(); if raw_fd >= FIRST_HIGH_FD { @@ -145,7 +145,7 @@ pub fn open_cloexec(path: &CStr, flags: OFlag, mode: nix::sys::stat::Mode) -> ni // If it is that's our cancel signal, so we abort. loop { let ret = nix::fcntl::open(path, flags | OFlag::O_CLOEXEC, mode); - let ret = ret.map(File::from); + let ret = ret.and_then(|fd| heightenize_fd(fd, true)).map(File::from); match ret { Ok(file) => { return Ok(file); diff --git a/src/universal_notifier/inotify.rs b/src/universal_notifier/inotify.rs index d28f6bd30..8b812ba24 100644 --- a/src/universal_notifier/inotify.rs +++ b/src/universal_notifier/inotify.rs @@ -1,11 +1,12 @@ use crate::env_universal_common::default_vars_path; +use crate::fds::heightenize_fd; use crate::prelude::*; use crate::universal_notifier::UniversalNotifier; use crate::wutil::{wbasename, wdirname}; use fish_widestring::wcs2osstring; use nix::sys::inotify::{AddWatchFlags, InitFlags, Inotify}; use std::ffi::OsString; -use std::os::fd::{AsFd as _, AsRawFd as _, RawFd}; +use std::os::fd::{AsFd as _, AsRawFd as _, OwnedFd, RawFd}; /// A notifier based on inotify. pub struct InotifyNotifier { @@ -30,6 +31,9 @@ pub fn new_at(path: &wstr) -> Option { let dirname = wdirname(path); let basename = wbasename(path); let inotify = Inotify::init(InitFlags::IN_CLOEXEC | InitFlags::IN_NONBLOCK).ok()?; + let inotify = heightenize_fd(OwnedFd::from(inotify), true).ok()?; + // SAFETY: We pass a valid inotify fd. + let inotify = unsafe { Inotify::from_owned_fd(inotify) }; inotify .add_watch( wcs2osstring(dirname).as_os_str(), diff --git a/src/universal_notifier/kqueue.rs b/src/universal_notifier/kqueue.rs index 511119293..e309070a2 100644 --- a/src/universal_notifier/kqueue.rs +++ b/src/universal_notifier/kqueue.rs @@ -1,4 +1,5 @@ use crate::env_universal_common::default_vars_path; +use crate::fds::heightenize_fd; use crate::flogf; use crate::prelude::*; use crate::universal_notifier::UniversalNotifier; @@ -6,7 +7,7 @@ use fish_widestring::wcs2osstring; use nix::sys::event::{EvFlags, EventFilter, FilterFlag, KEvent, Kqueue}; use std::fs::File; -use std::os::fd::AsFd; +use std::os::fd::{AsFd, OwnedFd}; use std::os::unix::fs::MetadataExt; use std::os::unix::io::{AsRawFd, RawFd}; use std::path::PathBuf; @@ -48,6 +49,7 @@ pub fn new_at(path: &wstr) -> Option { // Open the directory to get a valid file descriptor or bail if it doesn't exist let dir_fd = File::open(dir.as_os_str()).ok()?; + let dir_fd = File::from(heightenize_fd(OwnedFd::from(dir_fd), true).ok()?); // Add a watch for EVFILT_VNODE events let change_event = KEvent::new( diff --git a/tests/checks/fds.fish b/tests/checks/fds.fish index a86a223f4..8166fd44f 100644 --- a/tests/checks/fds.fish +++ b/tests/checks/fds.fish @@ -1,4 +1,4 @@ -# RUN: %fish -C "set helper %fish_test_helper" %s +# RUN: fish=%fish %fish -C "set helper %fish_test_helper" %s #REQUIRES: command -v %fish_test_helper # Check that we don't leave stray FDs. @@ -89,3 +89,39 @@ $helper print_fds 19$tmpfile" creates a temporary fd on a fd n +# (n being the first free descriptor) during parsing, then "n>&1" redirects fd n +# to fd 1 (still stdout at this stage), lastly, ">$tmpfile" redirects fd 1 is +# redirected to the "temporary" fd n (which is now stdout instead of $tmpfile). +# +# If heightenization does work, ">$tmpfile" creates a temporary fd on 10+, +# then fd n is redirected to fd 1 (still stdout), the fd 1 is redirect to +# the "temporary" fd 10+ (still $tmpfile) +# +# Since we don't know what `n` will be, we need to redirect all the descriptors +# reserved for the user +set -l tmpfile (mktemp) +printf 'stdout: %s\n' "$($fish -c "$helper print_fds 3>&1 4>&1 5>&1 6>&1 7>&1 8>&1 9>&1 >$tmpfile")" +printf "tmp file: %s\n" $(cat $tmpfile) +# CHECK: stdout: +# CHECK: tmp file: 0 1 2 3 4 5 6 7 8 9 + +# Check heightenization of the file descriptors when sourcing +echo ' +path filter /proc/$fish_pid/fd/{3,4,5,6,7,8,9} +' >"$tmpfile" +echo start source +$fish -c "source '$tmpfile'" +echo end source +# CHECK: start source +# CHECK: end source + +# Check heightenization of descriptors for script files passed to fish +echo start fish +$fish "$tmpfile" +echo end fish +# CHECK: start fish +# CHECK: end fish