Skip to content

Commit 87b55de

Browse files
committed
Also don't allow index buffers in ram without robust access
1 parent c617d6f commit 87b55de

File tree

7 files changed

+57
-23
lines changed

7 files changed

+57
-23
lines changed

demo/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ fn main() -> anyhow::Result<()> {
638638
});
639639
subpass.bind_pipeline(&pipeline);
640640
subpass.bind_vertex_buffers(0, &[(&vertex_buffer, 0)])?;
641-
subpass.bind_index_buffer(&index_buffer, 0, vk::IndexType::UINT16);
641+
subpass.bind_index_buffer(&index_buffer, 0, vk::IndexType::UINT16)?;
642642
subpass.bind_descriptor_sets(
643643
vk::PipelineBindPoint::GRAPHICS,
644644
&pipeline_layout,

src/buffer.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ impl BufferWithoutMemory {
5656
}
5757
impl Buffer {
5858
// TODO: Bulk bind
59-
/// Note that it is an error to bind a storage buffer to host-visible memory
60-
/// when robust buffer access is not enabled.
59+
/// Note that it is an error to bind a storage, uniform, vertex, or index
60+
/// buffer to host-visible memory when robust buffer access is not enabled.
6161
#[doc = crate::man_link!(vkBindBufferMemory)]
6262
pub fn new(
6363
buffer: BufferWithoutMemory, memory: &DeviceMemory, offset: u64,
@@ -128,9 +128,9 @@ impl BufferWithoutMemory {
128128
self.handle.borrow_mut()
129129
}
130130
/// If [`BufferCreateInfo::usage`] includes an abritrarily indexable buffer
131-
/// usage type (uniform, storage, or vertex) and the robust buffer access
132-
/// feature was not enabled at device creation, any host-visible memory
133-
/// types will be removed from the output. Note that on
131+
/// usage type (uniform, storage, vertex, or index) and the robust buffer
132+
/// access feature was not enabled at device creation, any host-visible
133+
/// memory types will be removed from the output. Note that on
134134
/// some physical devices (eg software rasterizers), *all* memory types are
135135
/// host-visible.
136136
///
@@ -154,7 +154,7 @@ impl BufferWithoutMemory {
154154
result
155155
}
156156
/// Allocate a single piece of memory for the buffer and bind it. Note that
157-
/// it is an error to bind a uniform, storage, or vertex buffer to
157+
/// it is an error to bind a uniform, storage, vertex, or index buffer to
158158
/// host-visible memory when robust buffer access is not enabled.
159159
pub fn allocate_memory(
160160
self, memory_type_index: u32,

src/command_buffer/bind.rs

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,12 @@ impl<'a> RenderPassRecording<'a> {
7373
self.rec.bind_vertex_buffers(first_binding, buffers_offsets)
7474
}
7575
/// Reference count of `buffer` is incremented. Returns
76-
/// [`Error::InvalidArgument`] if `buffers_offsets` is empty.
76+
/// [`Error::InvalidArgument`] if `buffer` does not have the `INDEX_BUFFER`
77+
/// usage flag.
7778
#[doc = crate::man_link!(vkCmdBindIndexBuffer)]
7879
pub fn bind_index_buffer(
7980
&mut self, buffer: &Arc<Buffer>, offset: u64, index_type: IndexType,
80-
) {
81+
) -> Result<()> {
8182
self.rec.bind_index_buffer(buffer, offset, index_type)
8283
}
8384
}
@@ -92,11 +93,12 @@ impl<'a> SecondaryCommandRecording<'a> {
9293
self.rec.bind_vertex_buffers(first_binding, buffers_offsets)
9394
}
9495
/// Reference count of `buffer` is incremented. Returns
95-
/// [`Error::InvalidArgument`] if `buffers_offsets` is empty.
96+
/// [`Error::InvalidArgument`] if `buffer` does not have the `INDEX_BUFFER`
97+
/// usage flag.
9698
#[doc = crate::man_link!(vkCmdBindIndexBuffer)]
9799
pub fn bind_index_buffer(
98100
&mut self, buffer: &Arc<Buffer>, offset: u64, index_type: IndexType,
99-
) {
101+
) -> Result<()> {
100102
self.rec.bind_index_buffer(buffer, offset, index_type)
101103
}
102104
}
@@ -113,9 +115,6 @@ impl<'a> CommandRecording<'a> {
113115
return Err(Error::InvalidArgument);
114116
}
115117
}
116-
for &(buffer, _) in buffers_offsets {
117-
self.add_resource(buffer.clone());
118-
}
119118
let buffers = self.scratch.alloc_slice_fill_iter(
120119
buffers_offsets.iter().map(|&(b, _)| b.handle()),
121120
);
@@ -132,14 +131,21 @@ impl<'a> CommandRecording<'a> {
132131
Array::from_slice(offsets).ok_or(Error::InvalidArgument)?,
133132
)
134133
}
134+
for &(buffer, _) in buffers_offsets {
135+
self.add_resource(buffer.clone());
136+
}
135137
Ok(())
136138
}
137139
/// Reference count of `buffer` is incremented. Returns
138-
/// [`Error::InvalidArgument`] if `buffers_offsets` is empty.
140+
/// [`Error::InvalidArgument`] if `buffer` does not have the `INDEX_BUFFER`
141+
/// usage flag.
139142
#[doc = crate::man_link!(vkCmdBindIndexBuffer)]
140143
pub fn bind_index_buffer(
141144
&mut self, buffer: &Arc<Buffer>, offset: u64, index_type: IndexType,
142-
) {
145+
) -> Result<()> {
146+
if !buffer.usage().contains(BufferUsageFlags::INDEX_BUFFER) {
147+
return Err(Error::InvalidArgument);
148+
}
143149
self.add_resource(buffer.clone());
144150
unsafe {
145151
(self.pool.device.fun.cmd_bind_index_buffer)(
@@ -149,6 +155,7 @@ impl<'a> CommandRecording<'a> {
149155
index_type,
150156
)
151157
}
158+
Ok(())
152159
}
153160
}
154161

