#1286

Use-after-free in io_uring

Opensfc-gh-rnarubinopened 6mo ago

The io_uring implementation is susceptible to use-after-free bugs. The buffers are passed to the kernel for reading/writing, but their ownership is moved to the returned future and can be freely dropped by user code:

        let (ptr, len) = buf.as_raw_parts();
        let rbuf = RawBuf { ptr, len };                             // <---- pointer to buf
        // ...
        let _ = shard.send(UringIoCtx {
            tx,
            io_type: UringIoType::Write,
            rbuf,                                                  // <---- pointer passed to other thread
            addr,
            #[cfg(feature = "tracing")]
            span,
        });
        async move {
            let res = match rx.await {
                Ok(res) => res,
                Err(e) => Err(Error::new(ErrorKind::ChannelClosed, "io completion channel closed").with_source(e)),
            };
            let buf: Box<dyn IoB> = buf.into_iob();        // <---- buf moved to future, future can be dropped immediately by user code
            (buf, res)
        }
        .boxed()
        .into()

I've written a reproduction case. It's naturally flaky, but with some iterations it can trigger memory corruption and show e.g. malloc failures

        /// Issue reads and immediately drop the handles, freeing the buffers while
        /// the kernel still holds raw pointers into them. The kernel write to freed
        /// memory can segfault (flaky, may require several attempts).
        #[tokio::test]
        async fn test_uring_read_use_after_free() {
            let dir = tempfile::tempdir().unwrap();
            let device = FsDeviceBuilder::new(dir)
                .build()
                .unwrap();
            device.create_partition(1024*1024).unwrap();

            let engine = UringIoEngineConfig::new()
                .boxed()
                .build(IoEngineBuildContext {
                    spawner: Spawner::current(),
                })
                .await
                .unwrap();

            // write to the file so there's something to read
            let mut wbuf = Box::new(IoSliceMut::new(16 * 1024));
            wbuf.fill(0xAB);
            let (_wbuf, res) = engine
                .write(wbuf, device.partition(0).as_ref(), 0)
                .await;
            res.unwrap();

            for _ in 0..64 {
                let buf = Box::new(IoSliceMut::new(16 * 1024));
                let handle = engine.read(buf, device.partition(0).as_ref(), 0);
                drop(handle);
            }

            // keep the test alive long enough for the IOs to complete
            tokio::time::sleep(std::time::Duration::from_millis(500)).await;
        }
malloc(): invalid size (unsorted)
error: test failed, to rerun pass `--lib`

Caused by:
  process didn't exit successfully: `/home/rnarubin/foyer/target/release/deps/foyer_storage-f3b14ac405db6d72` (signal: 6, SIGABRT: process abort signal)

Activity

1
MrCroxx6mo ago

Hi Renar. Nice to see you again here. Thank you for reportnig. Let me try to reproduce and fix it. ❤️