From: Maarten Lankhorst Date: Wed, 24 Jun 2015 06:59:25 +0000 (+0200) Subject: drm/atomic: Cleanup on error properly in the atomic ioctl. X-Git-Url: http://drtracing.org/?a=commitdiff_plain;h=ec9f932ed41622d120de52a5b525e4d77b9ef17e;p=deliverable%2Flinux.git drm/atomic: Cleanup on error properly in the atomic ioctl. It's probably allowed to leave old_fb set to garbage when unlocking, but to prevent undefined behavior unset it just in case. Also crtc_state->event could be NULL on memory allocation failure, in which case event_space is increased for no reason. Note: Contains some general simplification of the cleanup code too. Signed-off-by: Maarten Lankhorst [danvet: Add note about the other changes in here. And fix long line while at it.] Signed-off-by: Daniel Vetter --- diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index bd7f723c708e..acebd1617264 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1463,18 +1463,18 @@ retry: if (get_user(obj_id, objs_ptr + copied_objs)) { ret = -EFAULT; - goto fail; + goto out; } obj = drm_mode_object_find(dev, obj_id, DRM_MODE_OBJECT_ANY); if (!obj || !obj->properties) { ret = -ENOENT; - goto fail; + goto out; } if (get_user(count_props, count_props_ptr + copied_objs)) { ret = -EFAULT; - goto fail; + goto out; } copied_objs++; @@ -1486,25 +1486,25 @@ retry: if (get_user(prop_id, props_ptr + copied_props)) { ret = -EFAULT; - goto fail; + goto out; } prop = drm_property_find(dev, prop_id); if (!prop) { ret = -ENOENT; - goto fail; + goto out; } if (copy_from_user(&prop_value, prop_values_ptr + copied_props, sizeof(prop_value))) { ret = -EFAULT; - goto fail; + goto out; } ret = atomic_set_prop(state, obj, prop, prop_value); if (ret) - goto fail; + goto out; copied_props++; } @@ -1523,7 +1523,7 @@ retry: e = create_vblank_event(dev, file_priv, arg->user_data); if (!e) { ret = -ENOMEM; - goto fail; + goto out; } crtc_state->event = e; @@ -1533,13 +1533,15 @@ retry: if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) { ret = drm_atomic_check_only(state); /* _check_only() does not free state, unlike _commit() */ - drm_atomic_state_free(state); + if (!ret) + drm_atomic_state_free(state); } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { ret = drm_atomic_async_commit(state); } else { ret = drm_atomic_commit(state); } +out: /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping * locks (ie. while it is still safe to deref plane->state). We * need to do this here because the driver entry points cannot @@ -1552,41 +1554,35 @@ retry: drm_framebuffer_reference(new_fb); plane->fb = new_fb; plane->crtc = plane->state->crtc; - } else { - plane->old_fb = NULL; - } - if (plane->old_fb) { - drm_framebuffer_unreference(plane->old_fb); - plane->old_fb = NULL; + + if (plane->old_fb) + drm_framebuffer_unreference(plane->old_fb); } + plane->old_fb = NULL; } - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); - - return ret; + if (ret == -EDEADLK) { + drm_atomic_state_clear(state); + drm_modeset_backoff(&ctx); + goto retry; + } -fail: - if (ret == -EDEADLK) - goto backoff; + if (ret) { + if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) { + for_each_crtc_in_state(state, crtc, crtc_state, i) { + if (!crtc_state->event) + continue; - if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) { - for_each_crtc_in_state(state, crtc, crtc_state, i) { - destroy_vblank_event(dev, file_priv, crtc_state->event); - crtc_state->event = NULL; + destroy_vblank_event(dev, file_priv, + crtc_state->event); + } } - } - drm_atomic_state_free(state); + drm_atomic_state_free(state); + } drm_modeset_drop_locks(&ctx); drm_modeset_acquire_fini(&ctx); return ret; - -backoff: - drm_atomic_state_clear(state); - drm_modeset_backoff(&ctx); - - goto retry; }