[RFC PATCH v3 4/4] tests/drv_module_reload: add ipvr support

[Thierry Reding]

On Mon, Nov 24, 2014 at 02:14:48PM +0100, Daniel Vetter wrote:
> On Mon, Nov 24, 2014 at 10:55:46AM +0100, Thierry Reding wrote:
> > On Fri, Nov 21, 2014 at 09:36:33PM +0100, Daniel Vetter wrote:
> > > On Fri, Nov 21, 2014 at 09:27:04PM +0100, Thierry Reding wrote:
> > > > On Sat, Nov 22, 2014 at 03:10:01AM +0800, Yao Cheng wrote:
> > > > > on vlv, if ipvr is installed, it need be manually unloaded before
> > > > > i915, otherwise user might run into use-after-free issue.
> > > > 
> > > > Huh? That doesn't sound right. What exactly is it that's going wrong?
> > > > You should never have to do this. If you do you're almost certainly
> > > > doing something wrong in the kernel module.
> > > 
> > > It's the hilarity called platform devices. Removing them is somewhat racy,
> > > so doing that upfront makes the entire thing a bit safer. The use after
> > > free is on the text, since grabbing a module refcount for the platform
> > > device doesn't work (it would pin the module forever).
> > 
> > I don't understand what the issue is here. I've used platform devices
> > quite extensively on ARM and I've never encountered a situation where
> > they were insufficient (or racy for that matter).
> > 
> > If I understand correctly what this commit tries to achieve, then it
> > unloads one module before another module that it depends on so that the
> > dependency can be removed subsequently without causing a crash. That
> > sounds really brittle to me. How are you going to document this for
> > users so that they don't accidentally go and unload the i915 module and
> > crash their system?
> 
> Module unloading taints your kernel and isn't an end-user supported
> feature. That simple ;-)
> 
> Also afaik the problem is that you actually can't unload i915 until you've
> unloaded the subordinate driver, since i915 registering the platform
> driver prevents unload.

That doesn't sound at all like use-after-free, so if that's really the
only problem then the commit description should be more accurate.

> Or at least that was my understanding, I didn't test this myself. I
> just asked whether the unload script still works and apparently it
> breaks.
> 
> I guess what's different with ARM is that DT creates all the platform
> devices, and not modules themselves?

No, I don't think that has anything to do with it. I'm pretty sure I've
seen this work reliably with something like MFD where one module can
create a number of platform devices, and remove them again, just as
well.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20141217/24ca9713/attachment-0001.sig>