Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] amdgpu: incomplete bounds check in atom_op_calltable() mixes units --
commit 6d85d4d710e40392c30bb7f8392682eb670b9185 Author: Aldo Ariel Panzardo <[email protected]> drm/amdgpu: prevent parameter-space underflow in nested ATOM table calls This commit attempts to prevent underflow when calculating parameter space for nested ATOM table calls. It adds a check to verify if the required shift exceeds the available size. > diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c > b/drivers/gpu/drm/amd/amdgpu/atom.c > index 1a599d5cf6a5c..bcaeec1b057d3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/atom.c > +++ b/drivers/gpu/drm/amd/amdgpu/atom.c [ ... ] > @@ -646,8 +646,13 @@ static void atom_op_calltable(atom_exec_context *ctx, > int *ptr, int arg) > SDEBUG(" table: %d (%s)\n", idx, atom_table_names[idx]); > else > SDEBUG(" table: %d\n", idx); > - if (U16(ctx->ctx->cmd_table + 4 + 2 * idx)) > + if (U16(ctx->ctx->cmd_table + 4 + 2 * idx)) { > + if (ctx->ps_shift > ctx->ps_size) { [Severity: High] Does this code compare mismatched units? ctx->ps_shift appears to be calculated in dwords (ps / 4) in amdgpu_atom_execute_table_locked(), while ctx->ps_size is in bytes. If a child table requests 60 bytes (15 dwords), but only 16 bytes are available in ps_size, the comparison 15 > 16 evaluates to false. This would bypass the abort check even though the required capacity exceeds the available capacity. > + ctx->abort = true; > + return; > + } > r = amdgpu_atom_execute_table_locked(ctx->ctx, idx, ctx->ps + > ctx->ps_shift, ctx->ps_size - ctx->ps_shift); [Severity: High] Does this subtraction mix bytes and dwords, corrupting the parameter size passed to the child table? Following the example above where ps_shift is 15 dwords and ps_size is 16 bytes, ctx->ps_size - ctx->ps_shift would compute to 1, while advancing the ctx->ps pointer by 15 dwords (60 bytes). This advances the pointer far past the allocated stack buffer, potentially leaving the code vulnerable to out-of-bounds reads and writes. > + } > if (r) { > ctx->abort = true; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
