From: Max Kellermann <max@duempel.org>
To: Shuah Khan <shuahkh@osg.samsung.com>,
linux-media@vger.kernel.org, mchehab@osg.samsung.com,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] drivers/media/media-device: fix double free bug in _unregister()
Date: Thu, 16 Jun 2016 11:29:26 +0200 [thread overview]
Message-ID: <20160616092926.GA1333@swift.blarg.de> (raw)
In-Reply-To: <20160615203753.GA30666@swift.blarg.de>
(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".
next prev parent reply other threads:[~2016-06-16 9:29 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-15 20:15 [PATCH 1/3] drivers/media/dvb-core/en50221: use kref to manage struct dvb_ca_private Max Kellermann
2016-06-15 20:15 ` [PATCH 2/3] drivers/media/media-entity: clear media_gobj.mdev in _destroy() Max Kellermann
2016-06-16 16:24 ` Shuah Khan
2016-06-16 18:43 ` Max Kellermann
2016-06-16 18:55 ` Shuah Khan
2016-06-17 12:53 ` Sakari Ailus
2016-06-17 13:04 ` Max Kellermann
2016-06-15 20:15 ` [PATCH 3/3] drivers/media/media-device: fix double free bug in _unregister() Max Kellermann
2016-06-15 20:32 ` Shuah Khan
2016-06-15 20:37 ` Max Kellermann
2016-06-15 21:50 ` Shuah Khan
2016-06-16 9:29 ` Max Kellermann [this message]
2016-06-16 13:40 ` Shuah Khan
2016-06-16 16:06 ` [PATCH 1/3] drivers/media/dvb-core/en50221: use kref to manage struct dvb_ca_private Shuah Khan
2016-06-16 18:37 ` Max Kellermann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160616092926.GA1333@swift.blarg.de \
--to=max@duempel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@osg.samsung.com \
--cc=shuahkh@osg.samsung.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox