Re: [PATCH] xen-pciback: fix up cleanup path when alloc fails

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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