[Intel-gfx] [RESEND 5/6] drm/i915: Remove pci private pointer after destroying the device private

[Michal Wajdeczko]

On Mon, 16 Jul 2018 10:03:31 +0200, Chris Wilson  
<chris at chris-wilson.co.uk> wrote:

> On an aborted module load, we unwind and free our device private - but
> we left a dangling pointer to our privates inside the pci_device. After
> the attempted aborted unload, we may still get a call to  
> i915_pci_remove()
> when the module is removed, potentially chasing stale data.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |  1 +
>  drivers/gpu/drm/i915/i915_pci.c | 13 ++++++++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c  
> b/drivers/gpu/drm/i915/i915_drv.c
> index 700ffb611187..3834bd758a2e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1424,6 +1424,7 @@ int i915_driver_load(struct pci_dev *pdev, const  
> struct pci_device_id *ent)
>  	drm_dev_fini(&dev_priv->drm);
>  out_free:
>  	kfree(dev_priv);
> +	pci_set_drvdata(pdev, NULL);
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_pci.c  
> b/drivers/gpu/drm/i915/i915_pci.c
> index 55543f1b0236..6a4d1388ad2d 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -674,10 +674,16 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
> static void i915_pci_remove(struct pci_dev *pdev)
>  {
> -	struct drm_device *dev = pci_get_drvdata(pdev);
> +	struct drm_device *dev;
> +
> +	dev = pci_get_drvdata(pdev);
> +	if (!dev) /* driver load aborted, nothing to cleanup */
> +		return;
> 	i915_driver_unload(dev);
>  	drm_dev_put(dev);
> +
> +	pci_set_drvdata(pdev, NULL);

Why we can't put this inside i915_driver_release() ?
Then it will be in sync with kfree()

Michal

>  }
> static int i915_pci_probe(struct pci_dev *pdev, const struct  
> pci_device_id *ent)
> @@ -712,6 +718,11 @@ static int i915_pci_probe(struct pci_dev *pdev,  
> const struct pci_device_id *ent)
>  	if (err)
>  		return err;
> +	if (i915_inject_load_failure()) {
> +		i915_pci_remove(pdev);
> +		return -ENODEV;
> +	}
> +
>  	err = i915_live_selftests(pdev);
>  	if (err) {
>  		i915_pci_remove(pdev);