linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Hans Verkuil (hansverk)" <hansverk@cisco.com>
To: Wen Yang <yellowriver2010@hotmail.com>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: 答复: [PATCH 1/4] media: cec-notifier: fix possible object reference leak
Date: Tue, 12 Feb 2019 13:39:44 +0000	[thread overview]
Message-ID: <7021cb53c38d49c8b111bc67503f70f0@XCH-ALN-012.cisco.com> (raw)
In-Reply-To: HK0PR02MB3634D1937109CD28ECBF1654B2650@HK0PR02MB3634.apcprd02.prod.outlook.com

Hi Wen,

On 2/12/19 2:04 PM, Wen Yang wrote:
> Hi Hans, thank you for your comments.
> I will use my company's mailbox and submit a v2 patch to fix these problems tomorrow.

I made a bunch of patches fixing this yesterday. It's available here:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=cec-refcnt

I haven't posted anything since I had no chance to test it with actual
hardware. But if you have hardware and are able to verify that this
solves the issue, then please let me know so that I can get this merged.

Regards,

	Hans

> 
> Regards,
> 
>         Wen
> 
> ________________________________________
> 发件人: Hans Verkuil (hansverk) <hansverk@cisco.com>
> 发送时间: 2019年2月11日 10:57
> 收件人: Wen Yang; Hans Verkuil; Mauro Carvalho Chehab
> 抄送: Linux Media Mailing List; LKML
> 主题: Re: [PATCH 1/4] media: cec-notifier: fix possible object reference leak
> 
> On 11/02/2019 11:38, Hans Verkuil (hansverk) wrote:
>> On 09/02/2019 03:48, Wen Yang wrote:
>>> put_device() should be called in cec_notifier_release(),
>>> since the dev is being passed down to cec_notifier_get_conn(),
>>> which holds reference. On cec_notifier destruction, it
>>> should drop the reference to the device.
>>>
>>> Fixes: 6917a7b77413 ("[media] media: add CEC notifier support")
>>> Signed-off-by: Wen Yang <yellowriver2010@hotmail.com>
>>> ---
>>>  drivers/media/cec/cec-notifier.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/media/cec/cec-notifier.c b/drivers/media/cec/cec-notifier.c
>>> index dd2078b..621d4ae 100644
>>> --- a/drivers/media/cec/cec-notifier.c
>>> +++ b/drivers/media/cec/cec-notifier.c
>>> @@ -66,6 +66,7 @@ static void cec_notifier_release(struct kref *kref)
>>>              container_of(kref, struct cec_notifier, kref);
>>>
>>>      list_del(&n->head);
>>> +    put_device(n->dev);
>>>      kfree(n->conn);
>>>      kfree(n);
>>>  }
>>>
>>
>> Sorry, no. The dev pointer is just a search key that the notifier code looks
>> for. It is not the notifier's responsibility to take a reference, that would
>> be the responsibility of the hdmi and cec drivers.
> 
> Correction: the cec driver should never take a reference of the hdmi device.
> It never accesses the HDMI device, it only needs the HDMI device pointer as
> a key in the notifier list.
> 
> The real problem is that several CEC drivers take a reference of the HDMI device
> and never release it. So those drivers need to be fixed.
> 
> Regards,
> 
>         Hans
> 
>>
>> If you can demonstrate that there is an object reference leak, then please
>> provide the details: it is likely a bug elsewhere and not in the notifier
>> code.
>>
>> BTW, your patch series didn't arrive on the linux-media mailinglist for
>> some reason.
>>
>> Regards,
>>
>>       Hans
>>
> 
> 


      parent reply	other threads:[~2019-02-12 13:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <HK0PR02MB363461179B3A702DB9CAEC55B26A0@HK0PR02MB3634.apcprd02.prod.outlook.com>
2019-02-11 10:38 ` [PATCH 1/4] media: cec-notifier: fix possible object reference leak Hans Verkuil (hansverk)
2019-02-11 10:57   ` Hans Verkuil (hansverk)
     [not found]     ` <HK0PR02MB3634D1937109CD28ECBF1654B2650@HK0PR02MB3634.apcprd02.prod.outlook.com>
2019-02-12 13:39       ` Hans Verkuil (hansverk) [this message]

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=7021cb53c38d49c8b111bc67503f70f0@XCH-ALN-012.cisco.com \
    --to=hansverk@cisco.com \
    --cc=hans.verkuil@cisco.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=yellowriver2010@hotmail.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;
as well as URLs for NNTP newsgroup(s).