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

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

[Aldo Ariel Panzardo]

atom_op_calltable() invokes a child ATOM table, forwarding the
parent's parameter space with an offset:

    amdgpu_atom_execute_table_locked(ctx->ctx, idx,
        ctx->ps + ctx->ps_shift,
        ctx->ps_size - ctx->ps_shift);

ctx->ps_shift is derived from the child table's PS byte count
(ps / 4) in amdgpu_atom_execute_table_locked(), while ctx->ps_size
carries the remaining capacity from the parent. A malformed ATOM
table chain in the VBIOS (or a GPU that reports corrupted table
headers) can produce ps_shift > ps_size, underflowing the
subtraction to a huge positive value passed as params_size to the
recursive call. The child table then reads and writes far beyond
the stack-allocated parameter buffer.

Reject the call when the shift exceeds the available size.

Fixes: d38ceaf99ed0 ("drm/amdgpu: add coordinate ATOMBIOS table support")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
index e0e585f..638bd18 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) {
+			ctx->abort = true;
+			return;
+		}
 		r = amdgpu_atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift, ctx->ps_size - ctx->ps_shift);
+	}
 	if (r) {
 		ctx->abort = true;
 	}
-- 
2.43.0

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

[sashiko-bot]

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

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

[Aldo Ariel Panzardo]

atom_op_calltable() invokes a child ATOM table, forwarding the
parent's parameter space with an offset:

    amdgpu_atom_execute_table_locked(ctx->ctx, idx,
        ctx->ps + ctx->ps_shift,
        ctx->ps_size - ctx->ps_shift);

ctx->ps_shift is in dwords (set to ps / 4 in
amdgpu_atom_execute_table_locked()), while ctx->ps_size is the
remaining capacity in bytes. The subtraction therefore mixes units:
a child table requesting 60 bytes (ps_shift = 15 dwords) with only
16 bytes remaining would compute 16 - 15 = 1 instead of the correct
16 - 60 = underflow.

Convert ps_shift to bytes (ps_shift * 4) in both the guard and the
subtraction so the units are consistent and oversized requests are
correctly rejected.

Fixes: d38ceaf99ed0 ("drm/amdgpu: add coordinate ATOMBIOS table support")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
v2: convert ps_shift to bytes (ps_shift * 4) before comparing with
    ps_size, fixing the unit mismatch found by Sashiko AI review.

 drivers/gpu/drm/amd/amdgpu/atom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
index e0e585f..0940bfb 100644
--- a/drivers/gpu/drm/amd/amdgpu/atom.c
+++ b/drivers/gpu/drm/amd/amdgpu/atom.c
@@ -646,8 +646,8 @@ 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))
-		r = amdgpu_atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift, ctx->ps_size - ctx->ps_shift);
+	if (U16(ctx->ctx->cmd_table + 4 + 2 * idx) && ctx->ps_shift * 4 <= ctx->ps_size)
+		r = amdgpu_atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift, ctx->ps_size - ctx->ps_shift * 4);
 	if (r) {
 		ctx->abort = true;
 	}
-- 
2.43.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/amdgpu: silent bypass of oversized ATOM table execution fails to abort
--

commit 6b59921539d22666969d301a49ef26fc67ab007b
Author: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

drm/amdgpu: prevent parameter-space underflow in nested ATOM table calls

atom_op_calltable() invokes a child ATOM table, forwarding the parent's
parameter space with an offset. The previous logic mixed units (dwords vs
bytes) when subtracting the offset from the remaining capacity. This commit
fixes the unit mismatch by converting ps_shift to bytes.

> diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
> index 1a599d5cf6a5c..7db0118e654c0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atom.c
> @@ -646,8 +646,8 @@ 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))
> -		r = amdgpu_atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift, ctx->ps_size - ctx->ps_shift);
> +	if (U16(ctx->ctx->cmd_table + 4 + 2 * idx) && ctx->ps_shift * 4 <= ctx->ps_size)
> +		r = amdgpu_atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift, ctx->ps_size - ctx->ps_shift * 4);

[Severity: High]
Does this code silently skip the table execution without aborting when the
parameter space request is oversized?

Looking at atom_op_calltable(), the variable r is initialized to 0. When
a child table requests an oversized parameter space where
ctx->ps_shift * 4 > ctx->ps_size, the new condition prevents the call to
amdgpu_atom_execute_table_locked(), leaving r as 0.

>  	if (r) {
>  		ctx->abort = true;
>  	}

