Sean Paul writes:
> On Tue, Nov 20, 2012 at 4:30 AM, Egbert Eich <eich@xxxxxxx> wrote:
> > drm_get_edid() returns a pointer to an EDID block. The caller
> > is responsible to free this pointer itself.
> > Here the pointer gets assigned to the local variable raw_edid.
> > Therefore it should be freed before the variable goes out of
> > scope.
> >
> > Signed-off-by: Egbert Eich <eich@xxxxxxx>
> > ---
> > drivers/gpu/drm/exynos/exynos_hdmi.c | 1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> > index 2c115f8..bc87bca 100644
> > --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> > +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> > @@ -1293,6 +1293,7 @@ static int hdmi_get_edid(void *ctx, struct drm_connector *connector,
> > DRM_DEBUG_KMS("%s : width[%d] x height[%d]\n",
> > (hdata->dvi_mode ? "dvi monitor" : "hdmi monitor"),
> > raw_edid->width_cm, raw_edid->height_cm);
> > + kfree(raw_edid);
>
> This will actually cause the memory to be freed twice.
>
> The reason this happens is drm_get_edid attaches this to
> connector->display_info.raw_edid, which is then freed in the
> exynos_drm_connector function that gets the edid.
No, the raw_edid member is gone from struct drm_display_info.
All other drivers free the edid data returned by drm_get_edid()
themselves.
I would actually prefer if the edid data would be freed by
drm_connector_destroy rather than by the drivers as this would
allow us to cache an edid without creating copies to pass to
the drivers. However some drivers store the pointer to returned
edid block somewhere in their own data structures, so if we
do this it can easily get out of sync when we reread the
edid and it has changed for whatever reason in which case
the driver will have a stale and invalid pointer.
>
> The whole thing is ugly, and needs to be revised. I've uploaded a
> patch to refactor this against the chromium tree, but haven't yet
> rebased against upstream. See
> https://gerrit.chromium.org/gerrit/#/c/38406/
>
> For now, please drop this patch.
>
I've looked at the code in the exynos driver in the drm_next
branch - there's nothing that destroys this edid data any more.
Cheers,
Egbert.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
