From: Joseph Qi <joseph.qi@linux.alibaba.com>
To: Cen Zhang <zzzccc427@gmail.com>, akpm@linux-foundation.org
Cc: mark@fasheh.com, jlbec@evilplan.org, heming.zhao@suse.com,
kees@kernel.org, kmehltretter@gmail.com,
christophe.jaillet@wanadoo.fr, ericterminal@gmail.com,
rppt@kernel.org, rollkingzzc@gmail.com, kurt.hackel@oracle.com,
brauner@kernel.org, jack@suse.cz, ocfs2-devel@lists.linux.dev,
linux-kernel@vger.kernel.org, stable@vger.kernel.org,
baijiaju1990@gmail.com, jjzuming@gmail.com
Subject: Re: [PATCH v3] ocfs2: Pin owning modules for open debugfs files
Date: Tue, 20 Oct 2026 11:46:51 +0800 [thread overview]
Message-ID: <a5aad315-d1a6-44cd-9b2c-23b2e8a0a3f1@linux.alibaba.com> (raw)
In-Reply-To: <pm-ocfs2-objects-candidate-0089-v6-f7fda843f80887488145@gmail.com>
On 10/16/26 9:17 AM, Cen Zhang wrote:
> The OCFS2 debugfs file operation tables do not set owner. The
> full debugfs proxy obtains real_fops with fops_get() at open and
> keeps it until final release. Without an owner, this does not acquire
> a reference to the module containing the table and its callbacks.
> Removing the debugfs entries does not drain retained open files.
>
> An open connected_nodes file can therefore outlive ocfs2_nodemanager
> and make full_proxy_release() read real_fops->release from freed
> module memory. Heming also reproduced the issue with fs_state and
> asked for the other OCFS2 debugfs operation tables to be checked.
>
> Set owner to THIS_MODULE in all eleven explicit debugfs operation
> tables: five in ocfs2_nodemanager, four in ocfs2_dlm, and two in
> ocfs2. The existing debugfs get/put pair then holds the appropriate
> module until final release. Leave the open, read and release callbacks
> unchanged. The blockcheck table already sets owner through
> DEFINE_DEBUGFS_ATTRIBUTE(), so it needs no change.
>
> The original controlled test used an unloadable OCFS2 module and
> case-local probe filters without changing the candidate open, unload
> or release logic. Closing a retained connected_nodes file after
> normal unload produced:
>
> BUG: unable to handle page fault for address: fffffbfff804cfef
> Oops: Oops: 0000 [#1] SMP KASAN NOPTI
> RIP: 0010:full_proxy_release+0xbf/0x190
> Call Trace:
> <TASK>
> __fput+0x39a/0xa60
> fput_close_sync+0xff/0x200
> __x64_sys_close+0x8c/0xf0
> do_syscall_64+0x114/0x620
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> </TASK>
> Modules linked in: [last unloaded: ocfs2_nodemanager]
>
> Fixes: 2309e9e040fe ("ocfs2/net: Add debug interface to o2net")
> Fixes: 87d3d3f3931f ("ocfs2/hb: Expose the list of heartbeating nodes via debugfs")
> Fixes: 3ba169ccec1c ("ocfs2/cluster: Add new function o2net_fill_node_map()")
> Fixes: 50397507e856 ("ocfs2: Expose the file system state via debugfs")
> Fixes: ccd979bdbce9 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
> Fixes: 7209300a9b98 ("ocfs2/dlm: Dumps the purgelist into a debugfs file")
> Fixes: d0129aceaecc ("ocfs2/dlm: Dumps the mles into a debugfs file")
> Fixes: 4e3d24ed1a12 ("ocfs2/dlm: Dumps the lockres' into a debugfs file")
> Fixes: 007dce53a29c ("ocfs2/dlm: Dump the dlm state in a debugfs file")
> Assisted-by: LLM
> Cc: <stable@vger.kernel.org>
> Suggested-by: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
>
> ---
> Changes in v3:
> - Match each .owner assignment to the indentation of neighboring members
> throughout, as requested by Kees Cook.
>
> Changes in v2:
> - Cover fs_state and locking_state in the OCFS2 filesystem module.
> - Cover all four OCFS2 DLM debugfs operation tables.
> - Keep the five node manager fixes and the original diagnostic.
> - Add explicit module headers and Fixes tags for the added tables.
> - Add Cc: stable@vger.kernel.org.
>
> Link to v1: https://lore.kernel.org/r/pm-ocfs2-objects-candidate-0089-v2-4c67f48d4e2d0fc85990@gmail.com
> Link to v2: https://lore.kernel.org/r/pm-ocfs2-objects-candidate-0089-v5-076c780eeaf45afb2a53@gmail.com
>
> fs/ocfs2/cluster/heartbeat.c | 1 +
> fs/ocfs2/cluster/netdebug.c | 4 ++++
> fs/ocfs2/dlm/dlmdebug.c | 5 +++++
> fs/ocfs2/dlmglue.c | 2 ++
> fs/ocfs2/super.c | 1 +
> 5 files changed, 13 insertions(+)
>
> diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
> index 8e6ead433228e7419bfbbe5c7cdde65c654a24f4..1cb516df53f4e1b3b2a292eb2550faa3d16e3fa2 100644
> --- a/fs/ocfs2/cluster/heartbeat.c
> +++ b/fs/ocfs2/cluster/heartbeat.c
> @@ -1447,6 +1447,7 @@ static ssize_t o2hb_debug_read(struct file *file, char __user *buf,
> #endif /* CONFIG_DEBUG_FS */
>
> static const struct file_operations o2hb_debug_fops = {
> + .owner = THIS_MODULE,
> .open = o2hb_debug_open,
> .release = o2hb_debug_release,
> .read = o2hb_debug_read,
> diff --git a/fs/ocfs2/cluster/netdebug.c b/fs/ocfs2/cluster/netdebug.c
> index f7ab66e70799302f3ed4e70ab6a8fb057dd0bfa9..5d373002297de1fdd9a05e270f7942d8c1ce2dac 100644
> --- a/fs/ocfs2/cluster/netdebug.c
> +++ b/fs/ocfs2/cluster/netdebug.c
> @@ -183,6 +183,7 @@ static int nst_fop_release(struct inode *inode, struct file *file)
> }
>
> static const struct file_operations nst_seq_fops = {
> + .owner = THIS_MODULE,
> .open = nst_fop_open,
> .read = seq_read,
> .llseek = seq_lseek,
> @@ -415,6 +416,7 @@ static int stats_fop_open(struct inode *inode, struct file *file)
> }
>
> static const struct file_operations stats_seq_fops = {
> + .owner = THIS_MODULE,
> .open = stats_fop_open,
> .read = seq_read,
> .llseek = seq_lseek,
Seems it misses the Fixes tag for stats_seq_fops:
db02754c8a12 ocfs2/cluster: Show o2net timing statistics
Other looks fine to me.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
prev parent reply other threads:[~2026-10-20 3:46 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-16 1:17 Cen Zhang
2026-10-20 3:46 ` Joseph Qi [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=a5aad315-d1a6-44cd-9b2c-23b2e8a0a3f1@linux.alibaba.com \
--to=joseph.qi@linux.alibaba.com \
--cc=akpm@linux-foundation.org \
--cc=baijiaju1990@gmail.com \
--cc=brauner@kernel.org \
--cc=christophe.jaillet@wanadoo.fr \
--cc=ericterminal@gmail.com \
--cc=heming.zhao@suse.com \
--cc=jack@suse.cz \
--cc=jjzuming@gmail.com \
--cc=jlbec@evilplan.org \
--cc=kees@kernel.org \
--cc=kmehltretter@gmail.com \
--cc=kurt.hackel@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark@fasheh.com \
--cc=ocfs2-devel@lists.linux.dev \
--cc=rollkingzzc@gmail.com \
--cc=rppt@kernel.org \
--cc=stable@vger.kernel.org \
--cc=zzzccc427@gmail.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
all inboxes | Powered by JetHome®