From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 09322346AE8 for ; Fri, 3 Jul 2026 12:02:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783080161; cv=none; b=qS0MqQideBcXQqhX1XXRKwDhk6Txhi+dvCdk2Jxp7CEfQulcc4/3jw1SrJYnoE13kMTqt0l/8HKvDjOV2dXHLLIk0oC36aZCpH8n2wq5X3i/e4649zkEfZq8TO6Ym0/5WSQEna++yGc91n37MYBwf/FN9qq3aOCgFugBpTrdokY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783080161; c=relaxed/simple; bh=CjqGgI8pYrz5nnlKeFS6qwIa3wqG//2zkaydG9JBW3U=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=WOQHWuyGINxV1dqiQ+IzpAUZ48hTIgyv/X5BkY2lgaQYojd78H0vGISLOl3+TGs2OQn9xBxN+xdzChPNfg7J3YAkk+dB1/W3Q6A+ybDBqtY0NMr6U0Thv62tf0kSz/e+z+2RHlFcIwACGNCmzP9uW7okeW4NlQ/o+2ULGhsmplE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=d1p5DJtE; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="d1p5DJtE" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 101971F000E9; Fri, 3 Jul 2026 12:02:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1783080159; bh=XJhoqtd+XutQZMwJdnD8usqZSwYWXDGcrDFKt/cuKCE=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=d1p5DJtEVbdyTioR5+QMpYmqo6VLyhMMI1B8Uot6GSf6A5lBx1DawbaxUIHRcHR9B qL5Pbva7ClpXPJUYn7ZPondXmHoWUiLfh+idbw2gmFGfGuNkPkcLPdOTauzjLBGl1m VKLJofi9RmKKbeG84d6CQ+FMxDgzsNH0Ht4e+P1q8nl3KUPT71UEaaoCyADQZnXVAf IbjkAuXhPHrZUmzUoqoEdHkt+RLAWDodwCsZmgEtvi0Q4MqsInKtXTihe8sGYRku/i xVi7OZHRKymXwdLgItswiFy6gNXIgfSUTOVOWmsQ1w3Ny9tHR3jsfUR/NesVUtfHPo kyNljEawMpAdw== From: sashiko-bot@kernel.org Subject: Re: [PATCH v9 08/14] drm/msm: Switch to generic PAS TZ APIs Reply-To: sashiko-reviews@lists.linux.dev To: "Sumit Garg" Cc: dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org In-Reply-To: <20260702115835.167602-9-sumit.garg@kernel.org> References: <20260702115835.167602-1-sumit.garg@kernel.org> <20260702115835.167602-9-sumit.garg@kernel.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Fri, 03 Jul 2026 12:02:38 +0000 Message-Id: <20260703120239.101971F000E9@smtp.kernel.org> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 4 potential issue(= s) to consider: New issues: - [High] Generic PAS TEE backend maps TA-level errors to -EINVAL, breaking = the DRM MSM fallback logic. - [High] TOCTOU race condition in generic PAS API wrappers leading to NULL = pointer dereference. - [High] Unconditional PAS availability check causes infinite probe deferra= l on platforms without PAS support. Pre-existing issues: - [High] The MSM DRM driver propagates -EOPNOTSUPP instead of -ENODEV, fata= lly failing GPU initialization. -- commit fd17ff54a9520d92288105f263b3bc29eb9cbd4b Author: Sumit Garg drm/msm: Switch to generic PAS TZ APIs =20 Switch drm/msm client drivers over to generic PAS TZ APIs. Generic PAS TZ service allows to support multiple TZ implementation backends like QTEE based SCM PAS service, OP-TEE based PAS service and any further future TZ backend service. > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/ms= m/adreno/adreno_gpu.c > index c62c45bb0ddbb..489462065ea95 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c [ ... ] > @@ -146,10 +147,10 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu,= const char *fwname, > goto out; > =20 > /* Send the image to the secure world */ > - ret =3D qcom_scm_pas_auth_and_reset(pasid); > + ret =3D qcom_pas_auth_and_reset(pasid); [Severity: High] Does this new PAS API wrapper have a TOCTOU race condition? If the PAS backend unregisters concurrently, could ops_ptr become NULL between the check and the dereference in qcom_pas_auth_and_reset()? drivers/firmware/qcom/qcom_pas.c:qcom_pas_auth_and_reset() { if (!ops_ptr) return -ENODEV; return ops_ptr->auth_and_reset(ops_ptr->dev, pas_id); } > =20 > /* > - * If the scm call returns -EOPNOTSUPP we assume that this target > + * If the pas call returns -EOPNOTSUPP we assume that this target > * doesn't need/support the zap shader so quietly fail > */ > if (ret =3D=3D -EOPNOTSUPP) [Severity: High] Is the DRM MSM fallback logic preserved with this generic backend? The driver expects -EOPNOTSUPP to gracefully handle unsupported zap shaders. If the generic PAS TEE backend's __qcom_pas_tee_auth_and_reset() unconditionally maps all TA errors to -EINVAL, will this cause GPU probe to fail fatally on TEE platforms where the zap shader is not supported? drivers/firmware/qcom/qcom_pas_tee.c:__qcom_pas_tee_auth_and_reset() { ret =3D tee_client_invoke_func(data->ctx, &inv_arg, param); if (ret < 0 || inv_arg.ret !=3D 0) { ... return ret ?: -EINVAL; } } [Severity: High] This isn't a bug introduced by this patch, but does this function propagate -EOPNOTSUPP instead of -ENODEV? If qcom_pas_auth_and_reset() returns -EOPNOTSUPP, the code sets zap_available =3D false but leaves ret =3D -EOPNOTSUPP. Since callers explicitly check for -ENODEV to fall back to SECVID_TRUST_CNTL, could this unmapped -EOPNOTSUPP abort hardware initialization? > zap_available =3D false; > else if (ret) > DRM_DEV_ERROR(dev, "Unable to authorize the image\n"); [ ... ] > @@ -175,9 +176,9 @@ int adreno_zap_shader_load(struct msm_gpu *gpu, u32 p= asid) > if (!zap_available) > return -ENODEV; > =20 > - /* We need SCM to be able to load the firmware */ > - if (!qcom_scm_is_available()) { > - DRM_DEV_ERROR(&pdev->dev, "SCM is not available\n"); > + /* We need PAS to be able to load the firmware */ > + if (!qcom_pas_is_available()) { > + DRM_DEV_ERROR(&pdev->dev, "PAS is not available\n"); > return -EPROBE_DEFER; > } [Severity: High] Could this unconditional PAS availability check cause infinite probe deferral on platforms without PAS support? By checking qcom_pas_is_available() before checking if the zap-shader DT node exists (which happens inside zap_shader_load_mdt()), do platforms that lack PAS capabilities fail to reach the DT node check and defer indefinitely? Previously, qcom_scm_is_available() returned true on these platforms, allowing the driver to gracefully return -ENODEV when the DT node was missing. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260702115835.1676= 02-1-sumit.garg@kernel.org?part=3D8