Access locking_state of dlm debugfs may cause crash as scene below:

Proc A:                  Proc that access debuginfo:
add_lockres_tracking(lockresA)
...
                         ocfs2_dlm_seq_next():
                           //priv->p_iter_res points to next
                           //lockres e.g. B. priv->p_tmp_res hold
                           //copy of lockres A before leave
                         ocfs2_dlm_seq_show()
...
remove_lockres_tracking(lockres B):
  //free lockres B, l_debug_list in
  //priv->p_ter_res is updated but not
  //priv->p_tmp_res
...
                         ocfs2_dlm_seq_next():
			   //priv->p_tmp_res which holds a old copy of
                           //lockres A, the l_debug_list holds a
                           //out-of-date succeed pointer, which will
                           //cause crash as //access invalid memory
                           iter = v; //priv->p_tmp_res
                           iter = ocfs2_dlm_next_res(iter, priv)

The root cause of this issue is that private->p_iter_res acts as the
agent of accessing lockres and is protected by ocfs2_dlm_tracking_lock
while p_tmp_res is only a copy of the lockres and will be out-of-dated
after leave critial region of ocfs2_dlm_tracking_lock. We should use
priv->p_ter_res as the forward iterater instead.

Signed-off-by: Rock Li <lihongweizz@inspur.com>
---
 fs/ocfs2/dlmglue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index c28bc98..5d84350 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -3109,7 +3109,7 @@ static void *ocfs2_dlm_seq_next(struct seq_file *m, void *v, loff_t *pos)
 	struct ocfs2_lock_res *dummy = &priv->p_iter_res;
 
 	spin_lock(&ocfs2_dlm_tracking_lock);
-	iter = ocfs2_dlm_next_res(iter, priv);
+	iter = ocfs2_dlm_next_res(dummy, priv);
 	list_del_init(&dummy->l_debug_list);
 	if (iter) {
 		list_add(&dummy->l_debug_list, &iter->l_debug_list);
-- 
1.8.3.1

Hi,
Sorry for the late reply.
It seems it is indeed an issue and I'll get into it more deeply.
I'm curious about how you figure out this? Is it a real issue you've
encountered?

Thanks,
Joseph

On 9/20/22 3:36 PM, Rock Li wrote:
> Access locking_state of dlm debugfs may cause crash as scene below:
> 
> Proc A:                  Proc that access debuginfo:
> add_lockres_tracking(lockresA)
> ...
>                          ocfs2_dlm_seq_next():
>                            //priv->p_iter_res points to next
>                            //lockres e.g. B. priv->p_tmp_res hold
>                            //copy of lockres A before leave
>                          ocfs2_dlm_seq_show()
> ...
> remove_lockres_tracking(lockres B):
>   //free lockres B, l_debug_list in
>   //priv->p_ter_res is updated but not
>   //priv->p_tmp_res
> ...
>                          ocfs2_dlm_seq_next():
> 			   //priv->p_tmp_res which holds a old copy of
>                            //lockres A, the l_debug_list holds a
>                            //out-of-date succeed pointer, which will
>                            //cause crash as //access invalid memory
>                            iter = v; //priv->p_tmp_res
>                            iter = ocfs2_dlm_next_res(iter, priv)
> 
> The root cause of this issue is that private->p_iter_res acts as the
> agent of accessing lockres and is protected by ocfs2_dlm_tracking_lock
> while p_tmp_res is only a copy of the lockres and will be out-of-dated
> after leave critial region of ocfs2_dlm_tracking_lock. We should use
> priv->p_ter_res as the forward iterater instead.
> 
> Signed-off-by: Rock Li <lihongweizz@inspur.com>
> ---
>  fs/ocfs2/dlmglue.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
> index c28bc98..5d84350 100644
> --- a/fs/ocfs2/dlmglue.c
> +++ b/fs/ocfs2/dlmglue.c
> @@ -3109,7 +3109,7 @@ static void *ocfs2_dlm_seq_next(struct seq_file *m, void *v, loff_t *pos)
>  	struct ocfs2_lock_res *dummy = &priv->p_iter_res;
>  
>  	spin_lock(&ocfs2_dlm_tracking_lock);
> -	iter = ocfs2_dlm_next_res(iter, priv);
> +	iter = ocfs2_dlm_next_res(dummy, priv);
>  	list_del_init(&dummy->l_debug_list);
>  	if (iter) {
>  		list_add(&dummy->l_debug_list, &iter->l_debug_list);