Re: [PATCH] ocfs2: Initialize status waiters before publishing them

[Joseph Qi]

On 10/9/26 9:43 AM, Cen Zhang wrote:
> o2net_prep_nsw() publishes its stack waiter in nn_status_idr and
> nn_status_list under nn_lock, then initializes the wait queue and
> completion status after releasing the lock. A concurrent disconnect
> can find that waiter through o2net_complete_nodes_nsw() and call
> wake_up() before the wait queue has a valid lock and list head. The
> sender can also overwrite a completion status with its initial value.
> 
> Initialize the wait queue and status before publishing the waiter.
> nn_lock then orders publication against completion, and no reader can
> observe a partially initialized object. Leave the IDR allocation and
> its failure handling unchanged.
> 
> A controlled case-modified test kernel used a synthetic connection to
> enter the real send path and overlap publication with disconnect. The
> candidate completion and wake-up code were unchanged. Its Oops includes:
> 
>   KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
>   RIP: 0010:__wake_up_common+0xa0/0x1f0
>   Call Trace:
>    <TASK>
>    __wake_up+0x36/0x60
>    o2net_complete_nsw_locked+0x222/0x370
>    o2net_set_nn_state+0x917/0xea0
>    o2net_disconnect_node+0xd0/0x190
>    o2net_validate_control_write+0x454/0x500
>    full_proxy_write+0x11f/0x180
>    vfs_write+0x25a/0x1010
>    ksys_write+0x111/0x200
>    do_syscall_64+0x114/0x620
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>    </TASK>
>   Modules linked in:
> 
> Fixes: 98211489d414 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
> Assisted-by: LLM
> Signed-off-by: Cen Zhang <zzzccc427@xxxxxxxxx>

Looks fine.
Reviewed-by: Joseph Qi <joseph.qi@xxxxxxxxxxxxxxxxx>

> ---
>  fs/ocfs2/cluster/tcp.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ocfs2/cluster/tcp.c b/fs/ocfs2/cluster/tcp.c
> index 474fe1414cee8609d7976b983332c6e120f06fb5..8b3830b601ba6e5fe964bec970581b8e4ddbb5dd 100644
> --- a/fs/ocfs2/cluster/tcp.c
> +++ b/fs/ocfs2/cluster/tcp.c
> @@ -301,6 +301,11 @@ static int o2net_prep_nsw(struct o2net_node *nn, struct o2net_status_wait *nsw)
>  {
>  	int ret;
>  
> +	/* Initialize all completion-visible state before publishing the waiter. */
> +	init_waitqueue_head(&nsw->ns_wq);
> +	nsw->ns_sys_status = O2NET_ERR_NONE;
> +	nsw->ns_status = 0;
> +
>  	spin_lock(&nn->nn_lock);
>  	ret = idr_alloc(&nn->nn_status_idr, nsw, 0, 0, GFP_ATOMIC);
>  	if (ret >= 0) {
> @@ -311,9 +316,6 @@ static int o2net_prep_nsw(struct o2net_node *nn, struct o2net_status_wait *nsw)
>  	if (ret < 0)
>  		return ret;
>  
> -	init_waitqueue_head(&nsw->ns_wq);
> -	nsw->ns_sys_status = O2NET_ERR_NONE;
> -	nsw->ns_status = 0;
>  	return 0;
>  }
>