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) <[email protected]> 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/[email protected]?part=9
