[Devel] [PATCH v2] nfs: protect callback execution against per-net callback thread shutdown

2017-11-07 Thread Stanislav Kinsburskiy
Here is the race:

CPU #0  CPU#1

cleanup_mnt nfs41_callback_svc (get xprt from the list)
nfs_callback_down   ...
... ...
svc_close_net   ...
... ...
svc_xprt_free   ...
svc_bc_sock_freebc_svc_process
kfree(xprt) svc_process_common
rqstp->rq_xprt->xpt_ops (use after free)

The problem is that per-net SUNRPC transports shutdown is done regardless
current callback execution. This is a race leading to transport use-after-free
in callback handler.
This patch fixes it in stright-forward way. I.e. it protects callback
execution with the same mutex used for per-net data creation and destruction.
Hopefully, it won't slow down NFS client significantly.

https://jira.sw.ru/browse/PSBM-75751

v2: Fix mutex deadlock, when shutdown callback waits for thread to exit (with
mutex taken), while thread wait for the mutex to take.

Signed-off-by: Stanislav Kinsburskiy 
---
 fs/nfs/callback.c |7 +++
 1 file changed, 7 insertions(+)

diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
index 0beb275..eed861a 100644
--- a/fs/nfs/callback.c
+++ b/fs/nfs/callback.c
@@ -118,6 +118,11 @@ nfs41_callback_svc(void *vrqstp)
continue;
 
prepare_to_wait(&serv->sv_cb_waitq, &wq, TASK_INTERRUPTIBLE);
+
+   if (!mutex_trylock(&nfs_callback_mutex) &&
+   kthread_should_stop())
+   return 0;
+
spin_lock_bh(&serv->sv_cb_lock);
if (!list_empty(&serv->sv_cb_list)) {
req = list_first_entry(&serv->sv_cb_list,
@@ -129,8 +134,10 @@ nfs41_callback_svc(void *vrqstp)
error = bc_svc_process(serv, req, rqstp);
dprintk("bc_svc_process() returned w/ error code= %d\n",
error);
+   mutex_unlock(&nfs_callback_mutex);
} else {
spin_unlock_bh(&serv->sv_cb_lock);
+   mutex_unlock(&nfs_callback_mutex);
schedule();
finish_wait(&serv->sv_cb_waitq, &wq);
}

___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel


Re: [Devel] [PATCH v2] nfs: protect callback execution against per-net callback thread shutdown

2017-11-07 Thread Stanislav Kinsburskiy
Sorry, this one is bad as well.

07.11.2017 11:28, Stanislav Kinsburskiy пишет:
> Here is the race:
> 
> CPU #0CPU#1
> 
> cleanup_mnt   nfs41_callback_svc (get xprt from the list)
> nfs_callback_down ...
> ...   ...
> svc_close_net ...
> ...   ...
> svc_xprt_free ...
> svc_bc_sock_free  bc_svc_process
> kfree(xprt)   svc_process_common
>   rqstp->rq_xprt->xpt_ops (use after free)
> 
> The problem is that per-net SUNRPC transports shutdown is done regardless
> current callback execution. This is a race leading to transport use-after-free
> in callback handler.
> This patch fixes it in stright-forward way. I.e. it protects callback
> execution with the same mutex used for per-net data creation and destruction.
> Hopefully, it won't slow down NFS client significantly.
> 
> https://jira.sw.ru/browse/PSBM-75751
> 
> v2: Fix mutex deadlock, when shutdown callback waits for thread to exit (with
> mutex taken), while thread wait for the mutex to take.
> 
> Signed-off-by: Stanislav Kinsburskiy 
> ---
>  fs/nfs/callback.c |7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/nfs/callback.c b/fs/nfs/callback.c
> index 0beb275..eed861a 100644
> --- a/fs/nfs/callback.c
> +++ b/fs/nfs/callback.c
> @@ -118,6 +118,11 @@ nfs41_callback_svc(void *vrqstp)
>   continue;
>  
>   prepare_to_wait(&serv->sv_cb_waitq, &wq, TASK_INTERRUPTIBLE);
> +
> + if (!mutex_trylock(&nfs_callback_mutex) &&
> + kthread_should_stop())
> + return 0;
> +
>   spin_lock_bh(&serv->sv_cb_lock);
>   if (!list_empty(&serv->sv_cb_list)) {
>   req = list_first_entry(&serv->sv_cb_list,
> @@ -129,8 +134,10 @@ nfs41_callback_svc(void *vrqstp)
>   error = bc_svc_process(serv, req, rqstp);
>   dprintk("bc_svc_process() returned w/ error code= %d\n",
>   error);
> + mutex_unlock(&nfs_callback_mutex);
>   } else {
>   spin_unlock_bh(&serv->sv_cb_lock);
> + mutex_unlock(&nfs_callback_mutex);
>   schedule();
>   finish_wait(&serv->sv_cb_waitq, &wq);
>   }
> 
> ___
> Devel mailing list
> Devel@openvz.org
> https://lists.openvz.org/mailman/listinfo/devel
> 
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel