Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1423843

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

From Max Kellermann <max@duempel.org>
Newsgroups linux.kernel
Subject Re: [PATCH 3/3] drivers/media/media-device: fix double free bug in _unregister()
Date 2016-06-16 11:30 +0200
Message-ID <rKBMB-6A6-13@gated-at.bofh.it> (permalink)
References <rKpBL-7cb-7@gated-at.bofh.it> <rKpBL-7cb-5@gated-at.bofh.it> <rKpLs-7fO-13@gated-at.bofh.it> <rKpLs-7fO-11@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


(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".

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 3/3] drivers/media/media-device: fix double free bug in  _unregister() Max Kellermann <max@duempel.org> - 2016-06-15 22:30 +0200
  Re: [PATCH 3/3] drivers/media/media-device: fix double free bug in  _unregister() Max Kellermann <max@duempel.org> - 2016-06-15 22:40 +0200
    Re: [PATCH 3/3] drivers/media/media-device: fix double free bug in  _unregister() Shuah Khan <shuahkh@osg.samsung.com> - 2016-06-16 00:00 +0200
    Re: [PATCH 3/3] drivers/media/media-device: fix double free bug in  _unregister() Max Kellermann <max@duempel.org> - 2016-06-16 11:30 +0200
      Re: [PATCH 3/3] drivers/media/media-device: fix double free bug in  _unregister() Shuah Khan <shuahkh@osg.samsung.com> - 2016-06-16 15:50 +0200
  Re: [PATCH 3/3] drivers/media/media-device: fix double free bug in  _unregister() Shuah Khan <shuahkh@osg.samsung.com> - 2016-06-15 22:40 +0200

csiph-web