Skip to contentSkip to conversation

drm/vmwgfx_kms: Fix potential NULL pointer dereference

PATCH In review Gustavo A. R. Silva 2 participants 3 messages Dec 4, 2017 – Dec 5, 2017

Subsystems (3)
DRM drivers DRM misc DRM driver for VMware virtual GPU
lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 drm/vmwgfx_kms: Fix potential NULL pointer dereference

crtc_state is being null checked in a previous code block, which implies that such pointer might be null.

crtc_state is dereferenced in drm_atomic_helper_check_plane_state, hence there is a potential null pointer dereference.

Fix this by warning-on and returning -EINVAL in case crtc_state is null.

Addresses-Coverity-ID: 1462412 ("Dereference after null check")

Fixes a01cb8ba3f62 ("drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +3 -0
index a2a93d7..72c3b290 100644
@@ -454,6 +454,9 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
 		clip.y2 = crtc_state->adjusted_mode.vdisplay;
 	}
 
+	if (WARN_ON(!crtc_state))
+		return -EINVAL;
+
 	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
 						  DRM_PLANE_HELPER_NO_SCALING,
 						  DRM_PLANE_HELPER_NO_SCALING,
Signature · 1 line
2.7.4
#1 lore raw
Quoted 5 lines · Gustavo A. R. Silva

crtc_state is being null checked in a previous code block, which implies that such pointer might be null.

crtc_state is dereferenced in drm_atomic_helper_check_plane_state, hence there is a potential null pointer dereference.

This is a false positive. drm_atomic_helper_check_plane_state() will not dereference crtc_state when plane_state->crtc is NULL.

Quoted 15 lines

Fix this by warning-on and returning -EINVAL in case crtc_state is null.

Addresses-Coverity-ID: 1462412 ("Dereference after null check")

Fixes a01cb8ba3f62 ("drm: Move drm_plane_helper_check_state() into drm_atomic_helper.c")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +0 -0
index a2a93d7..72c3b290 100644

drivers/gpu/drm/vmwgfx/vmwgfx_kms.c int vmw_du_primary_plane_atomic_check(struct drm_plane *plane, · line 458

Quoted 5 lines
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +2 -0
@@ -454,6 +454,9 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
 		clip.y2 = crtc_state->adjusted_mode.vdisplay;
 	}
 
+	if (WARN_ON(!crtc_state))
+		return -EINVAL;

This would in fact break the driver because it would flag an error whenever the plane is disabled.

Quoted 11 lines
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c +1 -0
+
 	ret = drm_atomic_helper_check_plane_state(state, crtc_state, &clip,
 						  DRM_PLANE_HELPER_NO_SCALING,
 						  DRM_PLANE_HELPER_NO_SCALING,
Signature · 6 lines
2.7.4

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 6 lines
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#2 lore raw

Hi Ville,

Quoting Ville Syrjälä <[email protected]>:

Quoted 10 lines · Gustavo A. R. Silva
Quoted 5 lines

crtc_state is being null checked in a previous code block, which implies that such pointer might be null.

crtc_state is dereferenced in drm_atomic_helper_check_plane_state, hence there is a potential null pointer dereference.

This is a false positive. drm_atomic_helper_check_plane_state() will not dereference crtc_state when plane_state->crtc is NULL.

You are right.

Thank you for clarifying.
--
Gustavo A. R. Silva