wgpu icon indicating copy to clipboard operation
wgpu copied to clipboard

bounds checks for texture-buffer copies happen too late

Open jimblandy opened this issue 1 year ago • 3 comments

Methods like command_encoder_copy_texture_to_buffer do check that ImageCopyTexture::mip_level field of their source and/or destination parameters is valid for the relevant texture, by calling validate_texture_copy_range. However, those checks occur after those functions have called TextureTracker::set_single, which assumes that the mip_level is in range.

The specific call chain is:

  • Global::command_encoder_copy_texture_to_buffer, or related
  • TextureTracker::set_single
  • wgpu_core::track::texture::insert_or_barrier_single
  • wgpu_core::track::texture::insert
  • ComplexTextureState::from_selector_state_iter

This last uses get_unchecked_mut on selector.mips.start.

cc @cwfitzgerald

jimblandy avatar Aug 08 '22 23:08 jimblandy

I don't understand all the plumbing here yet, but it seems like we ought to be able to do the validation in extract_texture_selector instead?

jimblandy avatar Aug 08 '22 23:08 jimblandy

Current plan is to fuse extract_texture_selector and validate_texture_copy_range into a single function which does all bounds checks, and call that before all resource tracking operations.

jimblandy avatar Aug 18 '22 00:08 jimblandy

for the curious, work in progress is here: https://github.com/jimblandy/wgpu/tree/early-texture-bounds-checks

jimblandy avatar Aug 18 '22 00:08 jimblandy

This specific case happens to not affect copy_buffer_to_texture, because that calls wgpu_core::command::transfer::handle_dst_texture_init, which calls has_copy_partial_init_tracker_coverage, which happens to check the destination mip level against the texture itself. The corresponding function for copies from textures is handle_src_texture_init, which doesn't make any such call.

jimblandy avatar Oct 06 '22 22:10 jimblandy

@nical was working on this; may be fixed now.

jimblandy avatar Jan 04 '24 15:01 jimblandy