amdxdna_cmd_get_payload() only bounds-checks the payload when a size pointer is passed, so its two callers get different guarantees from one function. amdxdna_cmd_set_error() takes the unchecked form and then reads cc->command_count and cc->data[0], neither of which has been shown to lie inside the BO.
Make the size mandatory and add amdxdna_cmd_get_chain(), which returns the chain only once the declared command count is known to fit. Signed-off-by: Taimuraz Kaitmazov <[email protected]> --- drivers/accel/amdxdna/amdxdna_ctx.c | 51 ++++++++++++++++++++++------- drivers/accel/amdxdna/amdxdna_ctx.h | 2 ++ 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c index 855da8c79a1c..9f44e3918bc1 100644 --- a/drivers/accel/amdxdna/amdxdna_ctx.c +++ b/drivers/accel/amdxdna/amdxdna_ctx.c @@ -125,20 +125,42 @@ void *amdxdna_cmd_get_payload(struct amdxdna_gem_obj *abo, u32 *size) else num_masks = 1 + FIELD_GET(AMDXDNA_CMD_EXTRA_CU_MASK, cmd->header); - if (size) { - count = FIELD_GET(AMDXDNA_CMD_COUNT, cmd->header); - if (unlikely(count <= num_masks || - count * sizeof(u32) + - offsetof(struct amdxdna_cmd, data[0]) > - abo->mem.size)) { - *size = 0; - return NULL; - } - *size = (count - num_masks) * sizeof(u32); + count = FIELD_GET(AMDXDNA_CMD_COUNT, cmd->header); + if (unlikely(count <= num_masks || + count * sizeof(u32) + + offsetof(struct amdxdna_cmd, data[0]) > + abo->mem.size)) { + *size = 0; + return NULL; } + *size = (count - num_masks) * sizeof(u32); + return &cmd->data[num_masks]; } +/* + * Returns the chain payload of @abo, with @count set to a command count that + * has been checked to fit. The chain fields live in a BO user space keeps + * mapped, so nothing may read them without going through here. + */ +struct amdxdna_cmd_chain * +amdxdna_cmd_get_chain(struct amdxdna_gem_obj *abo, u32 *count) +{ + struct amdxdna_cmd_chain *cc; + u32 len, ccnt; + + cc = amdxdna_cmd_get_payload(abo, &len); + if (!cc || len < sizeof(*cc)) + return NULL; + + ccnt = READ_ONCE(cc->command_count); + if (len < struct_size(cc, data, ccnt)) + return NULL; + + *count = ccnt; + return cc; +} + u32 amdxdna_cmd_get_cu_idx(struct amdxdna_gem_obj *abo) { struct amdxdna_cmd *cmd = amdxdna_gem_vmap(abo); @@ -177,8 +199,13 @@ int amdxdna_cmd_set_error(struct amdxdna_gem_obj *abo, cmd->header |= FIELD_PREP(AMDXDNA_CMD_STATE, error_state); if (amdxdna_cmd_get_op(abo) == ERT_CMD_CHAIN) { - cc = amdxdna_cmd_get_payload(abo, NULL); - cc->error_index = (cmd_idx < cc->command_count) ? cmd_idx : 0; + u32 ccnt; + + cc = amdxdna_cmd_get_chain(abo, &ccnt); + if (!cc || !ccnt) + return -EINVAL; + + cc->error_index = (cmd_idx < ccnt) ? cmd_idx : 0; abo = amdxdna_gem_get_obj(client, cc->data[0], AMDXDNA_BO_SHARE); if (!abo) return -EINVAL; diff --git a/drivers/accel/amdxdna/amdxdna_ctx.h b/drivers/accel/amdxdna/amdxdna_ctx.h index b6bef3af7dab..f6529d512217 100644 --- a/drivers/accel/amdxdna/amdxdna_ctx.h +++ b/drivers/accel/amdxdna/amdxdna_ctx.h @@ -196,6 +196,8 @@ amdxdna_cmd_get_state(struct amdxdna_gem_obj *abo) } void *amdxdna_cmd_get_payload(struct amdxdna_gem_obj *abo, u32 *size); +struct amdxdna_cmd_chain * +amdxdna_cmd_get_chain(struct amdxdna_gem_obj *abo, u32 *count); u32 amdxdna_cmd_get_cu_idx(struct amdxdna_gem_obj *abo); int amdxdna_cmd_set_error(struct amdxdna_gem_obj *abo, struct amdxdna_sched_job *job, u32 cmd_idx, -- 2.55.0
