Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] accel/qda: race between qda_ioctl_attach() and qda_ioctl_init_create() --
commit c03ef3a6851102dcdbec94040ce6c4949c48a5b9 Author: Ekansh Gupta <[email protected]> accel/qda: Add remote attach to root and sensors protection domains This commit adds an ioctl to attach a calling session to an existing root or sensors protection domain on the DSP instead of creating a new user process. > diff --git a/drivers/accel/qda/qda_ioctl.c b/drivers/accel/qda/qda_ioctl.c > index 999afa9e70c23..9b8cccfea1e3a 100644 > --- a/drivers/accel/qda/qda_ioctl.c > +++ b/drivers/accel/qda/qda_ioctl.c > @@ -121,6 +121,8 @@ static int qda_fastrpc_invoke(u32 type, void *data, > struct qda_session *session) > if (type == QDA_FASTRPC_RMID_INIT_CREATE) { > qda_session_set_init_mem(session, ctx->init_mem_gem_obj); > WRITE_ONCE(session->pd, QDA_USER_PD); > + } else if (type == QDA_FASTRPC_RMID_INIT_ATTACH) { > + WRITE_ONCE(session->pd, ctx->pd); > } > > out_put: > @@ -174,6 +176,40 @@ int qda_release_dsp_process(struct qda_dev *qdev, struct > qda_session *session) > return qda_fastrpc_invoke(QDA_FASTRPC_RMID_INIT_RELEASE, NULL, session); > } > > +/** > + * qda_ioctl_attach() - Attach to a DSP-resident protection domain > + * @dev: DRM device structure > + * @data: User-space data (struct drm_qda_remote_attach) > + * @file_priv: DRM file private data > + * > + * Return: 0 on success, negative error code on failure > + */ > +int qda_ioctl_attach(struct drm_device *dev, void *data, struct drm_file > *file_priv) > +{ > + struct drm_qda_remote_attach *args = data; > + struct qda_session *session = file_priv->driver_priv; > + int pd; > + > + if (args->pad) > + return -EINVAL; > + > + switch (args->pd_type) { > + case QDA_ATTACH_ROOT_PD: > + pd = QDA_ROOT_PD; > + break; > + case QDA_ATTACH_SENSORS_PD: > + pd = QDA_SENSORS_PD; > + break; > + default: > + return -EINVAL; > + } > + > + if (READ_ONCE(session->init_mem_gem_obj)) > + return -EBUSY; > + > + return qda_fastrpc_invoke(QDA_FASTRPC_RMID_INIT_ATTACH, &pd, session); > +} [Severity: High] Can a concurrent call to qda_ioctl_init_create() race with qda_ioctl_attach() on the same file descriptor and corrupt session->pd? As noted in the struct qda_session definition for init_mem_gem_obj, ioctls on a single file can run concurrently. In qda_ioctl_attach(), session->init_mem_gem_obj is checked without holding a lock before calling qda_fastrpc_invoke(), which sleeps in qda_fastrpc_invoke_common() while waiting for the DSP response. Meanwhile, qda_ioctl_init_create() -> qda_session_init_create() -> qda_fastrpc_invoke() only populates session->init_mem_gem_obj via qda_session_set_init_mem() after its own DSP invocation completes: if (type == QDA_FASTRPC_RMID_INIT_CREATE) { qda_session_set_init_mem(session, ctx->init_mem_gem_obj); WRITE_ONCE(session->pd, QDA_USER_PD); } else if (type == QDA_FASTRPC_RMID_INIT_ATTACH) { WRITE_ONCE(session->pd, ctx->pd); } If thread 1 runs qda_ioctl_init_create() and thread 2 runs qda_ioctl_attach() before thread 1's RPC finishes, thread 2 sees session->init_mem_gem_obj as null and proceeds. If thread 1 finishes first and sets session->init_mem_gem_obj and session->pd, and thread 2 finishes second, thread 2 will overwrite session->pd with the attached root or sensors domain (or conversely, qda_ioctl_init_create() can be called on an already-attached session). When the file is later closed, qda_postclose() checks session->init_mem_gem_obj and sends the process release request to session->pd: drivers/accel/qda/qda_drv.c:qda_postclose() { if (READ_ONCE(session->init_mem_gem_obj) && drm_dev_enter(dev, &idx)) { qda_release_dsp_process(session->qda_dev, session); drm_dev_exit(idx); } qda_session_put(session); } Because qda_fastrpc_prepare_args() sets ctx->pd from session->pd, wouldn't qda_release_dsp_process() send the release message to the wrong protection domain, leaving the user process running on the DSP while qda_session_put() frees session->init_mem_gem_obj? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=20