This means the check above will evaluate to false, failing to abort the
interpreter. The parent table will continue execution assuming the child
table executed successfully, which could lead to using uninitialized
data or skipping critical hardware setup.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923152912.1296884-1-qwe.aldo@gmail.com?part=1

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

[Aldo Ariel Panzardo]

atom_op_calltable() invokes a child ATOM table, forwarding the
parent's parameter space with an offset:

    amdgpu_atom_execute_table_locked(ctx->ctx, idx,
        ctx->ps + ctx->ps_shift,
        ctx->ps_size - ctx->ps_shift);

ctx->ps_shift is in dwords (set to ps / 4 in
amdgpu_atom_execute_table_locked()), while ctx->ps_size is the
remaining capacity in bytes. The subtraction therefore mixes units:
a child table requesting 60 bytes (ps_shift = 15 dwords) with only
16 bytes remaining would compute 16 - 15 = 1 instead of the correct
16 - 60 = underflow.

Convert ps_shift to bytes (ps_shift * 4) in both the guard and the
subtraction so the units are consistent. Abort the interpreter when
the request exceeds the available space so the parent table does not
continue with stale or uninitialized data.

Fixes: d38ceaf99ed0 ("drm/amdgpu: add coordinate ATOMBIOS table support")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
v3: abort the interpreter (ctx->abort = true + return) when the
    child table's parameter space exceeds the parent's remaining
    capacity, instead of silently skipping execution (found by
    Sashiko AI review on v2).
v2: convert ps_shift to bytes (ps_shift * 4) before comparing with
    ps_size, fixing the unit mismatch (found by Sashiko AI review).

 drivers/gpu/drm/amd/amdgpu/atom.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
index e0e585f..af283cd 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))
-		r = amdgpu_atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift, ctx->ps_size - ctx->ps_shift);
+	if (U16(ctx->ctx->cmd_table + 4 + 2 * idx)) {
+		if (ctx->ps_shift * 4 > ctx->ps_size) {
+			ctx->abort = true;
+			return;
+		}
+		r = amdgpu_atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift, ctx->ps_size - ctx->ps_shift * 4);
+	}
 	if (r) {
 		ctx->abort = true;
 	}
-- 
2.43.0

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

[Alex Deucher]

Applied.  Thanks!

On Wed, Sep 23, 2026 at 11:44 AM Aldo Ariel Panzardo <qwe.aldo@gmail.com> wrote:
>
> atom_op_calltable() invokes a child ATOM table, forwarding the
> parent's parameter space with an offset:
>
>     amdgpu_atom_execute_table_locked(ctx->ctx, idx,
>         ctx->ps + ctx->ps_shift,
>         ctx->ps_size - ctx->ps_shift);
>
> ctx->ps_shift is in dwords (set to ps / 4 in
> amdgpu_atom_execute_table_locked()), while ctx->ps_size is the
> remaining capacity in bytes. The subtraction therefore mixes units:
> a child table requesting 60 bytes (ps_shift = 15 dwords) with only
> 16 bytes remaining would compute 16 - 15 = 1 instead of the correct
> 16 - 60 = underflow.
>
> Convert ps_shift to bytes (ps_shift * 4) in both the guard and the
> subtraction so the units are consistent. Abort the interpreter when
> the request exceeds the available space so the parent table does not
> continue with stale or uninitialized data.
>
> Fixes: d38ceaf99ed0 ("drm/amdgpu: add coordinate ATOMBIOS table support")
> Cc: stable@vger.kernel.org
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
> v3: abort the interpreter (ctx->abort = true + return) when the
>     child table's parameter space exceeds the parent's remaining
>     capacity, instead of silently skipping execution (found by
>     Sashiko AI review on v2).
> v2: convert ps_shift to bytes (ps_shift * 4) before comparing with
>     ps_size, fixing the unit mismatch (found by Sashiko AI review).
>
>  drivers/gpu/drm/amd/amdgpu/atom.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
> index e0e585f..af283cd 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))
> -               r = amdgpu_atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift, ctx->ps_size - ctx->ps_shift);
> +       if (U16(ctx->ctx->cmd_table + 4 + 2 * idx)) {
> +               if (ctx->ps_shift * 4 > ctx->ps_size) {
> +                       ctx->abort = true;
> +                       return;
> +               }
> +               r = amdgpu_atom_execute_table_locked(ctx->ctx, idx, ctx->ps + ctx->ps_shift, ctx->ps_size - ctx->ps_shift * 4);
> +       }
>         if (r) {
>                 ctx->abort = true;
>         }
> --
> 2.43.0