Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Kernel panic via `__counted_by` array bounds violation when userspace supplies `command_count == 0`. -- --- Patch [1]: [PATCH] accel/amdxdna: put the chained BO when its mapping fails --- commit 78a13180a3a2715c4d150e03603955519d45a53a Author: Taimuraz Kaitmazov <[email protected]> accel/amdxdna: put the chained BO when its mapping fails This commit fixes a reference leak in amdxdna_cmd_set_error(). When the chained BO fails to map, the previously acquired reference on the BO is now properly dropped. > diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c > b/drivers/accel/amdxdna/amdxdna_ctx.c > index 31a414c3f0d96..888e857ec5582 100644 > --- a/drivers/accel/amdxdna/amdxdna_ctx.c > +++ b/drivers/accel/amdxdna/amdxdna_ctx.c > @@ -183,8 +183,10 @@ int amdxdna_cmd_set_error(struct amdxdna_gem_obj *abo, > if (!abo) > return -EINVAL; [Severity: Critical] This is a pre-existing issue, but does this code unconditionally access cc->data[0] before reaching here? Looking slightly above this diff hunk in amdxdna_cmd_set_error(): 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; abo = amdxdna_gem_get_obj(client, cc->data[0], AMDXDNA_BO_SHARE); Because the data flexible array member in struct amdxdna_cmd_chain is annotated with __counted_by(command_count), accessing cc->data[0] when command_count is 0 will result in an array bounds violation. Can a user intentionally trigger a kernel panic and a local denial of service by submitting a crafted ERT_CMD_CHAIN with command_count == 0 that later times out or fails? > cmd = amdxdna_gem_vmap(abo); > - if (!cmd) > + if (!cmd) { > + amdxdna_gem_put_obj(abo); > return -ENOMEM; > + } > } > > memset(cmd->data, 0xff, abo->mem.size - sizeof(*cmd)); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
