alloc_ordered_workqueue may fail and return NULL. The fix returns ENOMEM when it fails to avoid potential NULL pointer dereference.
Signed-off-by: Kangjie Lu kjlu@umn.edu --- drivers/gpu/drm/vkms/vkms_crtc.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 8a9aeb0a9ea8..bb66dbcd5e3f 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -219,6 +219,8 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, spin_lock_init(&vkms_out->state_lock);
vkms_out->crc_workq = alloc_ordered_workqueue("vkms_crc_workq", 0); + if (!vkms_out->crc_workq) + return -ENOMEM;
return ret; }
On Mar 8, 2019, at 10:36 PM, Kangjie Lu kjlu@umn.edu wrote:
alloc_ordered_workqueue may fail and return NULL. The fix returns ENOMEM when it fails to avoid potential NULL pointer dereference.
Signed-off-by: Kangjie Lu kjlu@umn.edu
drivers/gpu/drm/vkms/vkms_crtc.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 8a9aeb0a9ea8..bb66dbcd5e3f 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -219,6 +219,8 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, spin_lock_init(&vkms_out->state_lock);
vkms_out->crc_workq = alloc_ordered_workqueue("vkms_crc_workq", 0);
- if (!vkms_out->crc_workq)
return -ENOMEM;
Is this a reasonable patch?
return ret; } -- 2.17.1
On Fri, Mar 22, 2019 at 09:32:07PM -0500, Kangjie Lu wrote:
On Mar 8, 2019, at 10:36 PM, Kangjie Lu kjlu@umn.edu wrote:
alloc_ordered_workqueue may fail and return NULL. The fix returns ENOMEM when it fails to avoid potential NULL pointer dereference.
Signed-off-by: Kangjie Lu kjlu@umn.edu
drivers/gpu/drm/vkms/vkms_crtc.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 8a9aeb0a9ea8..bb66dbcd5e3f 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -219,6 +219,8 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, spin_lock_init(&vkms_out->state_lock);
vkms_out->crc_workq = alloc_ordered_workqueue("vkms_crc_workq", 0);
- if (!vkms_out->crc_workq)
return -ENOMEM;Is this a reasonable patch?
lgtm, applied and thanks for your patch. -Daniel
return ret; } -- 2.17.1
On 3/9/2019 10:06 AM, Kangjie Lu wrote:
alloc_ordered_workqueue may fail and return NULL. The fix returns ENOMEM when it fails to avoid potential NULL pointer dereference.
Signed-off-by: Kangjie Lu kjlu@umn.edu
drivers/gpu/drm/vkms/vkms_crtc.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 8a9aeb0a9ea8..bb66dbcd5e3f 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -219,6 +219,8 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, spin_lock_init(&vkms_out->state_lock);
vkms_out->crc_workq = alloc_ordered_workqueue("vkms_crc_workq", 0);
if (!vkms_out->crc_workq)
return -ENOMEM;return ret; }
Check the clean up path more carefully, you have undo which you have done successfully in drm_crtc_init_with_planes.
Thanks, Mukesh
alloc_ordered_workqueue may fail and return NULL. The fix cleans up drm plans and returns ENOMEM when it fails to avoid potential NULL pointer dereference.
Signed-off-by: Kangjie Lu kjlu@umn.edu --- V2: clean up resources --- drivers/gpu/drm/vkms/vkms_crtc.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 8a9aeb0a9ea8..018b52dd953a 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -219,6 +219,17 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, spin_lock_init(&vkms_out->state_lock);
vkms_out->crc_workq = alloc_ordered_workqueue("vkms_crc_workq", 0); + if (!vkms_out->crc_workq) { + ret = -ENOMEM; + goto cleanup; + } + + return ret;
+cleanup: + if (!IS_ERR_OR_NULL(cursor)) + drm_plane_cleanup(cursor); + if (!IS_ERR(primary)) + drm_plane_cleanup(primary); return ret; }
On Sat, Mar 23, 2019 at 04:42:16PM -0500, Kangjie Lu wrote:
alloc_ordered_workqueue may fail and return NULL. The fix cleans up drm plans and returns ENOMEM when it fails to avoid potential NULL pointer dereference.
Signed-off-by: Kangjie Lu kjlu@umn.edu
V2: clean up resources
drivers/gpu/drm/vkms/vkms_crtc.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c index 8a9aeb0a9ea8..018b52dd953a 100644 --- a/drivers/gpu/drm/vkms/vkms_crtc.c +++ b/drivers/gpu/drm/vkms/vkms_crtc.c @@ -219,6 +219,17 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, spin_lock_init(&vkms_out->state_lock);
vkms_out->crc_workq = alloc_ordered_workqueue("vkms_crc_workq", 0);
- if (!vkms_out->crc_workq) {
ret = -ENOMEM;goto cleanup;- }
- return ret;
+cleanup:
Style nit for the future, for clarity I'd label this err:, since this path is only taken for failures, and not to do cleanup for all cases.
Aside from that, I think your v1 was correct, vkms_crtc_init only sets up the crtc, it doesn't allocate the cursor/planes. That's done from vkms_output_init, which already has the cleanup code to handle this case. -Daniel
- if (!IS_ERR_OR_NULL(cursor))
drm_plane_cleanup(cursor);- if (!IS_ERR(primary))
drm_plane_cleanup(primary);}
2.17.1
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Kangjie Lu -
Mukesh Ojha