Re: [PATCH 3/3] drivers/media/media-device: fix double free bug in _unregister()

From: Max Kellermann
Date: Thu Jun 16 2016 - 05:29:34 EST


(Shuah, I did not receive your second reply; I only found it in an
email archive.)

> Yes media_devnode_create() creates the interfaces links and these
> links are deleted by media_devnode_remove().
> media_device_unregister() still needs to delete the interfaces
> links. The reason for that is the API dynalic use-case.
>
> Drivers (other than dvb-core and v4l2-core) can create and delete
> media devnode interfaces during run-time

My point was that they do not. There are no other
media_devnode_create() callers.

> So removing kfree() from media_device_unregister() isn't the correct
> fix.

Then what is? I don't know anything other than the (mostly
undocumented) code I read, and my patch implements the design that I
interpreted from the code. Apparently my interpretation of the design
is wrong after all.

> I don't see the stack trace for the double free error you are
> seeing?

Actually, it didn't crash at the double free; it hung forever because
it tried to lock a mutex which was already stale. I don't have a
stack trace of that; would it help to produce one?

> Could it be that there is a driver problem in the order in which it
> is calling media_device_unregister()?

Maybe it's due to my patch 1/3 which adds a kref, and it only occurs
if one process still has a file handle.

In any case, the kernel must decide who's responsible for freeing the
object, and how the dvbdev.c library gets to know that its pointer has
been invalidated.

Please explain how it should be done, and I'll try to adapt my patches
to the "grand design".