From patchwork Fri Oct 2 15:56:41 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sudip Mukherjee X-Patchwork-Id: 7317401 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 63E459F1B9 for ; Fri, 2 Oct 2015 15:56:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 912E72089E for ; Fri, 2 Oct 2015 15:56:57 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id A72DA2089B for ; Fri, 2 Oct 2015 15:56:56 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8144F6E2F6; Fri, 2 Oct 2015 08:56:55 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-pa0-f52.google.com (mail-pa0-f52.google.com [209.85.220.52]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8E5456E2F6 for ; Fri, 2 Oct 2015 08:56:53 -0700 (PDT) Received: by pacex6 with SMTP id ex6so110083261pac.0 for ; Fri, 02 Oct 2015 08:56:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=uKs2sUFYJ+3Qm558PdPqy5BEYcv5aI9Dh+IIjE5yjiE=; b=xxe0yCWNg2w//uJ+7aOT6uscmuyTbfENudfR2EV5bKkRXBxuy9//HrLaE/yZgF7uUH nUIZwbnrLtYdf+j+kZ8AuQJCJzxyJvQ5ly5aC1cTD8VZ5trDR7TodKpUzh7+fJ/GS4al mOzsABPEyL5JQK1RX/DG0ElA+5GINdoClUoqheVLo7Y7L1DwbFBBWCQWDGTRGoURUF+F BGAfDwqkuow6MDtRVkMo9NDod3A0mNhHL4WQhsmHAXFZeFy0GXjfmDSiDJ8w30rwwYyO RAXL5C0nIhx/Hu7d+b7f6/Gmitfq3cb1PmQpCs8SLBfCURACES+MWpnojlFUPjS3OI/0 EmWg== X-Received: by 10.66.161.7 with SMTP id xo7mr20555362pab.57.1443801413148; Fri, 02 Oct 2015 08:56:53 -0700 (PDT) Received: from sudip-pc ([49.206.244.224]) by smtp.gmail.com with ESMTPSA id gx11sm12636683pbd.82.2015.10.02.08.56.50 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Fri, 02 Oct 2015 08:56:52 -0700 (PDT) Date: Fri, 2 Oct 2015 21:26:41 +0530 From: Sudip Mukherjee To: Patrik Jakobsson Subject: Re: [PATCH] drm/gma500: fix double freeing Message-ID: <20151002155641.GA16809@sudip-pc> References: <1441803040-15998-1-git-send-email-sudipm.mukherjee@gmail.com> <20150924155725.GE10109@sudip-pc> <20150930061241.GC3500@sudip-pc> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Daniel Vetter , linux-kernel , dri-devel X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Oct 01, 2015 at 07:07:33PM +0200, Patrik Jakobsson wrote: > On Wed, Sep 30, 2015 at 8:12 AM, Sudip Mukherjee > wrote: > > On Tue, Sep 29, 2015 at 03:20:35PM +0200, Patrik Jakobsson wrote: > >> On Thu, Sep 24, 2015 at 5:57 PM, Sudip Mukherjee > >> wrote: > >> > On Wed, Sep 09, 2015 at 06:20:40PM +0530, Sudip Mukherjee wrote: > >> >> If backing->stolen is true then we were freeing backing by calling > >> >> psb_gtt_free_range() but we called it again after unlocking the mutex. > >> >> Lets make it NULL after freeing in psb_gtt_free_range() and check for > >> >> NULL before calling the function for the second time. > >> >> > >> >> Signed-off-by: Sudip Mukherjee > >> >> --- > >> > Hi Patrik, > >> > A gentle ping. > >> > > >> > regards > >> > sudip > >> > >> Hi, sorry for the late reply. > >> > >> Why are we freeing the range twice in the first case? > > I think, > > if backing->stolen is true then backing is released using > > psb_gtt_free_range() but if backing->stolen is false then the gem object > > is freed but the backing is not yet freed. To free that backing > > psb_gtt_free_range() has been called second time. My patch tried to fix > > the possibility of backing->stolen being true and backing being freed 2 > > times. > > > > regards > > sudip > > There are some special handling of the stolen framebuffer that I don't > remember entirely but the basic concept is that we free the backing > when we drop the last reference on a gem object. That will trigger a > psb_gtt_free_range(). So in this case it looks to me that the extra > free is not needed at all. That's my quick reasoning, feel free to > prove me wrong :) In this case we are allocating backing using psbfb_alloc() and so backing->stolen is always true. So we can remove the backing->stolen condition. And if drm_fb_helper_alloc_fbi() fails then we are jumping to out_err1. So the fitst free will not be needed. If it is ok, I can submit the v2. regards sudip diff --git a/drivers/gpu/drm/gma500/framebuffer.c b/drivers/gpu/drm/gma500/framebuffer.c index 2eaf1b3..932f07b 100644 --- a/drivers/gpu/drm/gma500/framebuffer.c +++ b/drivers/gpu/drm/gma500/framebuffer.c @@ -466,11 +466,6 @@ static int psbfb_create(struct psb_fbdev *fbdev, mutex_unlock(&dev->struct_mutex); return 0; out_unref: - if (backing->stolen) - psb_gtt_free_range(dev, backing); - else - drm_gem_object_unreference(&backing->gem); - drm_fb_helper_release_fbi(&fbdev->psb_fb_helper); out_err1: mutex_unlock(&dev->struct_mutex);