<br><br><div class="gmail_quote">2012/11/9 Rob Clark <span dir="ltr"><<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>></span><br><blockquote style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid" class="gmail_quote">
<div class="im">On Fri, Nov 9, 2012 at 1:39 AM, Inki Dae <<a href="mailto:inki.dae@samsung.com">inki.dae@samsung.com</a>> wrote:<br>
> 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>
</div>btw, this should instead be done by holding a ref to the GEM<br>
object(s)..  or these days you can increment the reference count on<br>
the fb and let the fb hold ref's to the GEM object(s) (which makes it<br>
a bit easier to deal with multi-planar formats)<br>
<br></blockquote><div> </div><div>Rob, let's discuss that again after you read my latest comment. Please, see my latest comment.</div><div> </div><div>Thanks,</div><div>Inki Dae </div><div> </div><blockquote style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid" class="gmail_quote">

BR,<br>
-R<br>
<div class="HOEnZb"><div class="h5"><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>
><br>
> 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">inki.dae@samsung.com</a>><br>
> Signed-off-by: Kyungmin Park <<a href="mailto:kyungmin.park@samsung.com">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>
> --<br>
> 1.7.4.1<br>
><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>
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>
</div></div></blockquote></div><br>