[PATCH v4] drm/imagination: Fix parameter validation in pvr_fw_object_destroy()

[PATCH v4] drm/imagination: Fix parameter validation in pvr_fw_object_destroy()

[Alexandru Dadu]

Fix parameter validation to avoid possible NULL pointer dereference from
pvr_fw_object_destroy() when handling allocation failures.

Sashiko report:
If pvr_gem_object_create() fails in
pvr_fw_object_create_and_map_common(), fw_obj->gem is explicitly set to
NULL before jumping to the error cleanup path.
The cleanup path then calls pvr_fw_object_destroy().

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/dri-devel/20260810132202.3C16D1F000E9@smtp.kernel.org/
Fixes: cc1aeedb98ad ("drm/imagination: Implement firmware infrastructure and META FW support")
Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
---
Changes in v4:
- Move the structs declarations after the early return to fix V3.
- Added kfree() to the early return path.
- Link to v3: https://patch.msgid.link/20260922-fix-null-pointer-dereference-v3-1-f099763ed676@imgtec.com

Changes in v3:
- Skimed the commit message removing redundant information.
- Refactored the parameter validation to return early.
- Link to v2: https://patch.msgid.link/20260903-fix-null-pointer-dereference-v2-1-138ff9b6f805@imgtec.com

Changes in v2:
- Commit message and cover letter updates.
- Link to v1: https://patch.msgid.link/20260812-fix-null-pointer-dereference-v1-1-f69b2ffe9520@imgtec.com

To: Alessio Belle <alessio.belle@imgtec.com>
To: Luigi Santivetti <luigi.santivetti@imgtec.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
To: Donald Robson <donald.robson@imgtec.com>
To: Sarah Walker <sarah.walker@imgtec.com>
Cc: imagination@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/gpu/drm/imagination/pvr_fw.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_fw.c b/drivers/gpu/drm/imagination/pvr_fw.c
index 850a3ec8e775..39d70f51f126 100644
--- a/drivers/gpu/drm/imagination/pvr_fw.c
+++ b/drivers/gpu/drm/imagination/pvr_fw.c
@@ -1425,6 +1425,14 @@ pvr_fw_object_create_and_map_offset(struct pvr_device *pvr_dev,
  */
 void pvr_fw_object_destroy(struct pvr_fw_object *fw_obj)
 {
+	if (!fw_obj)
+		return;
+
+	if (!fw_obj->gem) {
+		kfree(fw_obj);
+		return;
+	}
+
 	struct pvr_gem_object *pvr_obj = fw_obj->gem;
 	struct drm_gem_object *gem_obj = gem_from_pvr_gem(pvr_obj);
 	struct pvr_device *pvr_dev = to_pvr_device(gem_obj->dev);
@@ -1439,8 +1447,7 @@ void pvr_fw_object_destroy(struct pvr_fw_object *fw_obj)
 			return;
 	}
 
-	if (fw_obj->gem)
-		pvr_gem_object_put(fw_obj->gem);
+	pvr_gem_object_put(fw_obj->gem);
 
 	kfree(fw_obj);
 }

---
base-commit: bd4f284df04d76fd65e57141cb1e6e7a49e4c3cb
change-id: 20260812-fix-null-pointer-dereference-2c891142d988

Best regards,
--  
Alexandru Dadu <alexandru.dadu@imgtec.com>

Re: [PATCH v4] drm/imagination: Fix parameter validation in pvr_fw_object_destroy()

[Alessio Belle]

On Tue, 2026-09-22 at 09:11 +0300, Alexandru Dadu wrote:
> Fix parameter validation to avoid possible NULL pointer dereference from
> pvr_fw_object_destroy() when handling allocation failures.
> 
> Sashiko report:
> If pvr_gem_object_create() fails in
> pvr_fw_object_create_and_map_common(), fw_obj->gem is explicitly set to
> NULL before jumping to the error cleanup path.
> The cleanup path then calls pvr_fw_object_destroy().
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://lore.kernel.org/dri-devel/20260810132202.3C16D1F000E9@smtp.kernel.org/
> Fixes: cc1aeedb98ad ("drm/imagination: Implement firmware infrastructure and META FW support")
> Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
> ---
> Changes in v4:
> - Move the structs declarations after the early return to fix V3.
> - Added kfree() to the early return path.
> - Link to v3: https://patch.msgid.link/20260922-fix-null-pointer-dereference-v3-1-f099763ed676@imgtec.com
> 
> Changes in v3:
> - Skimed the commit message removing redundant information.
> - Refactored the parameter validation to return early.
> - Link to v2: https://patch.msgid.link/20260903-fix-null-pointer-dereference-v2-1-138ff9b6f805@imgtec.com
> 
> Changes in v2:
> - Commit message and cover letter updates.
> - Link to v1: https://patch.msgid.link/20260812-fix-null-pointer-dereference-v1-1-f69b2ffe9520@imgtec.com
> 
> To: Alessio Belle <alessio.belle@imgtec.com>
> To: Luigi Santivetti <luigi.santivetti@imgtec.com>
> To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> To: Maxime Ripard <mripard@kernel.org>
> To: Thomas Zimmermann <tzimmermann@suse.de>
> To: David Airlie <airlied@gmail.com>
> To: Simona Vetter <simona@ffwll.ch>
> To: Donald Robson <donald.robson@imgtec.com>
> To: Sarah Walker <sarah.walker@imgtec.com>
> Cc: imagination@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  drivers/gpu/drm/imagination/pvr_fw.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/imagination/pvr_fw.c b/drivers/gpu/drm/imagination/pvr_fw.c
> index 850a3ec8e775..39d70f51f126 100644
> --- a/drivers/gpu/drm/imagination/pvr_fw.c
> +++ b/drivers/gpu/drm/imagination/pvr_fw.c
> @@ -1425,6 +1425,14 @@ pvr_fw_object_create_and_map_offset(struct pvr_device *pvr_dev,
>   */
>  void pvr_fw_object_destroy(struct pvr_fw_object *fw_obj)
>  {
> +	if (!fw_obj)
> +		return;
> +
> +	if (!fw_obj->gem) {
> +		kfree(fw_obj);
> +		return;
> +	}
> +
>  	struct pvr_gem_object *pvr_obj = fw_obj->gem;
>  	struct drm_gem_object *gem_obj = gem_from_pvr_gem(pvr_obj);
>  	struct pvr_device *pvr_dev = to_pvr_device(gem_obj->dev);

I have a preference for variable declarations at the top, then checks, then
assignments, but don't feel strongly about it, so:

Reviewed-by: Alessio Belle <alessio.belle@imgtec.com>

Thanks,
Alessio

> @@ -1439,8 +1447,7 @@ void pvr_fw_object_destroy(struct pvr_fw_object *fw_obj)
>  			return;
>  	}
>  
> -	if (fw_obj->gem)
> -		pvr_gem_object_put(fw_obj->gem);
> +	pvr_gem_object_put(fw_obj->gem);
>  
>  	kfree(fw_obj);
>  }
> 
> ---
> base-commit: bd4f284df04d76fd65e57141cb1e6e7a49e4c3cb
> change-id: 20260812-fix-null-pointer-dereference-2c891142d988
> 
> Best regards,
> --  
> Alexandru Dadu <alexandru.dadu@imgtec.com>
>