From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/exynos: fix memory leak: free EDID block Date: Fri, 14 Dec 2012 12:24:40 +0200 Message-ID: <87d2yd2aon.fsf@intel.com> References: <1353403816-8091-1-git-send-email-eich@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id C4968E6841 for ; Fri, 14 Dec 2012 02:23:39 -0800 (PST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Sean Paul , Egbert Eich Cc: Takashi Iwai , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Tue, 20 Nov 2012, Sean Paul wrote: > On Tue, Nov 20, 2012 at 4:30 AM, Egbert Eich 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 >> --- >> 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. > > 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/ The patch is good. connector->display_info.raw_edid is gone since commit 451023dc32d4542c21b52ad1692e6e01cb75b099 Author: Jani Nikula Date: Wed Aug 15 09:32:39 2012 +0000 drm: remove the raw_edid field from struct drm_display_info BR, Jani. > > For now, please drop this patch. > > Sean > >> } else { >> return -ENODEV; >> } >> -- >> 1.7.7 >> >> _______________________________________________ >> 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