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

io: fix take when using evil reader (backport #4428) #4434

Merged
merged 4 commits into from Jan 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion .cirrus.yml
Expand Up @@ -10,10 +10,11 @@ task:
env:
LOOM_MAX_PREEMPTIONS: 2
RUSTFLAGS: -Dwarnings
RUST_TOOLCHAIN: 1.56.0
setup_script:
- pkg install -y bash curl
- curl https://sh.rustup.rs -sSf --output rustup.sh
- sh rustup.sh -y --profile minimal --default-toolchain stable
- sh rustup.sh -y --profile minimal --default-toolchain $RUST_TOOLCHAIN
- . $HOME/.cargo/env
- rustup target add i686-unknown-freebsd
- |
Expand Down
110 changes: 64 additions & 46 deletions .github/workflows/ci.yml
Expand Up @@ -9,8 +9,10 @@ name: CI
env:
RUSTFLAGS: -Dwarnings
RUST_BACKTRACE: 1
nightly: nightly-2021-04-25
minrust: 1.45.2
rust_stable: 1.56.0
rust_nightly: nightly-2021-04-25
rust_clippy: 1.52.0
rust_min: 1.45.2

jobs:
# Depends on all action sthat are required for a "successful" CI run.
Expand Down Expand Up @@ -43,6 +45,11 @@ jobs:
- macos-latest
steps:
- uses: actions/checkout@v2
- name: Install Rust ${{ env.rust_stable }}
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.rust_stable }}
override: true
- name: Install Rust
run: rustup update stable
- name: Install cargo-hack
Expand Down Expand Up @@ -80,8 +87,11 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Install Rust
run: rustup update stable
- name: Install Rust ${{ env.rust_stable }}
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.rust_stable }}
override: true

- name: Install Valgrind
run: |
Expand Down Expand Up @@ -117,9 +127,11 @@ jobs:
- macos-latest
steps:
- uses: actions/checkout@v2
- name: Install Rust
run: rustup update stable

- name: Install Rust ${{ env.rust_stable }}
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.rust_stable }}
override: true
# Run `tokio` with "unstable" cfg flag.
- name: test tokio full --cfg unstable
run: cargo test --all-features
Expand All @@ -132,29 +144,28 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
- name: Install Rust ${{ env.rust_nightly }}
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.nightly }}
override: true
- name: Install Miri
toolchain: ${{ env.rust_nightly }}
override: true
components: miri
- name: miri
run: |
set -e
rustup component add miri
cargo miri setup
rm -rf tokio/tests

- name: miri
run: cargo miri test --features rt,rt-multi-thread,sync task
rm -rf tests
cargo miri test --features rt,rt-multi-thread,sync task
working-directory: tokio
san:
name: san
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
- name: Install Rust ${{ env.rust_nightly }}
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.nightly }}
override: true
toolchain: ${{ env.rust_nightly }}
override: true
- name: asan
run: cargo test --all-features --target x86_64-unknown-linux-gnu --lib -- --test-threads 1
working-directory: tokio
Expand All @@ -175,9 +186,10 @@ jobs:
- arm-linux-androideabi
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
- name: Install Rust ${{ env.rust_stable }}
uses: actions-rs/toolchain@v1
with:
toolchain: stable
toolchain: ${{ env.rust_stable }}
target: ${{ matrix.target }}
override: true
- uses: actions-rs/cargo@v1
Expand All @@ -191,16 +203,16 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
- name: Install Rust ${{ env.rust_nightly }}
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.nightly }}
toolchain: ${{ env.rust_nightly }}
target: ${{ matrix.target }}
override: true
- name: Install cargo-hack
run: cargo install cargo-hack

- name: check --each-feature
run: cargo hack check --all --each-feature -Z avoid-dev-deps

# Try with unstable feature flags
- name: check --each-feature --unstable
run: cargo hack check --all --each-feature -Z avoid-dev-deps
Expand All @@ -212,11 +224,11 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
- name: Install Rust ${{ env.rust_min }}
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.minrust }}
toolchain: ${{ env.rust_min }}
override: true

- name: "test --workspace --all-features"
run: cargo check --workspace --all-features

Expand All @@ -225,9 +237,10 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
- name: Install Rust ${{ env.rust_nightly }}
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.nightly }}
toolchain: ${{ env.rust_nightly }}
override: true
- name: Install cargo-hack
run: cargo install cargo-hack
Expand All @@ -245,11 +258,12 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Install Rust
run: rustup update stable
- name: Install rustfmt
run: rustup component add rustfmt

- name: Install Rust ${{ env.rust_stable }}
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.rust_stable }}
override: true
components: rustfmt
# Check fmt
- name: "rustfmt --check"
# Workaround for rust-lang/cargo#7732
Expand All @@ -264,10 +278,12 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Install Rust
run: rustup update 1.52.1 && rustup default 1.52.1
- name: Install clippy
run: rustup component add clippy
- name: Install Rust ${{ env.rust_clippy }}
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.rust_clippy }}
override: true
components: clippy

