| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 3 Aug 2018 17:13:34 +0530
From: Satendra Singh Thakur <satendra.t@...sung.com>
To: Gustavo Padovan <gustavo@...ovan.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Sean Paul <seanpaul@...omium.org>,
David Airlie <airlied@...ux.ie>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Cc: vineet.j@...sung.com, hemanshu.s@...sung.com, sst2005@...il.com,
Satendra Singh Thakur <satendra.t@...sung.com>
Subject: [PATCH v1] drm/kms/crtc: Improving the func drm_mode_setcrtc
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@...sung.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);
--
2.7.4
Powered by blists - more mailing lists