Re: [PATCH] drm/amdgpu: prevent parameter-space underflow in nested ATOM table calls

[sashiko-bot@kernel.org]

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 <qwe.aldo@gmail.com>
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/20260923145001.1244517-1-qwe.aldo@gmail.com?part=1