All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolai Stange <nstange@suse.de>
To: Taehee Yoo <ap420073@gmail.com>
Cc: Johannes Berg <johannes@sipsolutions.net>,
	David Laight <David.Laight@aculab.com>,
	"davem\@davemloft.net" <davem@davemloft.net>,
	"kuba\@kernel.org" <kuba@kernel.org>,
	"netdev\@vger.kernel.org" <netdev@vger.kernel.org>,
	Nicolai Stange <nstange@suse.de>,
	"linux-wireless\@vger.kernel.org"
	<linux-wireless@vger.kernel.org>,
	"wil6210\@qti.qualcomm.com" <wil6210@qti.qualcomm.com>,
	"brcm80211-dev-list\@cypress.com"
	<brcm80211-dev-list@cypress.com>,
	"b43-dev\@lists.infradead.org" <b43-dev@lists.infradead.org>,
	"linux-bluetooth\@vger.kernel.org"
	<linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH net 000/117] net: avoid to remove module when its debugfs is being used
Date: Fri, 09 Oct 2020 07:38:59 +0200	[thread overview]
Message-ID: <87r1q8gdqk.fsf@suse.de> (raw)
In-Reply-To: <CAMArcTUkC2MzN9MiTu_Qwouj6rFf0g0ac2uZWfSKWHTW9cR8xA@mail.gmail.com> (Taehee Yoo's message of "Fri, 9 Oct 2020 01:37:26 +0900")

Taehee Yoo <ap420073@gmail.com> writes:

> On Fri, 9 Oct 2020 at 01:14, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2020-10-08 at 15:59 +0000, David Laight wrote:
>
>> From: Taehee Yoo
>> > Sent: 08 October 2020 16:49
>> >
>> > When debugfs file is opened, its module should not be removed until
>> > it's closed.
>> > Because debugfs internally uses the module's data.
>> > So, it could access freed memory.

Yes, the file_operations' ->release() to be more specific -- that's not
covered by debugfs' proxy fops.


>> > In order to avoid panic, it just sets .owner to THIS_MODULE.
>> > So that all modules will be held when its debugfs file is opened.
>>
>> Can't you fix it in common code?
>
>> Yeah I was just wondering that too - weren't the proxy_fops even already
>> intended to fix this?
>
> I didn't try to fix this issue in the common code(debugfs).
> Because I thought It's a typical pattern of panic and THIS_MODULE
> can fix it clearly.
> So I couldn't think there is a root reason in the common code.

That's correct, ->owner should get set properly, c.f. my other mail in
this thread.


Thanks,

Nicolai

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Felix Imendörffer

WARNING: multiple messages have this Message-ID (diff)
From: Nicolai Stange <nstange@suse.de>
To: Taehee Yoo <ap420073@gmail.com>
Cc: Johannes Berg <johannes@sipsolutions.net>,
	David Laight <David.Laight@aculab.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Nicolai Stange <nstange@suse.de>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"wil6210@qti.qualcomm.com" <wil6210@qti.qualcomm.com>,
	"brcm80211-dev-list@cypress.com" <brcm80211-dev-list@cypress.com>,
	"b43-dev@lists.infradead.org" <b43-dev@lists.infradead.org>,
	"linux-bluetooth@vger.kernel.org"
	<linux-bluetooth@vger.kernel.org>
Subject: [PATCH net 000/117] net: avoid to remove module when its debugfs is being used
Date: Fri, 09 Oct 2020 07:38:59 +0200	[thread overview]
Message-ID: <87r1q8gdqk.fsf@suse.de> (raw)
In-Reply-To: <CAMArcTUkC2MzN9MiTu_Qwouj6rFf0g0ac2uZWfSKWHTW9cR8xA@mail.gmail.com> (Taehee Yoo's message of "Fri, 9 Oct 2020 01:37:26 +0900")

Taehee Yoo <ap420073@gmail.com> writes:

> On Fri, 9 Oct 2020 at 01:14, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2020-10-08 at 15:59 +0000, David Laight wrote:
>
>> From: Taehee Yoo
>> > Sent: 08 October 2020 16:49
>> >
>> > When debugfs file is opened, its module should not be removed until
>> > it's closed.
>> > Because debugfs internally uses the module's data.
>> > So, it could access freed memory.

Yes, the file_operations' ->release() to be more specific -- that's not
covered by debugfs' proxy fops.


>> > In order to avoid panic, it just sets .owner to THIS_MODULE.
>> > So that all modules will be held when its debugfs file is opened.
>>
>> Can't you fix it in common code?
>
>> Yeah I was just wondering that too - weren't the proxy_fops even already
>> intended to fix this?
>
> I didn't try to fix this issue in the common code(debugfs).
> Because I thought It's a typical pattern of panic and THIS_MODULE
> can fix it clearly.
> So I couldn't think there is a root reason in the common code.

That's correct, ->owner should get set properly, c.f. my other mail in
this thread.


Thanks,

Nicolai

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
(HRB 36809, AG N?rnberg), GF: Felix Imend?rffer

  reply	other threads:[~2020-10-09  5:39 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-08 15:48 [PATCH net 000/117] net: avoid to remove module when its debugfs is being used Taehee Yoo
2020-10-08 15:59 ` David Laight
2020-10-08 15:59   ` David Laight
2020-10-08 16:14   ` Johannes Berg
2020-10-08 16:14     ` Johannes Berg
2020-10-08 16:37     ` Taehee Yoo
2020-10-08 16:37       ` Taehee Yoo
2020-10-09  5:38       ` Nicolai Stange [this message]
2020-10-09  5:38         ` Nicolai Stange
2020-10-09 10:07         ` Taehee Yoo
2020-10-09 10:07           ` Taehee Yoo
2020-10-09  5:09     ` Nicolai Stange
2020-10-09  5:09       ` Nicolai Stange
2020-10-09  7:45       ` Johannes Berg
2020-10-09  7:45         ` Johannes Berg
2020-10-09 10:15         ` Taehee Yoo
2020-10-09 10:15           ` Taehee Yoo
2020-10-09 10:21           ` Johannes Berg
2020-10-09 10:21             ` Johannes Berg
2020-10-09 10:41             ` [RFC] debugfs: protect against rmmod while files are open Johannes Berg
2020-10-09 10:48               ` Johannes Berg
2020-10-09 10:56                 ` David Laight
2020-10-09 10:56                   ` Johannes Berg
2020-10-09 11:15                   ` gregkh
2020-10-09 15:33             ` [PATCH net 000/117] net: avoid to remove module when its debugfs is being used Steve deRosier
2020-10-09 15:33               ` Steve deRosier
2020-10-09  7:53       ` [CRAZY-RFF] debugfs: track open files and release on remove Johannes Berg
2020-10-09  8:03         ` Greg KH
2020-10-09  8:06           ` Johannes Berg
2020-10-09  8:16             ` Greg KH
2020-10-09  8:19               ` Johannes Berg
2020-10-09  8:34                 ` David Laight
2020-10-09  8:44                   ` Johannes Berg
2020-10-09  9:00                     ` David Laight
2020-10-09  8:47                 ` Greg KH
2020-10-09  8:48                   ` Johannes Berg
2020-10-10  9:38                     ` Greg KH
2020-10-10 10:47                       ` Johannes Berg

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=87r1q8gdqk.fsf@suse.de \
    --to=nstange@suse.de \
    --cc=David.Laight@aculab.com \
    --cc=ap420073@gmail.com \
    --cc=b43-dev@lists.infradead.org \
    --cc=brcm80211-dev-list@cypress.com \
    --cc=davem@davemloft.net \
    --cc=johannes@sipsolutions.net \
    --cc=kuba@kernel.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=wil6210@qti.qualcomm.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.