Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] The device node 'np' acquired via `of_get_available_child_by_name()` is never released with `of_node_put()`, causing a reference count leak. This patch makes the leak repeatedly reachable from userspace. - [High] Unchecked return value from `kasprintf()` leads to a NULL pointer dereference and kernel panic. -- --- Patch [1]: [PATCH] drm/msm/adreno: Only check for PAS when a zap shader is present --- commit 84273d207b58c12797426ab6b4931f54563e7fed Author: Paul Hollinsky <[email protected]> drm/msm/adreno: Only check for PAS when a zap shader is present This commit moves the qcom_pas_is_available() check from adreno_zap_shader_load() into zap_shader_load_mdt() after checking for the presence of a zap-shader node. This prevents a permanent -EPROBE_DEFER on boards that lack PAS and don't need a zap shader. Link: https://lore.kernel.org/r/[email protected] > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > index 489462065ea95..995161215ad56 100644 > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > @@ -52,6 +52,12 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const > char *fwname, > return -ENODEV; > } > > + /* We need PAS to be able to load the firmware */ > + if (!qcom_pas_is_available()) { > + DRM_DEV_ERROR(dev, "PAS is not available\n"); > + return -EPROBE_DEFER; > + } > + [Severity: High] This is a pre-existing issue, but does this new error return path leak the device node reference for np? of_get_available_child_by_name() increments the reference count of the returned device node, which needs to be released with of_node_put(). It appears zap_shader_load_mdt() completely lacks any of_node_put() calls on its exit paths. Because this patch moves the qcom_pas_is_available() check after the node lookup, a failure here returns -EPROBE_DEFER and leaves the node refcounted. As noted in the commit message, since GPU initialization is re-attempted on every DRM open() syscall, could this allow an unprivileged user to continuously leak the device node reference count from userspace? [ ... ] While reviewing this patch, I also noticed another pre-existing issue in zap_shader_load_mdt() further down in the file: [Severity: High] This is a pre-existing issue, but does this code lead to a NULL pointer dereference if the memory allocation fails? drivers/gpu/drm/msm/adreno/adreno_gpu.c:zap_shader_load_mdt() { ... char *newname; newname = kasprintf(GFP_KERNEL, "qcom/%s", fwname); ret = qcom_mdt_load(dev, fw, newname, pasid, mem_region, mem_phys, mem_size, NULL); kfree(newname); ... } kasprintf() is called with GFP_KERNEL and can return NULL if the memory allocation fails. The resulting newname pointer is passed immediately as the fw_name argument to qcom_mdt_load() without a NULL check. Since the pointer is passed down the call chain: qcom_mdt_load() -> qcom_mdt_load_no_init() -> qcom_mdt_read_metadata() -> mdt_load_split_segment() -> strlen(fw_name) Will passing NULL to strlen() cause a kernel panic here? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
