<br><br><div class="gmail_quote">2012/11/9 Prathyush K <span dir="ltr"><<a href="mailto:prathyush@chromium.org" target="_blank">prathyush@chromium.org</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br><div class="gmail_extra"><div class="gmail_quote"><div class="im">On Fri, Nov 9, 2012 at 1:09 PM, Inki Dae <span dir="ltr"><<a href="mailto:inki.dae@samsung.com" target="_blank">inki.dae@samsung.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This patch fixes access issue to invalid memory region.<br>
<br>
crtc had only one drm_framebuffer object so when framebuffer<br>
cleanup was requested after page flip, it'd try to disable<br>
hardware overlay to current crtc.<br>
But if current crtc points to another drm_framebuffer,<br>
This may induce invalid memory access.<br>
<br>
Assume that some application are doing page flip with two<br>
drm_framebuffer objects. At this time, if second drm_framebuffer<br>
is set to current crtc and the cleanup to first drm_framebuffer<br>
is requested once drm release is requested, then first<br>
drm_framebuffer would be cleaned up without disabling<br>
hardware overlay because current crtc points to second<br>
drm_framebuffer. After that, gem buffer to first drm_framebuffer<br>
would be released and this makes dma access invalid memory region.<br></blockquote><div><br></div><div><br></div></div><div>I am confused regarding this. If crtc points to second frame buffer, then the dma is accessing the memory region</div>
<div>of the second framebuffer. Why can't we free the first framebuffer and its gem buffer?</div><div><br></div><div>With this patch, there is no way to free a framebuffer (which has been set to a crtc), without disabling</div>
<div>the crtc.</div><div><br></div><div>Consider this example:</div><div>I have three framebuffers (0, 1, 2) and I set them to a crtc A one by one. So with your patch,</div><div>fb[0]->crtc = A</div><div>fb[1]->crtc = A</div>
<div>fb[2]->crtc = A</div><div><br></div><div>After this, I am using only framebuffer 0 and 1 i.e. 0 and 1 are being page flipped.</div><div>Now, I want to remove framebuffer 2.</div><div>fb[2]->crtc = A .. so while removing the framebuffer, we will end up disabling the crtc</div>
<div>which is not correct.</div><div><br></div><div>I think, there should be an additional interface to unset a fb to a crtc. That way, we can<br></div><div>reset the crtc inside a framebuffer so that it can be freed if not in use.</div>
<div>i.e. fb[2]->crtc = NULL;</div><div><div class="h5"><div><br></div><div><br></div></div></div></div></div></blockquote><div><br>Right, thank you for comments. There is my missing point. how about adding fb->crtc = NULL like below then? <br>
<br>int drm_mode_rmfb(struct drm_device *dev, void *data, struct drm_file *file_priv) {<br> ....<br> fb->crtc = NULL;<br> fb->funcs->destroy(fb);<br>out:<br> ...<br>}<br><br>With this, when user requested rmfb to remove fb[2], fb[2]'s crtc
becomes NULL so the fb would be freed without
disabling crtc.<br><br>Thanks,<br>Inki Dae<br><br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">
<div></div><div><br></div><div> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
This patch adds drm_framebuffer to drm_crtc structure as member<br>
and makes drm_framebuffer_cleanup function check if fb->crtc is<br>
same as desired fb. And also when setcrtc and pageflip are<br>
requested, it makes each drm_framebuffer point to current crtc.<br>
<br>
Signed-off-by: Inki Dae <<a href="mailto:inki.dae@samsung.com" target="_blank">inki.dae@samsung.com</a>><br>
Signed-off-by: Kyungmin Park <<a href="mailto:kyungmin.park@samsung.com" target="_blank">kyungmin.park@samsung.com</a>><br>
---<br>
drivers/gpu/drm/drm_crtc.c | 7 ++++---<br>
include/drm/drm_crtc.h | 1 +<br>
2 files changed, 5 insertions(+), 3 deletions(-)<br>
<br>
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c<br>
index ef1b221..5c04bd4 100644<br>
--- a/drivers/gpu/drm/drm_crtc.c<br>
+++ b/drivers/gpu/drm/drm_crtc.c<br>
@@ -386,7 +386,7 @@ void drm_framebuffer_remove(struct drm_framebuffer *fb)<br>
<br>
/* remove from any CRTC */<br>
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {<br>
- if (crtc->fb == fb) {<br>
+ if (fb->crtc == crtc) {<br>
/* should turn off the crtc */<br>
memset(&set, 0, sizeof(struct drm_mode_set));<br>
set.crtc = crtc;<br>
@@ -2027,6 +2027,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,<br>
set.mode = mode;<br>
set.connectors = connector_set;<br>
set.num_connectors = crtc_req->count_connectors;<br>
+ fb->crtc = crtc;<br>
set.fb = fb;<br>
ret = crtc->funcs->set_config(&set);<br>
<br>
@@ -3635,8 +3636,8 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,<br>
spin_unlock_irqrestore(&dev->event_lock, flags);<br>
kfree(e);<br>
}<br>
- }<br>
-<br>
+ } else<br>
+ fb->crtc = crtc;<br>
out:<br>
mutex_unlock(&dev->mode_config.mutex);<br>
return ret;<br>
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h<br>
index 3fa18b7..92889be 100644<br>
--- a/include/drm/drm_crtc.h<br>
+++ b/include/drm/drm_crtc.h<br>
@@ -256,6 +256,7 @@ struct drm_framebuffer {<br>
struct kref refcount;<br>
struct list_head head;<br>
struct drm_mode_object base;<br>
+ struct drm_crtc *crtc;<br>
const struct drm_framebuffer_funcs *funcs;<br>
unsigned int pitches[4];<br>
unsigned int offsets[4];<br>
<span><font color="#888888">--<br>
1.7.4.1<br>
<br>
_______________________________________________<br>
dri-devel mailing list<br>
<a href="mailto:dri-devel@lists.freedesktop.org" target="_blank">dri-devel@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
</font></span></blockquote></div></div></div><br></div>
<br>_______________________________________________<br>
dri-devel mailing list<br>
<a href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
<br></blockquote></div><br>