src/command_buffer/draw.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use crate::ffi::Array;
1212
use crate::render_pass::RenderPass;
1313
use crate::subobject::Owner;
1414
use crate::types::*;
15+
use crate::vk::BufferUsageFlags;
1516

1617
use super::{
1718
Bindings, CommandRecording, ExternalRenderPassRecording,
@@ -216,6 +217,24 @@ impl<'a> SecondaryCommandRecording<'a> {
216217
self.rec.draw_indexed_indirect(buffer, offset, draw_count, stride)
217218
}
218219
}
220+
221+
fn bounds_check_n(
222+
count: u32, size: u32, mut stride: u32, buf: &Arc<Buffer>, offset: u64,
223+
) -> Result<()> {
224+
if count < 2 {
225+
stride = size;
226+
}
227+
if stride < size || offset & 3 != 0 {
228+
return Err(Error::InvalidArgument);
229+
}
230+
let len =
231+
(count as u64).checked_mul(stride as u64).ok_or(Error::OutOfBounds)?;
232+
if !buf.bounds_check(offset, len) {
233+
return Err(Error::OutOfBounds);
234+
}
235+
Ok(())
236+
}
237+
219238
impl<'a> CommandRecording<'a> {
220239
fn draw(
221240
&mut self, vertex_count: u32, instance_count: u32, first_vertex: u32,
@@ -237,6 +256,10 @@ impl<'a> CommandRecording<'a> {
237256
&mut self, buffer: &Arc<Buffer>, offset: u64, draw_count: u32,
238257
stride: u32,
239258
) -> Result<()> {
259+
if !buffer.usage().contains(BufferUsageFlags::INDIRECT_BUFFER) {
260+
return Err(Error::InvalidArgument);
261+
}
262+
bounds_check_n(draw_count, 16, stride, buffer, offset)?;
240263
self.graphics.check()?;
241264
self.add_resource(buffer.clone());
242265
unsafe {
@@ -271,6 +294,10 @@ impl<'a> CommandRecording<'a> {
271294
&mut self, buffer: &Arc<Buffer>, offset: u64, draw_count: u32,
272295
stride: u32,
273296
) -> Result<()> {
297+
bounds_check_n(draw_count, 20, stride, buffer, offset)?;
298+
if !buffer.usage().contains(BufferUsageFlags::INDIRECT_BUFFER) {
299+
return Err(Error::InvalidArgument);
300+
}
274301
self.graphics.check()?;
275302
self.add_resource(buffer.clone());
276303
unsafe {

src/enums.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -283,13 +283,9 @@ bitflags! {
283283
impl BufferUsageFlags {
284284
/// Does the usage support arbitrary indexing?
285285
pub fn indexable(self) -> bool {
286-
self.intersects(
287-
Self::STORAGE_TEXEL_BUFFER
288-
| Self::STORAGE_BUFFER
289-
| Self::UNIFORM_TEXEL_BUFFER
290-
| Self::UNIFORM_BUFFER
291-
| Self::VERTEX_BUFFER,
292-
)
286+
let allowed =
287+
Self::TRANSFER_SRC | Self::TRANSFER_DST | Self::INDIRECT_BUFFER;
288+
!allowed.contains(self)
293289
}
294290
}
295291

src/image.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ impl ImageWithoutMemory {
102102
}
103103
}
104104
impl Image {
105+
/// Note that it is an error to bind a storage image to
106+
/// host-visible memory when robust buffer access is not enabled.
105107
#[doc = crate::man_link!(vkBindImageMemory)]
106108
pub fn new(
107109
image: ImageWithoutMemory, memory: &DeviceMemory, offset: u64,

src/types.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1464,6 +1464,8 @@ pub struct AttachmentReference {
14641464

14651465
#[repr(C)]
14661466
#[derive(Debug)]
1467+
/// Create with
1468+
/// [`vk::SubpassDescription::try_into`](SubpassDescription).
14671469
#[doc = crate::man_link!(VkSubpassDescription)]
14681470
pub struct VkSubpassDescription<'a> {
14691471
flags: SubpassDescriptionFlags,

0 commit comments

Comments
 (0)