Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UB due to unsafe pinning in actix-codec #91

Closed
Tracked by #327
sebzim4500 opened this issue Jan 30, 2020 · 2 comments
Closed
Tracked by #327

UB due to unsafe pinning in actix-codec #91

sebzim4500 opened this issue Jan 30, 2020 · 2 comments
Labels
bug Something isn't working UB undefined behavior

Comments

@sebzim4500
Copy link

sebzim4500 commented Jan 30, 2020

The following code segfaults, due to incorrect pinning in Framed.

use futures::task::{noop_waker, Context, Poll};
use actix_codec::{Framed, AsyncRead, BytesCodec, AsyncWrite};
use std::pin::Pin;
use futures::io::Error;
use std::future::Future;
use pin_project::pin_project;

#[pin_project]
struct FakeSocket<F> {
    #[pin]
    inner: F
}

impl<F: Future> AsyncRead for FakeSocket<F> {
    fn poll_read(self: Pin<&mut Self>, cx: &mut Context<'_>, buf: &mut [u8]) -> Poll<Result<usize, Error>> {
        self.project().inner.poll(cx).map(|x| Ok(0))
    }
}

impl<F> AsyncWrite for FakeSocket<F> {
    fn poll_write(self: Pin<&mut Self>, cx: &mut Context<'_>, buf: &[u8]) -> Poll<Result<usize, Error>> {
        unimplemented!()
    }

    fn poll_flush(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<(), Error>> {
        unimplemented!()
    }

    fn poll_shutdown(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Result<(), Error>> {
        unimplemented!()
    }
}

fn main() {
    let (sender, receiver) = futures::channel::oneshot::channel();
    let mut framed: Result<_, [u8; 32]> = Ok(Framed::new(FakeSocket {
        inner: async {
            let x = Box::new(0);
            let y = &x;
            receiver.await.unwrap();
            let z = **y;
        }
    }, BytesCodec));

    let waker = noop_waker();
    let mut context = Context::from_waker(&waker);

    framed.as_mut().unwrap().next_item(&mut context);
    sender.send(()).unwrap();
    let _ = std::mem::replace(&mut framed, Err([0; 32])).unwrap().next_item(&mut context);
}
@robjtede
Copy link
Member

I think this is fixed in c41b5d8. The provided code doesn't compile anymore.

@Shnatsel
Copy link
Contributor

pin-project is sound, and there's no remaining unsafe in the Framed implementation, so this indeed should be fixed. Thanks to @Nemo157 for confirming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working UB undefined behavior
Projects
None yet
Development

No branches or pull requests

4 participants