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

attempt to subtract with overflow in vello-encoding when drawing MANY images, resulting in potential OOB access #788

Open
BloodStainedCrow opened this issue Jan 14, 2025 · 1 comment

Comments

@BloodStainedCrow
Copy link

BloodStainedCrow commented Jan 14, 2025

Vello Version: 0.3.0
Rust Version: 1.86.0-nightly (2025-01-11)

Adapting the example in examples/simple with

fn add_shapes_to_scene(scene: &mut Scene) {
    let blob = Blob::new(Arc::new(vec![
        200;
        vello::peniko::Format::Rgba8
            .size_in_bytes(100, 100)
            .unwrap()
    ]));

    let img = Image::new(blob, vello::peniko::Format::Rgba8, 100, 100);

    for i in 0..1_000_000 {
        scene.draw_image(&img, Affine::IDENTITY);
    }
}

results in a overflow panic in debug mode:

thread 'main' panicked at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/vello_encoding-0.3.0/src/config.rs:186:31:
attempt to subtract with overflow
stack backtrace:
   0: rust_begin_unwind
             at /rustc/eb54a50837ad4bcc9842924f27e7287ca66e294c/library/std/src/panicking.rs:692:5
   1: core::panicking::panic_fmt
             at /rustc/eb54a50837ad4bcc9842924f27e7287ca66e294c/library/core/src/panicking.rs:75:14
   2: core::panicking::panic_const::panic_const_sub_overflow
             at /rustc/eb54a50837ad4bcc9842924f27e7287ca66e294c/library/core/src/panicking.rs:178:21
   3: vello_encoding::config::RenderConfig::new
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/vello_encoding-0.3.0/src/config.rs:186:31
   4: vello::render::Render::render_encoding_coarse
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/vello-0.3.0/src/render.rs:160:13
   5: vello::render::render_encoding_full
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/vello-0.3.0/src/render.rs:100:25
   6: vello::render::render_full
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/vello-0.3.0/src/render.rs:85:5
   7: vello::Renderer::render_to_texture
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/vello-0.3.0/src/lib.rs:423:13
   8: vello::Renderer::render_to_surface
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/vello-0.3.0/src/lib.rs:467:9
   9: <factory::rendering::SimpleVelloApp as winit::application::ApplicationHandler>::window_event
             at ./src/rendering/mod.rs:141:17
  10: winit::event_loop::dispatch_event_for_app
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/winit-0.30.8/src/event_loop.rs:642:52
  11: winit::event_loop::EventLoop<T>::run_app::{{closure}}
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/winit-0.30.8/src/event_loop.rs:265:49
  12: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &mut F>::call_mut
             at /nix/store/cl98jzwbz3rblf4sx7cif8l5qmax2kzy-rust-default-1.86.0-nightly-2025-01-12/lib/rustlib/src/rust/library/core/src/ops/function.rs:294:13
  13: winit::platform_impl::linux::wayland::event_loop::EventLoop<T>::single_iteration
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/winit-0.30.8/src/platform_impl/linux/wayland/event_loop/mod.rs:469:17
  14: winit::platform_impl::linux::wayland::event_loop::EventLoop<T>::pump_events
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/winit-0.30.8/src/platform_impl/linux/wayland/event_loop/mod.rs:211:13
  15: winit::platform_impl::linux::wayland::event_loop::EventLoop<T>::run_on_demand
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/winit-0.30.8/src/platform_impl/linux/wayland/event_loop/mod.rs:181:19
  16: winit::platform_impl::linux::EventLoop<T>::run_on_demand
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/winit-0.30.8/src/platform_impl/linux/mod.rs:819:56
  17: winit::platform_impl::linux::EventLoop<T>::run
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/winit-0.30.8/src/platform_impl/linux/mod.rs:812:9
  18: winit::event_loop::EventLoop<T>::run_app
             at /home/tim/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/winit-0.30.8/src/event_loop.rs:265:9

The underflow happens while calculating binning_size: buffer_sizes.bin_data.len() - layout.bin_data_start.

In Release mode the program does not panic (since overflow checks are disabled), and the images do not appear.

If I understand this correctly, this value is used as the size to some buffer, it under-/overflowing will result in potential OOB accesses on the gpu, which would explain why nothing gets rendered.

While I doubt drawing 1_000_000 images in this way is a typical usecase, it seems to me like something fishy is happening here.

@DJMcNab
Copy link
Member

DJMcNab commented Jan 15, 2025

We definitely do not support that many images. Our current support is limited by the maximum atlas size (which is up to 8192x8192 pixels).

I believe that the underflow there is ultimately part of #366, and so could be resolved if we can find bandwidth to move that forward. That probably also explains the failure to ever render a frame, as we're hitting the fallback added in #537.

If I understand this correctly, this value is used as the size to some buffer, it under-/overflowing will result in potential OOB accesses on the gpu, which would explain why nothing gets rendered.

If you have a proof of concept of an underflow leading to out of bounds access on the GPU, it will likely need to go through a responsible disclosure process in wgpu (which adds the bounds checking we are using).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants