Following changes are done to this func: 1. The declaration of plane and it's assignment plane = crtc->primary are only used when mode_valid is set. Therefore, moved it inside the if(mode_valid) statement.
2. The declaration of connector and set_connectors_ptr and out_id are moved inside the for loop, as their scope is limited within that block.
3. Currently, there are 3 checks on count_connectors and 4 checks on mode related params (mode_valid, mode, fb). if (crtc_req->mode_valid) { if (crtc_req->count_connectors == 0 && mode) { if (crtc_req->count_connectors > 0 && (!mode || !fb)) { if (crtc_req->count_connectors > 0) {
In the modified code, there are just 1 check on mode_valid and 2 checks count_connectors. Checks on mode and fb are not needed as these variables will be non-NULL by the end of if(mode_valid) statement if mode_valid is set. If mode_valid is clear, mode and fb will be NULL. Therefore, we just check mode_valid and NOT mode or fb.
4. Moved kfree inside if statement
Signed-off-by: Satendra Singh Thakur satendra.t@samsung.com
---
v1: Hi Mr Maarten, Thanks for the comments. I have fixed some of them and done more modifications to the patch. Please review.
drivers/gpu/drm/drm_crtc.c | 57 ++++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 98a36e6..9842985 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -559,12 +559,10 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, struct drm_mode_config *config = &dev->mode_config; struct drm_mode_crtc *crtc_req = data; struct drm_crtc *crtc; - struct drm_plane *plane; - struct drm_connector **connector_set = NULL, *connector; + struct drm_connector **connector_set = NULL; struct drm_framebuffer *fb = NULL; struct drm_display_mode *mode = NULL; struct drm_mode_set set; - uint32_t __user *set_connectors_ptr; struct drm_modeset_acquire_ctx ctx; int ret; int i; @@ -586,7 +584,6 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, } DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name);
- plane = crtc->primary;
mutex_lock(&crtc->dev->mode_config.mutex); drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE); @@ -596,6 +593,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, goto out;
if (crtc_req->mode_valid) { + struct drm_plane *plane = crtc->primary; + /* Handle framebuffer and mode here*/ /* If we have a mode we need a framebuffer. */ /* If we pass -1, set the mode with the currently bound fb */ if (crtc_req->fb_id == -1) { @@ -636,8 +635,6 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, ret = -EINVAL; goto out; } - - ret = drm_mode_convert_umode(dev, mode, &crtc_req->mode); if (ret) { DRM_DEBUG_KMS("Invalid mode\n"); @@ -669,31 +666,22 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, mode, fb); if (ret) goto out; - - } - - if (crtc_req->count_connectors == 0 && mode) { - DRM_DEBUG_KMS("Count connectors is 0 but mode set\n"); - ret = -EINVAL; - goto out; - } - - if (crtc_req->count_connectors > 0 && (!mode || !fb)) { - DRM_DEBUG_KMS("Count connectors is %d but no mode or fb set\n", - crtc_req->count_connectors); - ret = -EINVAL; - goto out; - } - - if (crtc_req->count_connectors > 0) { - u32 out_id; - + /* Handle connector here + * crtc_req->mode_valid is set at this point + * and we have mode and fb non-NULL. + * We have already checked mode_valid + * hence, we don't check mode and fb here. + */ + if (!crtc_req->count_connectors) { + DRM_DEBUG_KMS("Mode_valid flag is set but connectors' count is 0\n"); + ret = -EINVAL; + goto out; + } /* Avoid unbounded kernel memory allocation */ if (crtc_req->count_connectors > config->num_connector) { ret = -EINVAL; goto out; } - connector_set = kmalloc_array(crtc_req->count_connectors, sizeof(struct drm_connector *), GFP_KERNEL); @@ -703,6 +691,9 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, }
for (i = 0; i < crtc_req->count_connectors; i++) { + struct drm_connector *connector; + uint32_t __user *set_connectors_ptr; + u32 out_id; connector_set[i] = NULL; set_connectors_ptr = (uint32_t __user *)(unsigned long)crtc_req->set_connectors_ptr; if (get_user(out_id, &set_connectors_ptr[i])) { @@ -723,6 +714,18 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
connector_set[i] = connector; } + + } else { + /* crtc_req->mode_valid is clear at this point + * if mode_valid is clear, mode and fb will be NULL + * hence, we don't check mode and fb here. + */ + if (crtc_req->count_connectors) { + DRM_DEBUG_KMS("Connectors's count is %u but mode_valid flag is clear\n", + crtc_req->count_connectors); + ret = -EINVAL; + goto out; + } }
set.crtc = crtc; @@ -743,8 +746,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data, if (connector_set[i]) drm_connector_put(connector_set[i]); } + kfree(connector_set); } - kfree(connector_set); drm_mode_destroy(dev, mode); if (ret == -EDEADLK) { ret = drm_modeset_backoff(&ctx);