Quoting Michal Wajdeczko (2018-07-16 11:32:22)
> On Mon, 16 Jul 2018 10:03:31 +0200, Chris Wilson
> <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> > ---
> > 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()
It pairs here with the pci remove. The driver release callback (being
the kref_put release) can happen later. That was my other choice, but I
went with doing it here because this we have the pci device here and the
synergy with the get+test followed set. My thinking was that if we tried
to dig out the pci device in a later release function (or pci callback
after remove), that should be clearly erroneous.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
