On Sat, Dec 10, 2016 at 10:52:55PM +0100, Daniel Vetter wrote:
Looping twice when we can do it once is silly. Also use a consistent style. Note that there's a good race with the connector list walking, since that is no longer protected by mode_config.mutex. But that's for a later patch to fix.
v2: Actually try to not blow up, somehow I lost the hunk that checks we don't copy too much. Noticed by Chris.
v3:
- squash all drm_mode_getresources cleanups into one
- use consistent style for walking objects (Chris)
Cc: Chris Wilson chris@chris-wilson.co.uk Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/drm_mode_config.c | 119 +++++++++++++++----------------------- 1 file changed, 47 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 2735a5847ffa..54e371dac694 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -84,17 +84,11 @@ int drm_mode_getresources(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_mode_card_res *card_res = data;
- struct list_head *lh; struct drm_framebuffer *fb; struct drm_connector *connector; struct drm_crtc *crtc; struct drm_encoder *encoder;
- int ret = 0;
- int connector_count = 0;
- int crtc_count = 0;
- int fb_count = 0;
- int encoder_count = 0;
- int copied = 0;
- int count, ret = 0; uint32_t __user *fb_id; uint32_t __user *crtc_id; uint32_t __user *connector_id;
@@ -105,89 +99,70 @@ int drm_mode_getresources(struct drm_device *dev, void *data,
mutex_lock(&file_priv->fbs_lock);
- /*
* For the non-control nodes we need to limit the list of resources* by IDs in the group list for this node*/- list_for_each(lh, &file_priv->fbs)
fb_count++;- /* handle this in 4 parts */
- /* FBs */
- if (card_res->count_fbs >= fb_count) {
copied = 0;fb_id = (uint32_t __user *)(unsigned long)card_res->fb_id_ptr;list_for_each_entry(fb, &file_priv->fbs, filp_head) {if (put_user(fb->base.id, fb_id + copied)) {mutex_unlock(&file_priv->fbs_lock);return -EFAULT;}copied++;
- count = 0;
- fb_id = (uint32_t __user *)(unsigned long)card_res->fb_id_ptr;
u64_to_user_ptr() please :)
- list_for_each_entry(fb, &file_priv->fbs, filp_head) {
count++;if (count > card_res->count_fbs)continue;if (put_user(fb->base.id, fb_id + count)) {
In this style increment of count has to happen after the copy.
i.e. if (count < card_res->count_fbs && put_user(fb->base.id, fb_id + count) { mutex_unlock() return -EFAULT; } count++;
mutex_unlock(&file_priv->fbs_lock);return -EFAULT;
- card_res->count_fbs = fb_count;
- card_res->count_fbs = count; mutex_unlock(&file_priv->fbs_lock);