From e2b044fe3fccd4aad93986baff85b3d8a5b38b47 Mon Sep 17 00:00:00 2001 From: Jose Fonseca Date: Fri, 11 Nov 2022 16:06:34 +0000 Subject: [PATCH] lavapipe: Prevent integer overflow adding index buffer offset and start index. Direct3D and Vulkan's robustBufferAccess2 feature mandate that index buffer out-of-bounds reads should return a zero index (ie, vertex at index zero, not to be confused with a vertex with zero attributes, as the kind resulting in vertex buffer out-of-bounds read.) lavapipe was adding index_offset and start index together without overflow checks, and if start index was sufficient large (as is the case with WHCK wgf11draw which sets start index to (UINT)-5) it would cause to wrap around causing fetches that should be out of bounds wrap around and fetch inside bounds. This change fixes this by doing a clamped add. This ensures start index is set to UINT32_MAX on overflow, which is sufficient in practice to trigger draw index OOB code-paths, yield zero index to be returned. Reviewed-by: Brian Paul Reviewed-by: Roland Scheidegger Part-of: --- src/gallium/frontends/lavapipe/lvp_execute.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/gallium/frontends/lavapipe/lvp_execute.c b/src/gallium/frontends/lavapipe/lvp_execute.c index 5c5dda77ed2..57756d8fa98 100644 --- a/src/gallium/frontends/lavapipe/lvp_execute.c +++ b/src/gallium/frontends/lavapipe/lvp_execute.c @@ -39,6 +39,7 @@ #include "util/u_sampler.h" #include "util/u_box.h" #include "util/u_inlines.h" +#include "util/u_math.h" #include "util/u_memory.h" #include "util/u_prim.h" #include "util/u_prim_restart.h" @@ -2535,7 +2536,7 @@ static void handle_draw_indexed(struct vk_cmd_queue_entry *cmd, state->info.index_bounds_valid = false; state->info.min_index = 0; - state->info.max_index = ~0; + state->info.max_index = ~0U; state->info.index_size = state->index_size; state->info.index.resource = state->index_buffer; state->info.start_instance = cmd->u.draw_indexed.first_instance; @@ -2547,7 +2548,8 @@ static void handle_draw_indexed(struct vk_cmd_queue_entry *cmd, draw.count = cmd->u.draw_indexed.index_count; draw.index_bias = cmd->u.draw_indexed.vertex_offset; /* TODO: avoid calculating multiple times if cmdbuf is submitted again */ - draw.start = (state->index_offset / state->index_size) + cmd->u.draw_indexed.first_index; + draw.start = util_clamped_uadd(state->index_offset / state->index_size, + cmd->u.draw_indexed.first_index); state->info.index_bias_varies = !cmd->u.draw_indexed.vertex_offset; state->pctx->set_patch_vertices(state->pctx, state->patch_vertices); @@ -2562,7 +2564,7 @@ static void handle_draw_multi_indexed(struct vk_cmd_queue_entry *cmd, state->info.index_bounds_valid = false; state->info.min_index = 0; - state->info.max_index = ~0; + state->info.max_index = ~0U; state->info.index_size = state->index_size; state->info.index.resource = state->index_buffer; state->info.start_instance = cmd->u.draw_multi_indexed_ext.first_instance; @@ -2583,7 +2585,8 @@ static void handle_draw_multi_indexed(struct vk_cmd_queue_entry *cmd, /* TODO: avoid calculating multiple times if cmdbuf is submitted again */ for (unsigned i = 0; i < cmd->u.draw_multi_indexed_ext.draw_count; i++) - draws[i].start = (state->index_offset / state->index_size) + draws[i].start; + draws[i].start = util_clamped_uadd(state->index_offset / state->index_size, + draws[i].start); state->info.index_bias_varies = !cmd->u.draw_multi_indexed_ext.vertex_offset; state->pctx->set_patch_vertices(state->pctx, state->patch_vertices); @@ -2602,7 +2605,7 @@ static void handle_draw_indirect(struct vk_cmd_queue_entry *cmd, state->info.index_bounds_valid = false; state->info.index_size = state->index_size; state->info.index.resource = state->index_buffer; - state->info.max_index = ~0; + state->info.max_index = ~0U; if (state->info.primitive_restart) state->info.restart_index = util_prim_restart_index_from_size(state->info.index_size); } else @@ -3105,7 +3108,7 @@ static void handle_draw_indirect_count(struct vk_cmd_queue_entry *cmd, state->info.index_bounds_valid = false; state->info.index_size = state->index_size; state->info.index.resource = state->index_buffer; - state->info.max_index = ~0; + state->info.max_index = ~0U; } else state->info.index_size = 0; state->indirect_info.offset = cmd->u.draw_indirect_count.offset;