# Run clippy
- name: "clippy --all"
Expand All @@ -278,11 +294,11 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions-rs/toolchain@v1
- name: Install Rust ${{ env.rust_nightly }}
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.nightly }}
toolchain: ${{ env.rust_nightly }}
override: true

- name: "doc --lib --all-features"
run: cargo doc --lib --no-deps --all-features --document-private-items
env:
Expand All @@ -302,9 +318,11 @@ jobs:
- time::driver
steps:
- uses: actions/checkout@v2
- name: Install Rust
run: rustup update stable

- name: Install Rust ${{ env.rust_stable }}
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.rust_stable }}
override: true
- name: loom ${{ matrix.scope }}
run: cargo test --lib --release --features full -- --nocapture $SCOPE
working-directory: tokio
Expand Down
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -56,7 +56,7 @@ Make sure you activated the full features of the tokio crate on Cargo.toml:

```toml
[dependencies]
tokio = { version = "1.8.0", features = ["full"] }
tokio = { version = "1.8.5", features = ["full"] }
```
Then, on your main.rs:

Expand Down
29 changes: 29 additions & 0 deletions tokio/CHANGELOG.md
@@ -1,3 +1,32 @@
# 1.8.5 (January 27, 2022)

This release backports a bug fix from 1.16.0

Fixes a soundness bug in `io::Take` ([#4428]). The unsoundness is exposed when
leaking memory in the given `AsyncRead` implementation and then overwriting the
supplied buffer:

```rust
impl AsyncRead for Buggy {
fn poll_read(
self: Pin<&mut Self>,
cx: &mut Context<'_>,
buf: &mut ReadBuf<'_>
) -> Poll<Result<()>> {
let new_buf = vec![0; 5].leak();
*buf = ReadBuf::new(new_buf);
buf.put_slice(b"hello");
Poll::Ready(Ok(()))
}
}
```

### Fixed

- io: **soundness** don't expose uninitialized memory when using `io::Take` in edge case ([#4428])

[#4428]: https://github.com/tokio-rs/tokio/pull/4428

# 1.8.4 (November 15, 2021)

This release backports a bug fix from 1.13.1.
Expand Down
4 changes: 2 additions & 2 deletions tokio/Cargo.toml
Expand Up @@ -7,12 +7,12 @@ name = "tokio"
# - README.md
# - Update CHANGELOG.md.
# - Create "v1.0.x" git tag.
version = "1.8.4"
version = "1.8.5"
edition = "2018"
authors = ["Tokio Contributors <team@tokio.rs>"]
license = "MIT"
readme = "README.md"
documentation = "https://docs.rs/tokio/1.8.4/tokio/"
documentation = "https://docs.rs/tokio/1.8.5/tokio/"
repository = "https://github.com/tokio-rs/tokio"
homepage = "https://tokio.rs"
description = """
Expand Down
2 changes: 1 addition & 1 deletion tokio/LICENSE
@@ -1,4 +1,4 @@
Copyright (c) 2021 Tokio Contributors
Copyright (c) 2022 Tokio Contributors

Permission is hereby granted, free of charge, to any
person obtaining a copy of this software and associated
Expand Down
4 changes: 4 additions & 0 deletions tokio/src/io/util/take.rs
Expand Up @@ -86,7 +86,11 @@ impl<R: AsyncRead> AsyncRead for Take<R> {

let me = self.project();
let mut b = buf.take(*me.limit_ as usize);

let buf_ptr = b.filled().as_ptr();
ready!(me.inner.poll_read(cx, &mut b))?;
assert_eq!(b.filled().as_ptr(), buf_ptr);

let n = b.filled().len();

// We need to update the original ReadBuf
Expand Down
30 changes: 29 additions & 1 deletion tokio/tests/io_take.rs
@@ -1,7 +1,9 @@
#![warn(rust_2018_idioms)]
#![cfg(feature = "full")]

use tokio::io::AsyncReadExt;
use std::pin::Pin;
use std::task::{Context, Poll};
use tokio::io::{self, AsyncRead, AsyncReadExt, ReadBuf};
use tokio_test::assert_ok;

#[tokio::test]
Expand All @@ -14,3 +16,29 @@ async fn take() {
assert_eq!(n, 4);
assert_eq!(&buf, &b"hell\0\0"[..]);
}

struct BadReader;

impl AsyncRead for BadReader {
fn poll_read(
self: Pin<&mut Self>,
_cx: &mut Context<'_>,
read_buf: &mut ReadBuf<'_>,
) -> Poll<io::Result<()>> {
let vec = vec![0; 10];

let mut buf = ReadBuf::new(vec.leak());
buf.put_slice(&[123; 10]);
*read_buf = buf;

Poll::Ready(Ok(()))
}
}

#[tokio::test]
#[should_panic]
async fn bad_reader_fails() {
let mut buf = Vec::with_capacity(10);

BadReader.take(10).read_buf(&mut buf).await.unwrap();
}