Re: [PATCH v2 09/19] accel: ethosu: Reject unsupported commands

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Time-of-Check to Time-of-Use (TOCTOU) vulnerability due to DMA-BUF export of command stream BO
- [Medium] Sandbox escape vulnerability due to default-allow validation of unknown commands
--

commit d6039ab4982bea39258f6865972e83f60eb4978c
Author: Rob Herring (Arm) <robh@kernel.org>

accel: ethosu: Reject unsupported commands

The command-stream validator does not model U85 branches, indexed DMA,
or OFM transposes. A branch can bypass the linear validation state,
indexed DMA accesses an unchecked index buffer, and a transpose changes
the feature-map address calculation.

> diff --git a/drivers/accel/ethosu/ethosu_gem.c b/drivers/accel/ethosu/ethosu_gem.c
> index ec2832eb9a07a..c913c95e48ae0 100644
> --- a/drivers/accel/ethosu/ethosu_gem.c
> +++ b/drivers/accel/ethosu/ethosu_gem.c
[ ... ]
> @@ -885,18 +896,26 @@ static int ethosu_gem_cmdstream_copy_and_validate(struct drm_device *ddev,
>  			break;
>  
>  		case NPU_SET_DMA0_SRC_REGION:
> +			if (param & NPU_DMA_REGION_INDEX_MODE)
> +				return -EINVAL;
[ ... ]

[Severity: Medium]
This is a pre-existing issue, but I noticed a potential sandbox escape
vulnerability in this switch statement due to default-allow validation
of unknown commands.

In ethosu_gem_cmdstream_copy_and_validate(), if a future NPU hardware
revision introduces new opcodes that perform memory accesses, the current
driver will blindly allow them without calculating their memory bounds
because of the default case:

        case NPU_SET_DMA0_LEN:
            st.dma.src.len = st.dma.dst.len = addr;
            break;
        default:
            break;
        }

Can this allow the NPU to execute unvalidated commands and access memory
outside the validated BO regions if such opcodes are introduced?

[Severity: High]
This is also a pre-existing issue, but could this driver be vulnerable to a
Time-of-Check to Time-of-Use (TOCTOU) attack via DMA-BUF export?

In drivers/accel/ethosu/ethosu_gem.c:ethosu_gem_funcs, the .get_sg_table
callback is exposed without overriding the .export callback:

static const struct drm_gem_object_funcs ethosu_gem_funcs = {
    .free = ethosu_gem_free_object,
    .print_info = drm_gem_dma_object_print_info,
    .get_sg_table = drm_gem_dma_object_get_sg_table,
    ...
};

This permits the command stream BO to be exported as a DMA-BUF and mapped
by other devices.

Meanwhile, in drivers/accel/ethosu/ethosu_job.c:ethosu_ioctl_submit_job(),
the job submission blindly trusts the pre-calculated cmd_info (including
region_size) without preventing concurrent modifications to the buffer
contents:

        /* Verify the command stream doesn't have accesses outside the BO */
        if (cmd_info->region_size[i] > gem->size) {
            dev_err(dev->dev,
                "cmd stream region %d size greater than BO size (%llu > %zu)\n",
                i, cmd_info->region_size[i], gem->size);
            ret = -EOVERFLOW;
            goto out_cleanup_job;
        }

Could a malicious userspace application create a command stream, export it
as a DMA-BUF to another device, and overwrite the command stream with
malicious instructions using the secondary device's DMA capabilities after
validation but before execution?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-ethosu-fixes-v2-0-3767738756a4@kernel.org?part=9