2012/11/9 Rob Clark robdclark@gmail.com
On Fri, Nov 9, 2012 at 1:39 AM, Inki Dae inki.dae@samsung.com wrote:
This patch fixes access issue to invalid memory region.
crtc had only one drm_framebuffer object so when framebuffer cleanup was requested after page flip, it'd try to disable hardware overlay to current crtc. But if current crtc points to another drm_framebuffer, This may induce invalid memory access.
btw, this should instead be done by holding a ref to the GEM object(s).. or these days you can increment the reference count on the fb and let the fb hold ref's to the GEM object(s) (which makes it a bit easier to deal with multi-planar formats)
Rob, let's discuss that again after you read my latest comment. Please, see my latest comment.
Thanks, Inki Dae
BR, -R
Assume that some application are doing page flip with two drm_framebuffer objects. At this time, if second drm_framebuffer is set to current crtc and the cleanup to first drm_framebuffer is requested once drm release is requested, then first drm_framebuffer would be cleaned up without disabling hardware overlay because current crtc points to second drm_framebuffer. After that, gem buffer to first drm_framebuffer would be released and this makes dma access invalid memory region.
This patch adds drm_framebuffer to drm_crtc structure as member and makes drm_framebuffer_cleanup function check if fb->crtc is same as desired fb. And also when setcrtc and pageflip are requested, it makes each drm_framebuffer point to current crtc.
Signed-off-by: Inki Dae inki.dae@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
drivers/gpu/drm/drm_crtc.c | 7 ++++--- include/drm/drm_crtc.h | 1 + 2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index ef1b221..5c04bd4 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -386,7 +386,7 @@ void drm_framebuffer_remove(struct drm_framebuffer
*fb)
/* remove from any CRTC */ list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
if (crtc->fb == fb) {
if (fb->crtc == crtc) { /* should turn off the crtc */ memset(&set, 0, sizeof(struct drm_mode_set)); set.crtc = crtc;@@ -2027,6 +2027,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void
*data,
set.mode = mode; set.connectors = connector_set; set.num_connectors = crtc_req->count_connectors;
fb->crtc = crtc; set.fb = fb; ret = crtc->funcs->set_config(&set);@@ -3635,8 +3636,8 @@ int drm_mode_page_flip_ioctl(struct drm_device
*dev,
spin_unlock_irqrestore(&dev->event_lock, flags); kfree(e); }
}
} elsefb->crtc = crtc;out: mutex_unlock(&dev->mode_config.mutex); return ret; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 3fa18b7..92889be 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -256,6 +256,7 @@ struct drm_framebuffer { struct kref refcount; struct list_head head; struct drm_mode_object base;
struct drm_crtc *crtc; const struct drm_framebuffer_funcs *funcs; unsigned int pitches[4]; unsigned int offsets[4];-- 1.7.4.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel