Tokio ICE fix: Changed the type of EpollEventInterest::epfd from i32 to WeakFileDescriptionRef

Author: tiif

src/tools/miri/src/shims/unix/linux/epoll.rs

@@ -51,6 +51,9 @@ impl EpollEventInstance {
 #[derive(Clone, Debug)]
 pub struct EpollEventInterest {
     /// The file descriptor value of the file description registered.
+    /// This is only used for ready_list, to inform userspace which FD triggered an event.
+    /// For that, it is crucial to preserve the original FD number.
+    /// This FD number must never be "dereferenced" to a file description inside Miri.
     fd_num: i32,
     /// The events bitmask retrieved from `epoll_event`.
     events: u32,
@@ -61,8 +64,8 @@ pub struct EpollEventInterest {
     data: u64,
     /// Ready list of the epoll instance under which this EpollEventInterest is registered.
     ready_list: Rc<RefCell<BTreeMap<(FdId, i32), EpollEventInstance>>>,
-    /// The file descriptor value that this EpollEventInterest is registered under.
-    epfd_num: i32,
+    /// The epoll file description that this EpollEventInterest is registered under.
+    weak_epfd: WeakFileDescriptionRef,
 }
 
 /// EpollReadyEvents reflects the readiness of a file description.
@@ -343,7 +346,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 events,
                 data,
                 ready_list: Rc::clone(ready_list),
-                epfd_num: epfd_value,
+                weak_epfd: epfd.downgrade(),
             }));
 
             if op == epoll_ctl_add {
@@ -553,12 +556,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     if is_updated {
                         // Edge-triggered notification only notify one thread even if there are
                         // multiple threads block on the same epfd.
-                        let epfd = this.machine.fds.get(epoll_interest.borrow().epfd_num).unwrap();
 
                         // This unwrap can never fail because if the current epoll instance were
-                        // closed and its epfd value reused, the upgrade of weak_epoll_interest
+                        // closed, the upgrade of weak_epoll_interest
                         // above would fail. This guarantee holds because only the epoll instance
                         // holds a strong ref to epoll_interest.
+                        let epfd = epoll_interest.borrow().weak_epfd.upgrade().unwrap();
                         // FIXME: We can randomly pick a thread to unblock.
                         if let Some(thread_id) =
                             epfd.downcast::<Epoll>().unwrap().thread_id.borrow_mut().pop()

src/tools/miri/tests/pass-dep/libc/libc-epoll-no-blocking.rs

@@ -23,6 +23,7 @@ fn main() {
     test_ready_list_fetching_logic();
     test_epoll_ctl_epfd_equal_fd();
     test_epoll_ctl_notification();
+    test_issue_3858();
 }
 
 // Using `as` cast since `EPOLLET` wraps around
@@ -683,3 +684,40 @@ fn test_epoll_ctl_notification() {
     // for this epfd, because there is no I/O event between the two epoll_wait.
     check_epoll_wait::<1>(epfd0, &[]);
 }
+
+// Test for ICE caused by weak epoll interest upgrade succeed, but the attempt to retrieve
+// the epoll instance based on the epoll file descriptor value failed. EpollEventInterest
+// should store a WeakFileDescriptionRef instead of the file descriptor number, so if the
+// epoll instance is duped, it'd still be usable after `close` is called on the original
+// epoll file descriptor.
+// https://github.com/rust-lang/miri/issues/3858
+fn test_issue_3858() {
+    // Create an eventfd instance.
+    let flags = libc::EFD_NONBLOCK | libc::EFD_CLOEXEC;
+    let fd = unsafe { libc::eventfd(0, flags) };
+
+    // Create an epoll instance.
+    let epfd = unsafe { libc::epoll_create1(0) };
+    assert_ne!(epfd, -1);
+
+    // Register eventfd with EPOLLIN | EPOLLET.
+    let mut ev = libc::epoll_event {
+        events: (libc::EPOLLIN | libc::EPOLLET) as _,
+        u64: u64::try_from(fd).unwrap(),
+    };
+    let res = unsafe { libc::epoll_ctl(epfd, libc::EPOLL_CTL_ADD, fd, &mut ev) };
+    assert_eq!(res, 0);
+
+    // Dup the epoll instance.
+    let newfd = unsafe { libc::dup(epfd) };
+    assert_ne!(newfd, -1);
+
+    // Close the old epoll instance, so the new FD is now the only FD.
+    let res = unsafe { libc::close(epfd) };
+    assert_eq!(res, 0);
+
+    // Write to the eventfd instance.
+    let sized_8_data: [u8; 8] = 1_u64.to_ne_bytes();
+    let res = unsafe { libc::write(fd, sized_8_data.as_ptr() as *const libc::c_void, 8) };
+    assert_eq!(res, 8);
+}