From mboxrd@z Thu Jan  1 00:00:00 1970
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: Re: [PATCH] xen-pciback: fix up cleanup path when alloc
 fails
Date: Mon, 14 Dec 2015 15:21:02 -0500
Message-ID: <20151214202102.GA25842@char.us.oracle.com>
References: <1448569959-7245-1-git-send-email-cardoe@cardoe.com>
	<565EC964.3020006@citrix.com> <565F0681.8070905@cardoe.com>
	<566EE96D.7090106@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta3.messagelabs.com ([195.245.230.39])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <konrad@char.us.oracle.com>) id 1a8ZcS-0006Z2-Se
	for xen-devel@lists.xenproject.org; Mon, 14 Dec 2015 20:21:17 +0000
Content-Disposition: inline
In-Reply-To: <566EE96D.7090106@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: David Vrabel <david.vrabel@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>, Jonathan Creekmore <jonathan.creekmore@gmail.com>, Doug Goldstein <cardoe@cardoe.com>, linux-kernel@vger.kernel.org, Paul Durrant <paul.durrant@citrix.com>, xen-devel@lists.xenproject.org, Boris Ostrovsky <boris.ostrovsky@oracle.com>
List-Id: xen-devel@lists.xenproject.org

On Mon, Dec 14, 2015 at 04:08:13PM +0000, David Vrabel wrote:
> On 02/12/15 14:56, Doug Goldstein wrote:
> > On 12/2/15 4:35 AM, David Vrabel wrote:
> >> On 26/11/15 20:32, Doug Goldstein wrote:
> >>> When allocating a pciback device fails, avoid the possibility of a
> >>> use after free.
> >>
> >> We should not require clearing drvdata for correctness.  We should
> >> ensure we retain drvdata for as long as it is needed.
> >>
> >> I note that pcistub_device_release() has:
> >>
> >> 	kfree(dev_data);
> >> 	pci_set_drvdata(dev, NULL);
> >>
> >> 	/* Clean-up the device */
> >> 	xen_pcibk_config_free_dyn_fields(dev);
> >> 	xen_pcibk_config_free_dev(dev);
> >>
> >> Which should (at a minimum) be reordered to move the kfree(dev_data) to
> >> after the calls that require it
> >>
> >> David
> >>
> > 
> > I apologize but at this point I'm confused at what action I should be
> > taking. Are you saying NACK to the original patch and suggesting this as
> > the replacement? Or saying that this should be done in addition to the
> > original patch?
> 
> I'm suggesting that the goal should be to remove all
> pci_set_drvdata(dev, NULL) calls and have pciback work correctly without
> them.

Which would mean backend/frontend drivers to do this as well.

> 
> Konrad's the pciback maintainer though so I'll defer to him on this.

I would take the patch as is. The cleanup (pci_set_drvdata(dev, NULL)) can
be done another time.

> 
> David
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel