AlexIndustrial/mesa
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

pvr: Add stricter type checking in pvr_csb_pack().

Browse Source 
Since the packing functions generated by csbgen use a void pointer
for the buffer in which to pack, it's possible to easily write out
of bounds. This commits attempts to reduce the chances by
having the pack macro check that the pointer passed points to an
element sized equally to the state word being packed. Catching
these errors earlier.

As can be seen in this commit, there already was a case of this:
"pds_ctrl". The word size is meant to be 64 bits but the pointer
was pointing to a 32 bit field.

Although it's fine for the word size to be smaller than the
storage pointed to by the pointer, this is not allowed just to
be extra careful.

Signed-off-by: Karmjit Mahil <Karmjit.Mahil@imgtec.com>
Reviewed-by: Frank Binns <frank.binns@imgtec.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/15687>
This commit is contained in:
Karmjit Mahil
2022-03-14 11:46:54 +00:00
committed by Marge Bot
parent c4650cbdb0
commit e5439bf4aa
4 changed files with 35 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/imagination/vulkan/pvr_csb.h
+9 -7
View File
@@ -38,6 +38,7 @@
#include "pvr_bo.h"
#include "pvr_winsys.h"
#include "util/list.h"
#include "util/macros.h"
#define __pvr_address_type pvr_dev_addr_t
#define __pvr_get_address(pvr_dev_addr) (pvr_dev_addr).addr
@@ -137,13 +138,14 @@ VkResult pvr_csb_emit_terminate(struct pvr_csb *csb);
* This can be used by the caller to modify the command or
* state information before it's packed.
*/
#define pvr_csb_pack(_dst, cmd, name) \
for (struct pvr_cmd_struct(cmd) name = { pvr_cmd_header(cmd) }, \
*_loop_terminate = &name; \
__builtin_expect(_loop_terminate != NULL, 1); \
({ \
pvr_cmd_pack(cmd)((_dst), &name); \
_loop_terminate = NULL; \
#define pvr_csb_pack(_dst, cmd, name) \
for (struct pvr_cmd_struct(cmd) name = { pvr_cmd_header(cmd) }, \
*_loop_terminate = &name; \
__builtin_expect(_loop_terminate != NULL, 1); \
({ \
STATIC_ASSERT(sizeof(*(_dst)) == pvr_cmd_length(cmd) * 4); \
pvr_cmd_pack(cmd)((_dst), &name); \
_loop_terminate = NULL; \
}))
/**
src/imagination/vulkan/pvr_job_context.c
+18 -17
View File
@@ -759,15 +759,16 @@ static void pvr_render_ctx_ws_static_state_init(
struct pvr_render_ctx *ctx,
struct pvr_winsys_render_ctx_static_state *static_state)
{
void *dst;
uint64_t *q_dst;
uint32_t *d_dst;
dst = &static_state->vdm_ctx_state_base_addr;
pvr_csb_pack (dst, CR_VDM_CONTEXT_STATE_BASE, base) {
q_dst = &static_state->vdm_ctx_state_base_addr;
pvr_csb_pack (q_dst, CR_VDM_CONTEXT_STATE_BASE, base) {
base.addr = ctx->ctx_switch.vdm_state_bo->vma->dev_addr;
}
dst = &static_state->geom_ctx_state_base_addr;
pvr_csb_pack (dst, CR_TA_CONTEXT_STATE_BASE, base) {
q_dst = &static_state->geom_ctx_state_base_addr;
pvr_csb_pack (q_dst, CR_TA_CONTEXT_STATE_BASE, base) {
base.addr = ctx->ctx_switch.geom_state_bo->vma->dev_addr;
}
@@ -776,8 +777,8 @@ static void pvr_render_ctx_ws_static_state_init(
struct rogue_sr_programs *sr_prog = &ctx->ctx_switch.programs[i].sr;
/* Context store state. */
dst = &static_state->geom_state[i].vdm_ctx_store_task0;
pvr_csb_pack (dst, CR_VDM_CONTEXT_STORE_TASK0, task0) {
q_dst = &static_state->geom_state[i].vdm_ctx_store_task0;
pvr_csb_pack (q_dst, CR_VDM_CONTEXT_STORE_TASK0, task0) {
pvr_rogue_get_vdmctrl_pds_state_words(&sr_prog->pds.store_program,
PVRX(VDMCTRL_USC_TARGET_ANY),
sr_prog->usc.unified_size,
@@ -785,23 +786,23 @@ static void pvr_render_ctx_ws_static_state_init(
&task0.pds_state1);
}
dst = &static_state->geom_state[i].vdm_ctx_store_task1;
pvr_csb_pack (dst, CR_VDM_CONTEXT_STORE_TASK1, task1) {
d_dst = &static_state->geom_state[i].vdm_ctx_store_task1;
pvr_csb_pack (d_dst, CR_VDM_CONTEXT_STORE_TASK1, task1) {
pvr_csb_pack (&task1.pds_state2, VDMCTRL_PDS_STATE2, state) {
state.pds_code_addr.addr = sr_prog->pds.store_program.code_offset;
}
}
dst = &static_state->geom_state[i].vdm_ctx_store_task2;
pvr_csb_pack (dst, CR_VDM_CONTEXT_STORE_TASK2, task2) {
q_dst = &static_state->geom_state[i].vdm_ctx_store_task2;
pvr_csb_pack (q_dst, CR_VDM_CONTEXT_STORE_TASK2, task2) {
pvr_rogue_get_geom_state_stream_out_words(&pt_prog->pds_store_program,
&task2.stream_out1,
&task2.stream_out2);
}
/* Context resume state. */
dst = &static_state->geom_state[i].vdm_ctx_resume_task0;
pvr_csb_pack (dst, CR_VDM_CONTEXT_RESUME_TASK0, task0) {
q_dst = &static_state->geom_state[i].vdm_ctx_resume_task0;
pvr_csb_pack (q_dst, CR_VDM_CONTEXT_RESUME_TASK0, task0) {
pvr_rogue_get_vdmctrl_pds_state_words(&sr_prog->pds.load_program,
PVRX(VDMCTRL_USC_TARGET_ALL),
sr_prog->usc.unified_size,
@@ -809,15 +810,15 @@ static void pvr_render_ctx_ws_static_state_init(
&task0.pds_state1);
}
dst = &static_state->geom_state[i].vdm_ctx_resume_task1;
pvr_csb_pack (dst, CR_VDM_CONTEXT_RESUME_TASK1, task1) {
d_dst = &static_state->geom_state[i].vdm_ctx_resume_task1;
pvr_csb_pack (d_dst, CR_VDM_CONTEXT_RESUME_TASK1, task1) {
pvr_csb_pack (&task1.pds_state2, VDMCTRL_PDS_STATE2, state) {
state.pds_code_addr.addr = sr_prog->pds.load_program.code_offset;
}
}
dst = &static_state->geom_state[i].vdm_ctx_resume_task2;
pvr_csb_pack (dst, CR_VDM_CONTEXT_RESUME_TASK2, task2) {
q_dst = &static_state->geom_state[i].vdm_ctx_resume_task2;
pvr_csb_pack (q_dst, CR_VDM_CONTEXT_RESUME_TASK2, task2) {
pvr_rogue_get_geom_state_stream_out_words(&pt_prog->pds_resume_program,
&task2.stream_out1,
&task2.stream_out2);
src/imagination/vulkan/winsys/pvr_winsys.h
+5 -5
View File
@@ -233,10 +233,10 @@ struct pvr_winsys_compute_ctx_create_info {
uint64_t cdm_ctx_store_pds0;
uint64_t cdm_ctx_store_pds0_b;
uint64_t cdm_ctx_store_pds1;
uint32_t cdm_ctx_store_pds1;
uint64_t cdm_ctx_terminate_pds;
uint64_t cdm_ctx_terminate_pds1;
uint32_t cdm_ctx_terminate_pds1;
uint64_t cdm_ctx_resume_pds0;
uint64_t cdm_ctx_resume_pds0_b;
@@ -262,7 +262,7 @@ struct pvr_winsys_compute_submit_info {
struct {
uint64_t tpu_border_colour_table;
uint64_t cdm_item;
uint64_t compute_cluster;
uint32_t compute_cluster;
uint64_t cdm_ctrl_stream_base;
uint32_t tpu;
uint32_t cdm_resume_pds1;
@@ -309,7 +309,7 @@ struct pvr_winsys_render_submit_info {
struct pvr_winsys_geometry_state {
struct {
uint32_t pds_ctrl;
uint64_t pds_ctrl;
uint32_t ppp_ctrl;
uint32_t te_psg;
uint32_t tpu;
@@ -336,7 +336,7 @@ struct pvr_winsys_render_submit_info {
uint64_t isp_stencil_load_store_base;
uint64_t isp_zload_store_base;
uint64_t isp_zlsctl;
uint64_t isp_zls_pixels;
uint32_t isp_zls_pixels;
uint64_t pbe_word[PVR_MAX_COLOR_ATTACHMENTS]
[ROGUE_NUM_PBESTATE_REG_WORDS];
uint32_t pixel_phantom;
src/imagination/vulkan/winsys/pvrsrvkm/pvr_srv_job_render.c
+3 -1
View File
@@ -395,7 +395,9 @@ static void pvr_srv_geometry_cmd_init(
fw_regs->tpu = state->regs.tpu;
fw_regs->vdm_context_resume_task0_size =
state->regs.vdm_ctx_resume_task0_size;
fw_regs->pds_ctrl = state->regs.pds_ctrl;
assert(state->regs.pds_ctrl >> 32U == 0U);
fw_regs->pds_ctrl = (uint32_t)state->regs.pds_ctrl;
if (state->flags & PVR_WINSYS_GEOM_FLAG_FIRST_GEOMETRY)
cmd->flags |= ROGUE_FWIF_TAFLAGS_FIRSTKICK;